[PATCH 5/8] Fixed potential cochran memory leak.

Changed id, config and misc data pointers to fixed arrays. Checked freed pointers before malloc to avoid leak. --- src/cochran_common.c | 225 +++++++++++++++++------------------------------ src/cochran_common.h | 14 +-- src/cochran_emc_parser.c | 4 +- 3 files changed, 89 insertions(+), 154 deletions(-)

diff --git a/src/cochran_common.c b/src/cochran_common.c index 7bbe277..22630c6 100644 --- a/src/cochran_common.c +++ b/src/cochran_common.c @@ -215,8 +215,6 @@ cochran_common_device_open (dc_device_t **out, dc_context_t *context, if (rc != DC_STATUS_SUCCESS) { ERROR (context, "Device not responding."); serial_close (device->port); - free (device->data.id); - free (device->data.id0); free (device); return rc; } @@ -225,7 +223,6 @@ cochran_common_device_open (dc_device_t **out, dc_context_t *context, if ((device->data.model & 0xFF0000) == COCHRAN_MODEL_UNKNOWN) { ERROR (context, "Device not recognized."); serial_close (device->port); - free (device->data.id); free (device); return DC_STATUS_UNSUPPORTED; } @@ -248,13 +245,6 @@ cochran_common_device_close (dc_device_t *abstract) }

// Free memory. - free (device->data.id); - free (device->data.id0); - free (device->data.config1); - free (device->data.config2); - free (device->data.config3); - free (device->data.config4); - free (device->data.misc); free (device->data.logbook); free (device->data.sample); free (device); @@ -411,29 +401,17 @@ cochran_read_id (dc_device_t *abstract) dc_status_t rc; unsigned char command[6] = {0x05, 0x9D, 0xFF, 0x00, 0x43, 0x00};

- device->data.id = (unsigned char *) malloc(67); - if (device->data.id == NULL) { - ERROR (abstract->context, "Failed to allocate memory."); - return DC_STATUS_NOMEMORY; - } - rc = cochran_packet(device, command, 6, device->data.id, 67, 0); if (rc != DC_STATUS_SUCCESS) return rc;

if (strncmp(device->data.id, "(C)", 3) != 0) { // It's a Commander, read again - device->data.id0 = device->data.id; + memcpy(device->data.id0, device->data.id, 67);

command[1] = 0xBD; command[2] = 0x7F;

- device->data.id = (unsigned char *) malloc(67); - if (device->data.id == NULL) { - ERROR (abstract->context, "Failed to allocate memory."); - return DC_STATUS_NOMEMORY; - } - rc = cochran_packet(device, command, 6, device->data.id, 67, 0); if (rc != DC_STATUS_SUCCESS) return rc; @@ -453,64 +431,20 @@ cochran_read_config (dc_device_t *abstract) dc_status_t rc; unsigned char command[2] = { 0x96, 0x00 };

- data->config1 = (unsigned char *) malloc(512); - if (data->config1 == NULL) { - ERROR (abstract->context, "Failed to allocate memory."); - return DC_STATUS_NOMEMORY; - } - - rc = cochran_packet(device, command, 2, data->config1, 512, 0); - if (rc != DC_STATUS_SUCCESS) - return rc; - - data->last_interdive_offset = - array_uint32_le (data->config1 + data->last_interdive_ptr); - data->last_entry_offset = - array_uint32_le (data->config1 + data->last_entry_ptr); - - // Read second page - command[1] = 0x01; - - data->config2 = (unsigned char *) malloc(512); - if (data->config2 == NULL) { - ERROR (abstract->context, "Failed to allocate memory."); - return DC_STATUS_NOMEMORY; - } - - rc = cochran_packet(device, command, 2, data->config2, 512, 0); - if (rc != DC_STATUS_SUCCESS) - return rc; - - // EMC only seems to have two config pages if ((data->model & 0xFF0000) == COCHRAN_MODEL_EMC_FAMILY) - return DC_STATUS_SUCCESS; - - // Go on reading the extra configs for a Commander - // Read third page - command[1] = 0x02; - - data->config3 = (unsigned char *) malloc(512); - if (data->config3 == NULL) { - ERROR (abstract->context, "Failed to allocate memory."); - return DC_STATUS_NOMEMORY; - } - - rc = cochran_packet(device, command, 2, data->config3, 512, 0); - if (rc != DC_STATUS_SUCCESS) - return rc; - - // Read fourth page - command[1] = 0x03; + data->config_count = 2; + else + data->config_count = 4;

- data->config4 = (unsigned char *) malloc(512); - if (data->config4 == NULL) { - ERROR (abstract->context, "Failed to allocate memory."); - return DC_STATUS_NOMEMORY; + int n; + for (n = 0; n < data->config_count; n++) { + command[1] = n; + rc = cochran_packet(device, command, 2, data->config[n], 512, 0); + if (rc != DC_STATUS_SUCCESS) + return rc; }

- rc = cochran_packet(device, command, 2, data->config4, 512, 0); - - return rc; + return DC_STATUS_SUCCESS; }

@@ -535,13 +469,7 @@ cochran_read_misc (dc_device_t *abstract) return DC_STATUS_UNSUPPORTED; }

- device->data.misc = (unsigned char *) malloc(1500); - if (device->data.misc == NULL) { - ERROR (abstract->context, "Failed to allocate memory."); - return DC_STATUS_NOMEMORY; - } - - // Send first byte then wait for heartbeat before sding the rest + // Send first byte then wait for heartbeat before sending the rest serial_write(device->port, command, 1);

int n; @@ -564,13 +492,16 @@ cochran_read_logbook (dc_device_t *abstract)

// Determine size of dive list to read. Round up to nearest 16K if (d->dive_count_endian == COCHRAN_LE_TYPE) - d->dive_count = array_uint16_le (d->config1 + d->dive_count_ptr); + d->dive_count = array_uint16_le (d->config[0] + d->dive_count_ptr); else - d->dive_count = array_uint16_be (d->config1 + d->dive_count_ptr); + d->dive_count = array_uint16_be (d->config[0] + d->dive_count_ptr); d->logbook_size = ((d->dive_count * d->log_size) & 0xFFFFC000) + 0x4000;

+ if (d->logbook) + free(d->logbook); + // Allocate space for log book. d->logbook = (unsigned char *) malloc(d->logbook_size); if (device == NULL) { @@ -684,6 +615,9 @@ cochran_read_samples(dc_device_t *abstract) cochran_read_parms(abstract, &low_offset, &high_offset);

if (d->sample_size > 0) { + if (d->sample) + free(d->sample); + d->sample = (unsigned char *) malloc(d->sample_size); if (device == NULL) { ERROR (abstract->context, "Failed to allocate memory."); @@ -763,116 +697,117 @@ cochran_common_device_dump (dc_device_t *abstract, dc_buffer_t *data) int ptr = 0;

dc_status_t rc; + char *b; + int size;

rc = cochran_common_device_read_all (abstract);

if (rc != DC_STATUS_SUCCESS) return rc;

- // Reserve space for pointers + // Reserve space for block pointers + // Structure is: + // int ptr; + // char data_valid_flag; dc_buffer_resize(data, 10 * 5); - pack_uint32_array_le(dc_buffer_get_data(data) + ptr, - dc_buffer_get_size(data)); + // Set pointer to first block + size = dc_buffer_get_size(data); + pack_uint32_array_le(dc_buffer_get_data(data) + ptr, size);

- if (d->id0) { + if (d->extra_id_flag) { dc_buffer_append(data, d->id0, 67); *(dc_buffer_get_data(data) + ptr + 4) = 1; } else { - dc_buffer_resize(data, dc_buffer_get_size(data) + 67); - } - - ptr += 5; - pack_uint32_array_le(dc_buffer_get_data(data) + ptr, - dc_buffer_get_size(data)); + dc_buffer_resize(data, size + 67); + b = dc_buffer_get_data(data);

- if (d->id) { - dc_buffer_append (data, d->id, 67); - *(dc_buffer_get_data(data) + ptr + 4) = 1; - } else { - dc_buffer_resize(data, dc_buffer_get_size(data) + 67); + memset(b + size, 0, 67); } + // Set pointer to next block ptr += 5; - pack_uint32_array_le(dc_buffer_get_data(data) + ptr, - dc_buffer_get_size(data)); + size = dc_buffer_get_size(data); + pack_uint32_array_le(dc_buffer_get_data(data) + ptr, size);

- if (d->config1) { - dc_buffer_append (data, d->config1, 512); - *(dc_buffer_get_data(data) + ptr + 4) = 1; - } else { - dc_buffer_resize(data, dc_buffer_get_size(data) + 512); - } + dc_buffer_append (data, d->id, 67); + *(dc_buffer_get_data(data) + ptr + 4) = 1; - ptr += 5; - pack_uint32_array_le(dc_buffer_get_data(data) + ptr, - dc_buffer_get_size(data)); + // Add config blocks + int n; + for (n = 0; n < d->config_count; n++) { + // Set pointer to next block + ptr += 5; + size = dc_buffer_get_size(data); + pack_uint32_array_le(dc_buffer_get_data(data) + ptr, size);

- if (d->config2) { - dc_buffer_append (data, d->config2, 512); - *(dc_buffer_get_data(data) + ptr + 4) = 1; - } else { - dc_buffer_resize(data, dc_buffer_get_size(data) + 512); - } - - ptr += 5; - pack_uint32_array_le(dc_buffer_get_data(data) + ptr, - dc_buffer_get_size(data)); + dc_buffer_append (data, d->config[n], 512);

- if (d->config3) { - dc_buffer_append (data, d->config3, 512); *(dc_buffer_get_data(data) + ptr + 4) = 1; - } else { - dc_buffer_resize(data, dc_buffer_get_size(data) + 512); } - - ptr += 5; - pack_uint32_array_le(dc_buffer_get_data(data) + ptr, - dc_buffer_get_size(data));

- if (d->config4) { - dc_buffer_append (data, d->config4, 512); - *(dc_buffer_get_data(data) + ptr + 4) = 1; - } else { - dc_buffer_resize(data, dc_buffer_get_size(data) + 512); + // Add blank config blocks + for (n = d->config_count; n < 4; n++) { + // Set pointer to next block + ptr += 5; + size = dc_buffer_get_size(data); + pack_uint32_array_le(dc_buffer_get_data(data) + ptr, size); + + dc_buffer_resize(data, size + 512); + + b = dc_buffer_get_data(data); + memset(b + size, 0, 512); } + // Set pointer to next block ptr += 5; - pack_uint32_array_le(dc_buffer_get_data(data) + ptr, - dc_buffer_get_size(data)); + size = dc_buffer_get_size(data); + pack_uint32_array_le(dc_buffer_get_data(data) + ptr, size);

if (d->misc) { dc_buffer_append (data, d->misc, 1500); *(dc_buffer_get_data(data) + ptr + 4) = 1; } else { - dc_buffer_resize(data, dc_buffer_get_size(data) + 1500); + dc_buffer_resize(data, size + 1500); + + b = dc_buffer_get_data(data); + memset(b + size, 0, 1500); } + // Set pointer to next block ptr += 5; - pack_uint32_array_le(dc_buffer_get_data(data) + ptr, - dc_buffer_get_size(data)); + size = dc_buffer_get_size(data); + pack_uint32_array_le(dc_buffer_get_data(data) + ptr, size);

if (d->logbook) { dc_buffer_append (data, d->logbook, d->logbook_size); *(dc_buffer_get_data(data) + ptr + 4) = 1; } else { - dc_buffer_resize(data, dc_buffer_get_size(data) + d->logbook_size); + dc_buffer_resize(data, size + d->logbook_size); + + b = dc_buffer_get_data(data); + memset(b + size, 0, d->logbook_size); } + // Set pointer to next block ptr += 5; - pack_uint32_array_le(dc_buffer_get_data(data) + ptr, - dc_buffer_get_size(data)); + size = dc_buffer_get_size(data); + pack_uint32_array_le(dc_buffer_get_data(data) + ptr, size);

if (d->sample) { dc_buffer_append (data, d->sample, d->sample_size); *(dc_buffer_get_data(data) + ptr + 4) = 1; } else { dc_buffer_resize(data, dc_buffer_get_size(data) + d->sample_size); + + b = dc_buffer_get_data(data); + memset(b + size, 0, d->logbook_size); }

+ // Set pointer to end ptr += 5; - pack_uint32_array_le(dc_buffer_get_data(data) + ptr, - dc_buffer_get_size(data)); + size = dc_buffer_get_size(data); + pack_uint32_array_le(dc_buffer_get_data(data) + ptr, size);

return DC_STATUS_SUCCESS; } diff --git a/src/cochran_common.h b/src/cochran_common.h index 7bba3fb..3498696 100644 --- a/src/cochran_common.h +++ b/src/cochran_common.h @@ -45,16 +45,16 @@ typedef enum cochran_model_t { typedef struct cochran_data_t { cochran_model_t model;

- unsigned char *id0; - unsigned char *id; - unsigned char *config1; - unsigned char *config2; - unsigned char *config3; - unsigned char *config4; - unsigned char *misc; + unsigned char id0[67]; + unsigned char id[67]; + unsigned char config[4][512]; + unsigned char misc[1500]; unsigned char *logbook; unsigned char *sample;

+ unsigned int extra_id_flag; + unsigned int config_count; + unsigned short int dive_count; unsigned char fingerprint[2]; int fp_dive_num; diff --git a/src/cochran_emc_parser.c b/src/cochran_emc_parser.c index 881f7c7..e1f28ac 100644 --- a/src/cochran_emc_parser.c +++ b/src/cochran_emc_parser.c @@ -134,9 +134,9 @@ cochran_emc_guess_sample_size(cochran_data_t *data) unsigned int offset, next_offset, memory_start_offset, memory_end_offset;

// Get memory boundries - memory_start_offset = array_uint32_le(data->config1 + memory_start_offset = array_uint32_le(data->config[0] + data->sample_memory_start_address); - memory_end_offset = array_uint32_le(data->config1 + memory_end_offset = array_uint32_le(data->config[0] + data->sample_memory_end_address);

// see if there is a next sample