From: Philippe Proulx Date: Thu, 20 Jun 2019 19:54:48 +0000 (-0400) Subject: Fix: `ctf` plugin: do not have an `mdec` variable where not strictly needed X-Git-Url: http://git.efficios.com/?p=babeltrace.git;a=commitdiff_plain;h=3c8252a5b96d617770863d3f54398b58d4c3315a Fix: `ctf` plugin: do not have an `mdec` variable where not strictly needed Issue ===== The `metadata-info` query needs to call ctf_metadata_decoder_packetized_file_stream_to_buf() with a custom log level and no self component (`NULL`). Eventually, this reaches decode_packet() which is in the `decoder.c` file and thus uses `mdec->self_comp` to find its self component in the BT_COMP_LOG*() macros. But in this case, `mdec` is `NULL`, so there's an invalid memory access. Solution ======== Instead of using BT_COMP_LOG_CUR_LVL() everywhere in this function, and because decode_packet() and ctf_metadata_decoder_packetized_file_stream_to_buf_with_mdec() do not strictly need `mdec` except for logging its address at some places, move the functions to the new `decoder-packetized-file-stream-to-buf.c` file and make them not accept an `mdec` argument. This new file's logging uses the local `log_level` and `self_comp` variables, so the logging statement are not modified. Known drawbacks =============== None. Signed-off-by: Philippe Proulx Change-Id: I55874790b2b873ff1718a774e57011300e57518d Reviewed-on: https://review.lttng.org/c/babeltrace/+/1526 Tested-by: jenkins --- diff --git a/src/plugins/ctf/common/metadata/Makefile.am b/src/plugins/ctf/common/metadata/Makefile.am index fa5b6f8e..0de44051 100644 --- a/src/plugins/ctf/common/metadata/Makefile.am +++ b/src/plugins/ctf/common/metadata/Makefile.am @@ -22,6 +22,7 @@ libctf_ast_la_SOURCES = \ scanner.h \ scanner-symbols.h \ decoder.c \ + decoder-packetized-file-stream-to-buf.c \ decoder.h \ logging.c \ logging.h \ diff --git a/src/plugins/ctf/common/metadata/decoder-packetized-file-stream-to-buf.c b/src/plugins/ctf/common/metadata/decoder-packetized-file-stream-to-buf.c new file mode 100644 index 00000000..29986fab --- /dev/null +++ b/src/plugins/ctf/common/metadata/decoder-packetized-file-stream-to-buf.c @@ -0,0 +1,307 @@ +/* + * Copyright 2016-2017 - Philippe Proulx + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + */ + +#define BT_COMP_LOG_SELF_COMP self_comp +#define BT_LOG_OUTPUT_LEVEL log_level +#define BT_LOG_TAG "PLUGIN/CTF/META/DECODER-DECODE-PACKET" +#include "plugins/comp-logging.h" + +#include +#include +#include +#include +#include +#include "common/assert.h" +#include "compat/uuid.h" +#include "compat/memstream.h" +#include +#include +#include + +#include "ast.h" +#include "decoder.h" +#include "scanner.h" +#include "logging.h" + +#define TSDL_MAGIC 0x75d11d57 + +extern +int yydebug; + +struct ctf_metadata_decoder { + struct ctf_visitor_generate_ir *visitor; + uint8_t uuid[16]; + bool is_uuid_set; + int bo; + struct ctf_metadata_decoder_config config; +}; + +struct packet_header { + uint32_t magic; + uint8_t uuid[16]; + uint32_t checksum; + uint32_t content_size; + uint32_t packet_size; + uint8_t compression_scheme; + uint8_t encryption_scheme; + uint8_t checksum_scheme; + uint8_t major; + uint8_t minor; +} __attribute__((__packed__)); + +static +int decode_packet(FILE *in_fp, FILE *out_fp, + int byte_order, bool *is_uuid_set, uint8_t *uuid, + bt_logging_level log_level, bt_self_component *self_comp) +{ + struct packet_header header; + size_t readlen, writelen, toread; + uint8_t buf[512 + 1]; /* + 1 for debug-mode \0 */ + int ret = 0; + const long offset = ftell(in_fp); + + if (offset < 0) { + BT_COMP_LOGE_ERRNO("Failed to get current metadata file position", + "."); + goto error; + } + BT_COMP_LOGD("Decoding metadata packet: offset=%ld", offset); + readlen = fread(&header, sizeof(header), 1, in_fp); + if (feof(in_fp) != 0) { + BT_COMP_LOGI("Reached end of file: offset=%ld", ftell(in_fp)); + goto end; + } + if (readlen < 1) { + BT_COMP_LOGE("Cannot decode metadata packet: offset=%ld", offset); + goto error; + } + + if (byte_order != BYTE_ORDER) { + header.magic = GUINT32_SWAP_LE_BE(header.magic); + header.checksum = GUINT32_SWAP_LE_BE(header.checksum); + header.content_size = GUINT32_SWAP_LE_BE(header.content_size); + header.packet_size = GUINT32_SWAP_LE_BE(header.packet_size); + } + + if (header.compression_scheme) { + BT_COMP_LOGE("Metadata packet compression is not supported as of this version: " + "compression-scheme=%u, offset=%ld", + (unsigned int) header.compression_scheme, offset); + goto error; + } + + if (header.encryption_scheme) { + BT_COMP_LOGE("Metadata packet encryption is not supported as of this version: " + "encryption-scheme=%u, offset=%ld", + (unsigned int) header.encryption_scheme, offset); + goto error; + } + + if (header.checksum || header.checksum_scheme) { + BT_COMP_LOGE("Metadata packet checksum verification is not supported as of this version: " + "checksum-scheme=%u, checksum=%x, offset=%ld", + (unsigned int) header.checksum_scheme, header.checksum, + offset); + goto error; + } + + if (!ctf_metadata_decoder_is_packet_version_valid(header.major, + header.minor)) { + BT_COMP_LOGE("Invalid metadata packet version: " + "version=%u.%u, offset=%ld", + header.major, header.minor, offset); + goto error; + } + + /* Set expected trace UUID if not set; otherwise validate it */ + if (is_uuid_set) { + if (!*is_uuid_set) { + memcpy(uuid, header.uuid, sizeof(header.uuid)); + *is_uuid_set = true; + } else if (bt_uuid_compare(header.uuid, uuid)) { + BT_COMP_LOGE("Metadata UUID mismatch between packets of the same stream: " + "packet-uuid=\"%02x%02x%02x%02x-%02x%02x-%02x%02x-%02x%02x-%02x%02x%02x%02x%02x%02x\", " + "expected-uuid=\"%02x%02x%02x%02x-%02x%02x-%02x%02x-%02x%02x-%02x%02x%02x%02x%02x%02x\", " + "offset=%ld", + (unsigned int) header.uuid[0], + (unsigned int) header.uuid[1], + (unsigned int) header.uuid[2], + (unsigned int) header.uuid[3], + (unsigned int) header.uuid[4], + (unsigned int) header.uuid[5], + (unsigned int) header.uuid[6], + (unsigned int) header.uuid[7], + (unsigned int) header.uuid[8], + (unsigned int) header.uuid[9], + (unsigned int) header.uuid[10], + (unsigned int) header.uuid[11], + (unsigned int) header.uuid[12], + (unsigned int) header.uuid[13], + (unsigned int) header.uuid[14], + (unsigned int) header.uuid[15], + (unsigned int) uuid[0], + (unsigned int) uuid[1], + (unsigned int) uuid[2], + (unsigned int) uuid[3], + (unsigned int) uuid[4], + (unsigned int) uuid[5], + (unsigned int) uuid[6], + (unsigned int) uuid[7], + (unsigned int) uuid[8], + (unsigned int) uuid[9], + (unsigned int) uuid[10], + (unsigned int) uuid[11], + (unsigned int) uuid[12], + (unsigned int) uuid[13], + (unsigned int) uuid[14], + (unsigned int) uuid[15], + offset); + goto error; + } + } + + if ((header.content_size / CHAR_BIT) < sizeof(header)) { + BT_COMP_LOGE("Bad metadata packet content size: content-size=%u, " + "offset=%ld", header.content_size, offset); + goto error; + } + + toread = header.content_size / CHAR_BIT - sizeof(header); + + for (;;) { + size_t loop_read; + + loop_read = MIN(sizeof(buf) - 1, toread); + readlen = fread(buf, sizeof(uint8_t), loop_read, in_fp); + if (ferror(in_fp)) { + BT_COMP_LOGE("Cannot read metadata packet buffer: " + "offset=%ld, read-size=%zu", + ftell(in_fp), loop_read); + goto error; + } + if (readlen > loop_read) { + BT_COMP_LOGE("fread returned more byte than expected: " + "read-size-asked=%zu, read-size-returned=%zu", + loop_read, readlen); + goto error; + } + + writelen = fwrite(buf, sizeof(uint8_t), readlen, out_fp); + if (writelen < readlen || ferror(out_fp)) { + BT_COMP_LOGE("Cannot write decoded metadata text to buffer: " + "read-offset=%ld, write-size=%zu", + ftell(in_fp), readlen); + goto error; + } + + toread -= readlen; + if (toread == 0) { + int fseek_ret; + + /* Read leftover padding */ + toread = (header.packet_size - header.content_size) / + CHAR_BIT; + fseek_ret = fseek(in_fp, toread, SEEK_CUR); + if (fseek_ret < 0) { + BT_COMP_LOGW_STR("Missing padding at the end of the metadata stream."); + } + break; + } + } + + goto end; + +error: + ret = -1; + +end: + return ret; +} + +BT_HIDDEN +int ctf_metadata_decoder_packetized_file_stream_to_buf(FILE *fp, + char **buf, int byte_order, bool *is_uuid_set, + uint8_t *uuid, bt_logging_level log_level, + bt_self_component *self_comp) +{ + FILE *out_fp; + size_t size; + int ret = 0; + int tret; + size_t packet_index = 0; + + out_fp = bt_open_memstream(buf, &size); + if (out_fp == NULL) { + BT_COMP_LOGE("Cannot open memory stream: %s.", + strerror(errno)); + goto error; + } + + for (;;) { + if (feof(fp) != 0) { + break; + } + + tret = decode_packet(fp, out_fp, byte_order, is_uuid_set, + uuid, log_level, self_comp); + if (tret) { + BT_COMP_LOGE("Cannot decode packet: index=%zu", + packet_index); + goto error; + } + + packet_index++; + } + + /* Make sure the whole string ends with a null character */ + tret = fputc('\0', out_fp); + if (tret == EOF) { + BT_COMP_LOGE_STR( + "Cannot append '\\0' to the decoded metadata buffer."); + goto error; + } + + /* Close stream, which also flushes the buffer */ + ret = bt_close_memstream(buf, &size, out_fp); + /* + * See fclose(3). Further access to out_fp after both success + * and error, even through another bt_close_memstream(), results + * in undefined behavior. Nullify out_fp to ensure we don't + * fclose it twice on error. + */ + out_fp = NULL; + if (ret < 0) { + BT_COMP_LOGE_ERRNO("Cannot close memory stream", "."); + goto error; + } + + goto end; + +error: + ret = -1; + + if (out_fp) { + if (bt_close_memstream(buf, &size, out_fp)) { + BT_COMP_LOGE_ERRNO("Cannot close memory stream", "."); + } + } + + if (*buf) { + free(*buf); + *buf = NULL; + } + +end: + return ret; +} diff --git a/src/plugins/ctf/common/metadata/decoder.c b/src/plugins/ctf/common/metadata/decoder.c index fdaf54bf..a690ca71 100644 --- a/src/plugins/ctf/common/metadata/decoder.c +++ b/src/plugins/ctf/common/metadata/decoder.c @@ -92,272 +92,6 @@ end: return ret; } -static -bool is_version_valid(unsigned int major, unsigned int minor) -{ - return major == 1 && minor == 8; -} - -static -int decode_packet(struct ctf_metadata_decoder *mdec, FILE *in_fp, FILE *out_fp, - int byte_order) -{ - struct packet_header header; - size_t readlen, writelen, toread; - uint8_t buf[512 + 1]; /* + 1 for debug-mode \0 */ - int ret = 0; - const long offset = ftell(in_fp); - - if (offset < 0) { - BT_COMP_LOGE_ERRNO("Failed to get current metadata file position", - "."); - goto error; - } - BT_COMP_LOGD("Decoding metadata packet: mdec-addr=%p, offset=%ld", - mdec, offset); - readlen = fread(&header, sizeof(header), 1, in_fp); - if (feof(in_fp) != 0) { - BT_COMP_LOGI("Reached end of file: offset=%ld", ftell(in_fp)); - goto end; - } - if (readlen < 1) { - BT_COMP_LOGE("Cannot decode metadata packet: offset=%ld", offset); - goto error; - } - - if (byte_order != BYTE_ORDER) { - header.magic = GUINT32_SWAP_LE_BE(header.magic); - header.checksum = GUINT32_SWAP_LE_BE(header.checksum); - header.content_size = GUINT32_SWAP_LE_BE(header.content_size); - header.packet_size = GUINT32_SWAP_LE_BE(header.packet_size); - } - - if (header.compression_scheme) { - BT_COMP_LOGE("Metadata packet compression is not supported as of this version: " - "compression-scheme=%u, offset=%ld", - (unsigned int) header.compression_scheme, offset); - goto error; - } - - if (header.encryption_scheme) { - BT_COMP_LOGE("Metadata packet encryption is not supported as of this version: " - "encryption-scheme=%u, offset=%ld", - (unsigned int) header.encryption_scheme, offset); - goto error; - } - - if (header.checksum || header.checksum_scheme) { - BT_COMP_LOGE("Metadata packet checksum verification is not supported as of this version: " - "checksum-scheme=%u, checksum=%x, offset=%ld", - (unsigned int) header.checksum_scheme, header.checksum, - offset); - goto error; - } - - if (!is_version_valid(header.major, header.minor)) { - BT_COMP_LOGE("Invalid metadata packet version: " - "version=%u.%u, offset=%ld", - header.major, header.minor, offset); - goto error; - } - - /* Set expected trace UUID if not set; otherwise validate it */ - if (mdec) { - if (!mdec->is_uuid_set) { - memcpy(mdec->uuid, header.uuid, sizeof(header.uuid)); - mdec->is_uuid_set = true; - } else if (bt_uuid_compare(header.uuid, mdec->uuid)) { - BT_COMP_LOGE("Metadata UUID mismatch between packets of the same stream: " - "packet-uuid=\"%02x%02x%02x%02x-%02x%02x-%02x%02x-%02x%02x-%02x%02x%02x%02x%02x%02x\", " - "expected-uuid=\"%02x%02x%02x%02x-%02x%02x-%02x%02x-%02x%02x-%02x%02x%02x%02x%02x%02x\", " - "offset=%ld", - (unsigned int) header.uuid[0], - (unsigned int) header.uuid[1], - (unsigned int) header.uuid[2], - (unsigned int) header.uuid[3], - (unsigned int) header.uuid[4], - (unsigned int) header.uuid[5], - (unsigned int) header.uuid[6], - (unsigned int) header.uuid[7], - (unsigned int) header.uuid[8], - (unsigned int) header.uuid[9], - (unsigned int) header.uuid[10], - (unsigned int) header.uuid[11], - (unsigned int) header.uuid[12], - (unsigned int) header.uuid[13], - (unsigned int) header.uuid[14], - (unsigned int) header.uuid[15], - (unsigned int) mdec->uuid[0], - (unsigned int) mdec->uuid[1], - (unsigned int) mdec->uuid[2], - (unsigned int) mdec->uuid[3], - (unsigned int) mdec->uuid[4], - (unsigned int) mdec->uuid[5], - (unsigned int) mdec->uuid[6], - (unsigned int) mdec->uuid[7], - (unsigned int) mdec->uuid[8], - (unsigned int) mdec->uuid[9], - (unsigned int) mdec->uuid[10], - (unsigned int) mdec->uuid[11], - (unsigned int) mdec->uuid[12], - (unsigned int) mdec->uuid[13], - (unsigned int) mdec->uuid[14], - (unsigned int) mdec->uuid[15], - offset); - goto error; - } - } - - if ((header.content_size / CHAR_BIT) < sizeof(header)) { - BT_COMP_LOGE("Bad metadata packet content size: content-size=%u, " - "offset=%ld", header.content_size, offset); - goto error; - } - - toread = header.content_size / CHAR_BIT - sizeof(header); - - for (;;) { - size_t loop_read; - - loop_read = MIN(sizeof(buf) - 1, toread); - readlen = fread(buf, sizeof(uint8_t), loop_read, in_fp); - if (ferror(in_fp)) { - BT_COMP_LOGE("Cannot read metadata packet buffer: " - "offset=%ld, read-size=%zu", - ftell(in_fp), loop_read); - goto error; - } - if (readlen > loop_read) { - BT_COMP_LOGE("fread returned more byte than expected: " - "read-size-asked=%zu, read-size-returned=%zu", - loop_read, readlen); - goto error; - } - - writelen = fwrite(buf, sizeof(uint8_t), readlen, out_fp); - if (writelen < readlen || ferror(out_fp)) { - BT_COMP_LOGE("Cannot write decoded metadata text to buffer: " - "read-offset=%ld, write-size=%zu", - ftell(in_fp), readlen); - goto error; - } - - toread -= readlen; - if (toread == 0) { - int fseek_ret; - - /* Read leftover padding */ - toread = (header.packet_size - header.content_size) / - CHAR_BIT; - fseek_ret = fseek(in_fp, toread, SEEK_CUR); - if (fseek_ret < 0) { - BT_COMP_LOGW_STR("Missing padding at the end of the metadata stream."); - } - break; - } - } - - goto end; - -error: - ret = -1; - -end: - return ret; -} - -static -int ctf_metadata_decoder_packetized_file_stream_to_buf_with_mdec( - struct ctf_metadata_decoder *mdec, FILE *fp, - char **buf, int byte_order, bt_logging_level log_level, - bt_self_component *self_comp) -{ - FILE *out_fp; - size_t size; - int ret = 0; - int tret; - size_t packet_index = 0; - - out_fp = bt_open_memstream(buf, &size); - if (out_fp == NULL) { - BT_COMP_LOG_CUR_LVL(BT_LOG_ERROR, log_level, self_comp, - "Cannot open memory stream: %s: mdec-addr=%p", - strerror(errno), mdec); - goto error; - } - - for (;;) { - if (feof(fp) != 0) { - break; - } - - tret = decode_packet(mdec, fp, out_fp, byte_order); - if (tret) { - BT_COMP_LOG_CUR_LVL(BT_LOG_ERROR, log_level, self_comp, - "Cannot decode packet: index=%zu, mdec-addr=%p", - packet_index, mdec); - goto error; - } - - packet_index++; - } - - /* Make sure the whole string ends with a null character */ - tret = fputc('\0', out_fp); - if (tret == EOF) { - BT_COMP_LOG_CUR_LVL(BT_LOG_ERROR, log_level, self_comp, - "Cannot append '\\0' to the decoded metadata buffer: " - "mdec-addr=%p", mdec); - goto error; - } - - /* Close stream, which also flushes the buffer */ - ret = bt_close_memstream(buf, &size, out_fp); - /* - * See fclose(3). Further access to out_fp after both success - * and error, even through another bt_close_memstream(), results - * in undefined behavior. Nullify out_fp to ensure we don't - * fclose it twice on error. - */ - out_fp = NULL; - if (ret < 0) { - BT_COMP_LOG_ERRNO_CUR_LVL(BT_LOG_ERROR, log_level, self_comp, - "Cannot close memory stream", ": mdec-addr=%p", mdec); - goto error; - } - - goto end; - -error: - ret = -1; - - if (out_fp) { - if (bt_close_memstream(buf, &size, out_fp)) { - BT_COMP_LOG_ERRNO_CUR_LVL(BT_LOG_ERROR, log_level, - self_comp, "Cannot close memory stream", - ": mdec-addr=%p", mdec); - } - } - - if (*buf) { - free(*buf); - *buf = NULL; - } - -end: - return ret; -} - -BT_HIDDEN -int ctf_metadata_decoder_packetized_file_stream_to_buf( - FILE *fp, char **buf, int byte_order, - bt_logging_level log_level, - bt_self_component *self_comp) -{ - return ctf_metadata_decoder_packetized_file_stream_to_buf_with_mdec( - NULL, fp, buf, byte_order, log_level, self_comp); -} - BT_HIDDEN struct ctf_metadata_decoder *ctf_metadata_decoder_create( const struct ctf_metadata_decoder_config *config) @@ -430,8 +164,9 @@ enum ctf_metadata_decoder_status ctf_metadata_decoder_decode( if (ctf_metadata_decoder_is_packetized(fp, &mdec->bo, mdec->config.log_level, mdec->config.self_comp)) { BT_COMP_LOGI("Metadata stream is packetized: mdec-addr=%p", mdec); - ret = ctf_metadata_decoder_packetized_file_stream_to_buf_with_mdec( - mdec, fp, &buf, mdec->bo, mdec->config.log_level, + ret = ctf_metadata_decoder_packetized_file_stream_to_buf(fp, + &buf, mdec->bo, &mdec->is_uuid_set, + mdec->uuid, mdec->config.log_level, mdec->config.self_comp); if (ret) { BT_COMP_LOGE("Cannot decode packetized metadata packets to metadata text: " @@ -476,7 +211,8 @@ enum ctf_metadata_decoder_status ctf_metadata_decoder_decode( BT_COMP_LOGI("Found metadata stream version in signature: version=%u.%u", major, minor); - if (!is_version_valid(major, minor)) { + if (!ctf_metadata_decoder_is_packet_version_valid(major, + minor)) { BT_COMP_LOGE("Invalid metadata version found in plain text signature: " "version=%u.%u, mdec-addr=%p", major, minor, mdec); diff --git a/src/plugins/ctf/common/metadata/decoder.h b/src/plugins/ctf/common/metadata/decoder.h index e40356e3..42e79bf2 100644 --- a/src/plugins/ctf/common/metadata/decoder.h +++ b/src/plugins/ctf/common/metadata/decoder.h @@ -115,9 +115,16 @@ bool ctf_metadata_decoder_is_packetized(FILE *fp, int *byte_order, * text buffer using the given byte order. */ BT_HIDDEN -int ctf_metadata_decoder_packetized_file_stream_to_buf( - FILE *fp, char **buf, int byte_order, - bt_logging_level log_level, +int ctf_metadata_decoder_packetized_file_stream_to_buf(FILE *fp, + char **buf, int byte_order, bool *is_uuid_set, + uint8_t *uuid, bt_logging_level log_level, bt_self_component *self_comp); +static inline +bool ctf_metadata_decoder_is_packet_version_valid(unsigned int major, + unsigned int minor) +{ + return major == 1 && minor == 8; +} + #endif /* _METADATA_DECODER_H */ diff --git a/src/plugins/ctf/fs-src/query.c b/src/plugins/ctf/fs-src/query.c index a32f1bbd..8e91ba36 100644 --- a/src/plugins/ctf/fs-src/query.c +++ b/src/plugins/ctf/fs-src/query.c @@ -104,7 +104,8 @@ bt_query_status metadata_info_query( if (is_packetized) { ret = ctf_metadata_decoder_packetized_file_stream_to_buf( - metadata_fp, &metadata_text, bo, log_level, NULL); + metadata_fp, &metadata_text, bo, NULL, NULL, + log_level, NULL); if (ret) { BT_LOGE("Cannot decode packetized metadata file: path=\"%s\"", path);