From bf12395af9cb4b3b899da4a99c1ea29dfefc7255 Mon Sep 17 00:00:00 2001 From: Stefan Wahren Date: Tue, 29 May 2012 21:24:30 +0200 Subject: [PATCH] Fixed segmentation fault in case of much XML output Because of the multi telegrams it is impossible to use a fixed buffer for XML output on the stack. Now the buffer is allocated on the heap. These also leads to a change in return behaviour for all XML functions, because it's possible that the buffer is NULL. For variable data this buffer grows every time there are less than 1024 bytes left for a new record. --- bin/mbus-serial-request-data-multi-reply.c | 2 + bin/mbus-serial-request-data.c | 13 +- mbus/mbus-protocol-aux.c | 42 ++++-- mbus/mbus-protocol.c | 146 ++++++++++++++------- test/mbus_parse.c | 11 +- test/mbus_parse_hex.c | 12 +- 6 files changed, 157 insertions(+), 69 deletions(-) diff --git a/bin/mbus-serial-request-data-multi-reply.c b/bin/mbus-serial-request-data-multi-reply.c index 35eb473..9dd1648 100644 --- a/bin/mbus-serial-request-data-multi-reply.c +++ b/bin/mbus-serial-request-data-multi-reply.c @@ -169,7 +169,9 @@ main(int argc, char **argv) fprintf(stderr, "Failed to generate XML representation of MBUS frames: %s\n", mbus_error_str()); return 1; } + printf("%s", xml_result); + free(xml_result); mbus_disconnect(handle); return 0; diff --git a/bin/mbus-serial-request-data.c b/bin/mbus-serial-request-data.c index 5816c3c..d60a777 100644 --- a/bin/mbus-serial-request-data.c +++ b/bin/mbus-serial-request-data.c @@ -29,7 +29,7 @@ main(int argc, char **argv) mbus_frame_data reply_data; mbus_handle *handle = NULL; - char *device, *addr_str; + char *device, *addr_str, *xml_result; int address, baudrate = 9600; if (argc == 3) @@ -145,8 +145,15 @@ main(int argc, char **argv) fprintf(stderr, "M-bus data parse error.\n"); return 1; } - - printf("%s", mbus_frame_data_xml(&reply_data)); + + if ((xml_result = mbus_frame_data_xml(&reply_data)) == NULL) + { + fprintf(stderr, "Failed to generate XML representation of MBUS frame: %s\n", mbus_error_str()); + return 1; + } + + printf("%s", xml_result); + free(xml_result); // manual free if (reply_data.data_var.record) diff --git a/mbus/mbus-protocol-aux.c b/mbus/mbus-protocol-aux.c index aa0cf39..2323bec 100644 --- a/mbus/mbus-protocol-aux.c +++ b/mbus/mbus-protocol-aux.c @@ -1214,44 +1214,58 @@ mbus_data_variable_xml_normalized(mbus_data_variable *data) { mbus_data_record *record; mbus_record *norm_record; - static char buff[8192]; + char *buff = NULL; char str_encoded[768]; - size_t len = 0; + size_t len = 0, buff_size = 8192; size_t i; if (data) { - len += snprintf(&buff[len], sizeof(buff) - len, "\n\n"); + buff = (char*) malloc(buff_size); - len += snprintf(&buff[len], sizeof(buff) - len, "%s", mbus_data_variable_header_xml(&(data->header))); + if (buff == NULL) + return NULL; + + len += snprintf(&buff[len], buff_size - len, "\n\n"); + + len += snprintf(&buff[len], buff_size - len, "%s", mbus_data_variable_header_xml(&(data->header))); for (record = data->record, i = 0; record; record = record->next, i++) { norm_record = mbus_parse_variable_record(record); - len += snprintf(&buff[len], sizeof(buff) - len, " \n", i); + if ((buff_size - len) < 1024) + { + buff_size *= 2; + buff = (char*) realloc(buff,buff_size); + + if (buff == NULL) + return NULL; + } + + len += snprintf(&buff[len], buff_size - len, " \n", i); if (norm_record != NULL) { mbus_str_xml_encode(str_encoded, norm_record->function_medium, sizeof(str_encoded)); - len += snprintf(&buff[len], sizeof(buff) - len, " %s\n", str_encoded); + len += snprintf(&buff[len], buff_size - len, " %s\n", str_encoded); mbus_str_xml_encode(str_encoded, norm_record->unit, sizeof(str_encoded)); - len += snprintf(&buff[len], sizeof(buff) - len, " %s\n", str_encoded); + len += snprintf(&buff[len], buff_size - len, " %s\n", str_encoded); mbus_str_xml_encode(str_encoded, norm_record->quantity, sizeof(str_encoded)); - len += snprintf(&buff[len], sizeof(buff) - len, " %s\n", str_encoded); + len += snprintf(&buff[len], buff_size - len, " %s\n", str_encoded); if (norm_record->is_numeric) { - len += snprintf(&buff[len], sizeof(buff) - len, " %f\n", norm_record->value.real_val); + len += snprintf(&buff[len], buff_size - len, " %f\n", norm_record->value.real_val); } else { mbus_str_xml_encode(str_encoded, norm_record->value.str_val.value, sizeof(str_encoded)); - len += snprintf(&buff[len], sizeof(buff) - len, " %s\n", str_encoded); + len += snprintf(&buff[len], buff_size - len, " %s\n", str_encoded); } mbus_record_free(norm_record); @@ -1260,15 +1274,15 @@ mbus_data_variable_xml_normalized(mbus_data_variable *data) { } - len += snprintf(&buff[len], sizeof(buff) - len, " \n\n"); + len += snprintf(&buff[len], buff_size - len, " \n\n"); } - len += snprintf(&buff[len], sizeof(buff) - len, "\n"); + len += snprintf(&buff[len], buff_size - len, "\n"); return buff; } - return ""; + return NULL; } //------------------------------------------------------------------------------ @@ -1290,7 +1304,7 @@ mbus_frame_data_xml_normalized(mbus_frame_data *data) } } - return ""; + return NULL; } mbus_handle * diff --git a/mbus/mbus-protocol.c b/mbus/mbus-protocol.c index bd7d38f..af5118b 100644 --- a/mbus/mbus-protocol.c +++ b/mbus/mbus-protocol.c @@ -3358,29 +3358,42 @@ char * mbus_data_variable_xml(mbus_data_variable *data) { mbus_data_record *record; - static char buff[8192]; - char str_encoded[768]; - size_t len = 0; + char *buff = NULL; + size_t len = 0, buff_size = 8192; int i; if (data) { - len += snprintf(&buff[len], sizeof(buff) - len, "\n\n"); + buff = (char*) malloc(buff_size); - len += snprintf(&buff[len], sizeof(buff) - len, "%s", + if (buff == NULL) + return NULL; + + len += snprintf(&buff[len], buff_size - len, "\n\n"); + + len += snprintf(&buff[len], buff_size - len, "%s", mbus_data_variable_header_xml(&(data->header))); for (record = data->record, i = 0; record; record = record->next, i++) { - len += snprintf(&buff[len], sizeof(buff) - len, "%s", + if ((buff_size - len) < 1024) + { + buff_size *= 2; + buff = (char*) realloc(buff,buff_size); + + if (buff == NULL) + return NULL; + } + + len += snprintf(&buff[len], buff_size - len, "%s", mbus_data_variable_record_xml(record, i, -1, &(data->header))); } - len += snprintf(&buff[len], sizeof(buff) - len, "\n"); + len += snprintf(&buff[len], buff_size - len, "\n"); return buff; } - return ""; + return NULL; } //------------------------------------------------------------------------------ @@ -3389,64 +3402,69 @@ mbus_data_variable_xml(mbus_data_variable *data) char * mbus_data_fixed_xml(mbus_data_fixed *data) { - static char buff[8192]; + char *buff = NULL; char str_encoded[256]; - size_t len = 0; + size_t len = 0, buff_size = 8192; if (data) { - len += snprintf(&buff[len], sizeof(buff) - len, "\n\n"); + buff = (char*) malloc(buff_size); + + if (buff == NULL) + return NULL; - len += snprintf(&buff[len], sizeof(buff) - len, " \n"); - len += snprintf(&buff[len], sizeof(buff) - len, " %d\n", (int)mbus_data_bcd_decode(data->id_bcd, 4)); + len += snprintf(&buff[len], buff_size - len, "\n\n"); + + len += snprintf(&buff[len], buff_size - len, " \n"); + len += snprintf(&buff[len], buff_size - len, " %d\n", (int)mbus_data_bcd_decode(data->id_bcd, 4)); mbus_str_xml_encode(str_encoded, mbus_data_fixed_medium(data), sizeof(str_encoded)); - len += snprintf(&buff[len], sizeof(buff) - len, " %s\n", str_encoded); + len += snprintf(&buff[len], buff_size - len, " %s\n", str_encoded); - len += snprintf(&buff[len], sizeof(buff) - len, " %d\n", data->tx_cnt); - len += snprintf(&buff[len], sizeof(buff) - len, " %.2X\n", data->status); - len += snprintf(&buff[len], sizeof(buff) - len, " \n\n"); + len += snprintf(&buff[len], buff_size - len, " %d\n", data->tx_cnt); + len += snprintf(&buff[len], buff_size - len, " %.2X\n", data->status); + len += snprintf(&buff[len], buff_size - len, " \n\n"); - len += snprintf(&buff[len], sizeof(buff) - len, " \n"); + len += snprintf(&buff[len], buff_size - len, " \n"); mbus_str_xml_encode(str_encoded, mbus_data_fixed_function(data->status), sizeof(str_encoded)); - len += snprintf(&buff[len], sizeof(buff) - len, " %s\n", str_encoded); + len += snprintf(&buff[len], buff_size - len, " %s\n", str_encoded); mbus_str_xml_encode(str_encoded, mbus_data_fixed_unit(data->cnt1_type), sizeof(str_encoded)); - len += snprintf(&buff[len], sizeof(buff) - len, " %s\n", str_encoded); + len += snprintf(&buff[len], buff_size - len, " %s\n", str_encoded); if ((data->status & MBUS_DATA_FIXED_STATUS_FORMAT_MASK) == MBUS_DATA_FIXED_STATUS_FORMAT_BCD) { - len += snprintf(&buff[len], sizeof(buff) - len, " %d\n", (int)mbus_data_bcd_decode(data->cnt1_val, 4)); + len += snprintf(&buff[len], buff_size - len, " %d\n", (int)mbus_data_bcd_decode(data->cnt1_val, 4)); } else { - len += snprintf(&buff[len], sizeof(buff) - len, " %d\n", mbus_data_int_decode(data->cnt1_val, 4)); + len += snprintf(&buff[len], buff_size - len, " %d\n", mbus_data_int_decode(data->cnt1_val, 4)); } - len += snprintf(&buff[len], sizeof(buff) - len, " \n\n"); + len += snprintf(&buff[len], buff_size - len, " \n\n"); - len += snprintf(&buff[len], sizeof(buff) - len, " \n"); + len += snprintf(&buff[len], buff_size - len, " \n"); mbus_str_xml_encode(str_encoded, mbus_data_fixed_function(data->status), sizeof(str_encoded)); - len += snprintf(&buff[len], sizeof(buff) - len, " %s\n", str_encoded); + len += snprintf(&buff[len], buff_size - len, " %s\n", str_encoded); mbus_str_xml_encode(str_encoded, mbus_data_fixed_unit(data->cnt2_type), sizeof(str_encoded)); - len += snprintf(&buff[len], sizeof(buff) - len, " %s\n", str_encoded); + len += snprintf(&buff[len], buff_size - len, " %s\n", str_encoded); if ((data->status & MBUS_DATA_FIXED_STATUS_FORMAT_MASK) == MBUS_DATA_FIXED_STATUS_FORMAT_BCD) { - len += snprintf(&buff[len], sizeof(buff) - len, " %d\n", (int)mbus_data_bcd_decode(data->cnt2_val, 4)); + len += snprintf(&buff[len], buff_size - len, " %d\n", (int)mbus_data_bcd_decode(data->cnt2_val, 4)); } else { - len += snprintf(&buff[len], sizeof(buff) - len, " %d\n", mbus_data_int_decode(data->cnt2_val, 4)); + len += snprintf(&buff[len], buff_size - len, " %d\n", mbus_data_int_decode(data->cnt2_val, 4)); } - len += snprintf(&buff[len], sizeof(buff) - len, " \n\n"); + len += snprintf(&buff[len], buff_size - len, " \n\n"); - len += snprintf(&buff[len], sizeof(buff) - len, "\n"); + len += snprintf(&buff[len], buff_size - len, "\n"); return buff; } - return ""; + return NULL; } //------------------------------------------------------------------------------ @@ -3455,20 +3473,25 @@ mbus_data_fixed_xml(mbus_data_fixed *data) char * mbus_data_error_xml(int error) { - static char buff[512]; + char *buff = NULL; char str_encoded[256]; - size_t len = 0; - - len += snprintf(&buff[len], sizeof(buff) - len, "\n\n"); + size_t len = 0, buff_size = 8192; - len += snprintf(&buff[len], sizeof(buff) - len, " \n"); + buff = (char*) malloc(buff_size); + + if (buff == NULL) + return NULL; + + len += snprintf(&buff[len], buff_size - len, "\n\n"); + + len += snprintf(&buff[len], buff_size - len, " \n"); mbus_str_xml_encode(str_encoded, mbus_data_error_lookup(error), sizeof(str_encoded)); - len += snprintf(&buff[len], sizeof(buff) - len, " %s\n", str_encoded); + len += snprintf(&buff[len], buff_size - len, " %s\n", str_encoded); - len += snprintf(&buff[len], sizeof(buff) - len, " \n\n"); + len += snprintf(&buff[len], buff_size - len, " \n\n"); - len += snprintf(&buff[len], sizeof(buff) - len, "\n"); + len += snprintf(&buff[len], buff_size - len, "\n"); return buff; } @@ -3497,7 +3520,7 @@ mbus_frame_data_xml(mbus_frame_data *data) } } - return ""; + return NULL; } @@ -3511,9 +3534,9 @@ mbus_frame_xml(mbus_frame *frame) mbus_frame *iter; mbus_data_record *record; - static char buff[8192]; + char *buff = NULL; - size_t len = 0; + size_t len = 0, buff_size = 8192; int record_cnt = 0, frame_cnt; if (frame) @@ -3544,25 +3567,39 @@ mbus_frame_xml(mbus_frame *frame) { // // generate XML for a sequence of variable data frames - // + // + + buff = (char*) malloc(buff_size); + + if (buff == NULL) + return NULL; // include frame counter in XML output if more than one frame // is available (frame_cnt = -1 => not included in output) frame_cnt = (frame->next == NULL) ? -1 : 0; - len += snprintf(&buff[len], sizeof(buff) - len, "\n\n"); + len += snprintf(&buff[len], buff_size - len, "\n\n"); // only print the header info for the first frame (should be // the same for each frame in a sequence of a multi-telegram // transfer. - len += snprintf(&buff[len], sizeof(buff) - len, "%s", + len += snprintf(&buff[len], buff_size - len, "%s", mbus_data_variable_header_xml(&(frame_data.data_var.header))); // loop through all records in the current frame, using a global // record count as record ID in the XML output for (record = frame_data.data_var.record; record; record = record->next, record_cnt++) { - len += snprintf(&buff[len], sizeof(buff) - len, "%s", + if ((buff_size - len) < 1024) + { + buff_size *= 2; + buff = (char*) realloc(buff,buff_size); + + if (buff == NULL) + return NULL; + } + + len += snprintf(&buff[len], buff_size - len, "%s", mbus_data_variable_record_xml(record, record_cnt, frame_cnt, &(frame_data.data_var.header))); } @@ -3586,7 +3623,16 @@ mbus_frame_xml(mbus_frame *frame) // record count as record ID in the XML output for (record = frame_data.data_var.record; record; record = record->next, record_cnt++) { - len += snprintf(&buff[len], sizeof(buff) - len, "%s", + if ((buff_size - len) < 1024) + { + buff_size *= 2; + buff = (char*) realloc(buff,buff_size); + + if (buff == NULL) + return NULL; + } + + len += snprintf(&buff[len], buff_size - len, "%s", mbus_data_variable_record_xml(record, record_cnt, frame_cnt, &(frame_data.data_var.header))); } @@ -3595,15 +3641,15 @@ mbus_frame_xml(mbus_frame *frame) { mbus_data_record_free(frame_data.data_var.record); } - } + } - len += snprintf(&buff[len], sizeof(buff) - len, "\n"); + len += snprintf(&buff[len], buff_size - len, "\n"); return buff; } } - return ""; + return NULL; } diff --git a/test/mbus_parse.c b/test/mbus_parse.c index 69499b2..f55eb5e 100644 --- a/test/mbus_parse.c +++ b/test/mbus_parse.c @@ -28,6 +28,7 @@ main(int argc, char *argv[]) u_char buf[1024]; mbus_frame reply; mbus_frame_data frame_data; + char *xml_result = NULL; if (argc != 2) { @@ -51,7 +52,15 @@ main(int argc, char *argv[]) mbus_parse(&reply, buf, len); mbus_frame_data_parse(&reply, &frame_data); mbus_frame_print(&reply); - printf("%s", mbus_frame_data_xml(&frame_data)); + + if ((xml_result = mbus_frame_data_xml(&frame_data)) == NULL) + { + fprintf(stderr, "Failed to generate XML representation of MBUS frame: %s\n", mbus_error_str()); + return 1; + } + printf("%s", xml_result); + free(xml_result); + return 0; } diff --git a/test/mbus_parse_hex.c b/test/mbus_parse_hex.c index 41b9278..75f819d 100644 --- a/test/mbus_parse_hex.c +++ b/test/mbus_parse_hex.c @@ -28,6 +28,7 @@ main(int argc, char *argv[]) u_char raw_buff[4096], buff[4096], *ptr, *endptr; mbus_frame reply; mbus_frame_data frame_data; + char *xml_result = NULL; if (argc != 2) { @@ -89,7 +90,16 @@ main(int argc, char *argv[]) //mbus_frame_print(&reply); //mbus_frame_data_print(&frame_data); - printf("%s", mbus_frame_data_xml(&frame_data)); + + if ((xml_result = mbus_frame_data_xml(&frame_data)) == NULL) + { + fprintf(stderr, "Failed to generate XML representation of MBUS frame: %s\n", mbus_error_str()); + return 1; + } + printf("%s", xml_result); + free(xml_result); + + return 0; }