[PATCH 5/8] Fixed potential cochran memory leak.
John Van Ostrand
john at vanostrand.com
Sun Nov 2 18:52:31 PST 2014
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
--
1.8.3.1
More information about the devel
mailing list