Skip to content

Commit 1b62bd8

Browse files
committed
src.ctf.fs: be less strict when encountering trailing byte at the end of packetized CTF 1.8 metadata
A client has some traces with CTF 1.8 packetized metadata that has an unexpected 0x0A (\n) byte at the end. The metadata file consists of a sequence of several well-formed metadata packets, followed by one 0x0A byte. It is not known where this byte comes from, it could have been inserted as the result of some post-processing of the metadata files. While not a valid metadata stream, Babeltrace 2.0 accepted it without complaining. The code in 2.0 looked like: readlen = fread(&header, sizeof(header), 1, in_fp); ... where `header` represents a metadata packet header. Because the number of remaining bytes to read was less than the size of `header`, `fread` would return 0. Babeltrace would conclude that it had reached the end of the file and would exit the loop, without ever noticing this extra byte. In 2.1, Babeltrace is more strict, and emits this error: CAUSED BY ['source.ctf.fs'] (/home/smarchi/src/babeltrace/src/plugins/ctf/common/src/metadata/tsdl/metadata-stream-decoder.cpp:314) Failed to decode metadata stream section: data-ptr=0x79d1d9fec900, data-len-bytes=4097 CAUSED BY ['source.ctf.fs'] (/home/smarchi/src/babeltrace/src/plugins/ctf/common/src/metadata/tsdl/metadata-stream-decoder.cpp:274) Failed to read a metadata stream packet: offset-bytes=4096, pkt-idx=1 CAUSED BY ['source.ctf.fs'] (/home/smarchi/src/babeltrace/src/plugins/ctf/common/src/metadata/tsdl/metadata-stream-decoder.cpp:244) Remaining buffer isn't large enough to hold a packet header. Even though this isn't technically a valid metadata stream, and we don't know where this mysterious 0x0A byte comes from, the fact that Babeltrace 2.0 accepted it and 2.1 rejects it can be a problem for real-world users, if traces like this are in the wild. Since this extra byte doesn't really prevent Babeltrace from reading the trace as it did before, I propose to change the error to a warning. Add a test for this, which consists of a copy of the `2packets`, to which I added a trailing 0x0A byte to the metadata file. The result looks like this: 05-14 14:47:52.527 107232 107232 W PLUGIN/CTF/META/DECODER _textFromPacketizedMetadata@/home/smarchi/src/babeltrace/src/plugins/ctf/common/src/metadata/tsdl/metadata-stream-decoder.cpp:244 Remaining buffer isn't large enough to hold a packet header. Note that this warning appears three times when relying on the CLI's autodisovery feature, because the metadata is parsed three times in this case: - once during the "support info" query - once during the "get supported MIP versions" method call - once when instantiating the component Change-Id: Ibaaacd52e8993e0b126691d3591dee67fb94b3a0 Signed-off-by: Simon Marchi <[email protected]> Reviewed-on: https://review.lttng.org/c/babeltrace/+/14630 Tested-by: jenkins <[email protected]> Reviewed-by: Philippe Proulx <[email protected]> (cherry picked from commit b83661e) Reviewed-on: https://review.lttng.org/c/babeltrace/+/14869
1 parent 0325525 commit 1b62bd8

File tree

6 files changed

+14
-4
lines changed

6 files changed

+14
-4
lines changed

src/plugins/ctf/common/src/metadata/tsdl/metadata-stream-decoder.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -241,8 +241,8 @@ std::string MetadataStreamDecoder::_textFromPacketizedMetadata(const bt2c::Const
241241
const auto pktData = buffer.data() + curOffset.bytes();
242242

243243
if (curOffset + _PktHeader::len > bt2c::DataLen::fromBytes(buffer.size())) {
244-
BT_CPPLOGE_APPEND_CAUSE_AND_THROW(
245-
bt2c::Error, "Remaining buffer isn't large enough to hold a packet header.");
244+
BT_CPPLOGW("Remaining buffer isn't large enough to hold a packet header.");
245+
break;
246246
}
247247

248248
const auto header = this->_readPktHeader(pktData, *byteOrder, curOffset);

tests/cli/test-trace-copy.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ clean_tmp() {
2020
rm -rf "${out_path}" "${text_output1}" "${text_output2_intermediary}" "${text_output2}" "${stderr_file}"
2121
}
2222

23-
plan_tests 166
23+
plan_tests 169
2424

2525
for ctf_version in 1 2; do
2626
for path in "${BT_CTF_TRACES_PATH}/$ctf_version/succeed/"*; do
Binary file not shown.
Binary file not shown.
4 KB
Binary file not shown.

tests/plugins/src.ctf.fs/succeed/test-succeed.sh

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,16 @@ test_clock_offset_goes_back_in_time() {
197197
rm -rf "$tmp_dir"
198198
}
199199

200-
plan_tests 47
200+
test_meta_trailing_byte() {
201+
local trace_path="$BT_CTF_TRACES_PATH/1/succeed/meta-trailing-byte"
202+
203+
204+
bt_test_cli "packetized metadata with trailing byte" \
205+
--expect-stderr-re "Remaining buffer isn't large enough to hold a packet header" -- \
206+
"$trace_path" -c sink.text.details
207+
}
208+
209+
plan_tests 49
201210

202211
test_force_origin_unix_epoch 2packets barectf-event-before-packet
203212
test_ctf_gen_single simple
@@ -211,6 +220,7 @@ test_ctf_single struct-array-align-elem
211220
test_ctf_single meta-ctx-sequence
212221
test_ctf_single_version meta-clk-cls-before-trace-cls 2
213222
test_ctf_single_version def-clk-freq 1
223+
test_meta_trailing_byte
214224

215225
test_packet_end lttng-event-after-packet
216226
test_packet_end lttng-crash

0 commit comments

Comments
 (0)