[PATCH] Fix buffer overrun and primary sensor id issues in Liquivision import

Robert Bodily robert at bodily.com
Sun Jun 11 16:05:33 PDT 2017


Sorry, I'm new here and not entirely sure of the proper way to do this so
I'm copying the contents of the patch file here and also attaching it to
this email.

This changeset fixes 5 issues specific to importing from Liquivision dive
logs:

Issue #1: Buffer overrun causes segmentation fault.
At the end of a dive record, untranslatable data is skipped and the file is
scanned for the start of the next dive.  This scan was implemented without
regard to buffer size and so the scan ran over the buffer boundary when
trying
to scan for the next record after importing the last record in the file.

Issue #2: Incorrect identification of the primary sensor.
The primary tank pressure transmitter was being identified by using the
sensor
ID reported in the first pressure event record encountered.  When diving
with
multiple transmitters (buddy, student, or group transmitters), this is often
not the case and results in the buddy or other group transmitter's pressure
data being imported instead of the primary's.

Through empirical observation of several multi-sensor logs, I identified a
previously unhandled event code (0x10) as marking a sensor identification
event record.  Parsing this record allows the primary and other sensors
to be definitively identified regardless of which one sends the first
pressure
event.

Issue #3: Sensor values added to the sample collection regardless of sensor
ID.
When processing events, the code previously dropped through to create a
sample
for every pressure event record, regardless of which sensor ID that event is
associated with.  Pressure events for sensors other than the primary are now
ignored and omitted from the sample collection.

Issue #4: Duplicate samples when pressure event time syncs with sample time.
The sample index (d) was not incremented in this specific case resulting in
a duplicate sample (for the same sample time) being created when processing
the next pressure event record.

Issue #5: Unsigned time difference results in erroneous interpolated
samples.
When interpolating/extrapolating depth and temperature values for a between-
samples pressure event, a signed time value is subtracted from an unsigned
time
value, resulting in an unsigned term.  This term is used as a scaling factor
and should be signed to allow for a negative value.  Currently, negative
values
are instead treated as large unsigned values which result in erroneous
scaled
depth and temperature values.

Signed-off-by: Robert Bodily <robert at bodily.com>
---
 core/liquivision.c | 77
++++++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 57 insertions(+), 20 deletions(-)

diff --git a/core/liquivision.c b/core/liquivision.c
index fb2b444..e4e5c5f 100644
--- a/core/liquivision.c
+++ b/core/liquivision.c
@@ -21,7 +21,18 @@ struct lv_event {
 	} pressure;
 };
 
-uint16_t primary_sensor;
+// Liquivision supports the following sensor configurations:
+// Primary sensor only
+// Primary + Buddy sensor
+// Primary + Up to 4 additional sensors
+// Primary + Up to 9 addiitonal sensors
+struct lv_sensor_ids {
+	uint16_t primary;
+	uint16_t buddy;
+	uint16_t group[9];
+};
+
+struct lv_sensor_ids sensor_ids;
 
 static int handle_event_ver2(int code, const unsigned char *ps, unsigned
int ps_ptr, struct lv_event *event)
 {
@@ -61,28 +72,52 @@ static int handle_event_ver3(int code, const unsigned
char *ps, unsigned int ps_
 	case 0x000f:
 		// Tank pressure
 		event->time = array_uint32_le(ps + ps_ptr);
+		current_sensor = array_uint16_le(ps + ps_ptr + 4);
 
-		/* As far as I know, Liquivision supports 2 sensors, own and
buddie's. This is my
-		 * best guess how it is represented. */
+		event->pressure.sensor = -1;
+		event->pressure.mbar = array_uint16_le(ps + ps_ptr + 6) *
10; // cb->mb
 
-		current_sensor = array_uint16_le(ps + ps_ptr + 4);
-		if (primary_sensor == 0) {
-			primary_sensor = current_sensor;
-		}
-		if (current_sensor == primary_sensor) {
+		if (current_sensor == sensor_ids.primary) {
 			event->pressure.sensor = 0;
-			event->pressure.mbar = array_uint16_le(ps + ps_ptr +
6) * 10; // cb->mb
-		} else {
-			/* Ignoring the buddy sensor for no as we cannot
draw it on the profile.
+		} else if (current_sensor == sensor_ids.buddy) {
 			event->pressure.sensor = 1;
-			event->pressure.mbar = array_uint16_le(ps + ps_ptr +
6) * 10; // cb->mb
-			*/
+		} else {
+			int i;
+			for (i = 0; i < 9; ++i) {
+				if (current_sensor == sensor_ids.group[i]) {
+					event->pressure.sensor = i + 2;
+					break;
+				}
+			}
 		}
+
 		// 1 byte PSR
 		// 1 byte ST
 		skip = 10;
 		break;
 	case 0x0010:
+		// 4 byte time
+		// 2 byte primary transmitter S/N
+		// 2 byte buddy transmitter S/N
+		// 2 byte group transmitter S/N (9x)
+
+		// I don't think it's possible to change sensor IDs once a
dive has started but disallow it here just in case
+		if (sensor_ids.primary == 0) {
+			sensor_ids.primary = array_uint16_le(ps + ps_ptr +
4);
+		}
+
+		if (sensor_ids.buddy == 0) {
+			sensor_ids.buddy = array_uint16_le(ps + ps_ptr + 6);
+		}
+
+		int i;
+		const unsigned char *group_ptr = ps + ps_ptr + 8;
+		for (i = 0; i < 9; ++i, group_ptr += 2) {
+			if (sensor_ids.group[i] == 0) {
+				sensor_ids.group[i] =
array_uint16_le(group_ptr);
+			}
+		}
+
 		skip = 26;
 		break;
 	case 0x0015:	// Unknown
@@ -109,7 +144,7 @@ static void parse_dives (int log_version, const unsigned
char *buf, unsigned int
 		int i;
 		bool found_divesite = false;
 		dive = alloc_dive();
-		primary_sensor = 0;
+		memset(&sensor_ids, 0, sizeof(sensor_ids));
 		dc = &dive->dc;
 
 		/* Just the main cylinder until we can handle the buddy
cylinder porperly */
@@ -239,7 +274,7 @@ static void parse_dives (int log_version, const unsigned
char *buf, unsigned int
 			ptr += 4;
 			dive->otu = lrintf(*(float *) (buf + ptr));
 			ptr += 4;
-			dive_mode = *(buf + ptr++);	// 0=Deco, 1=Gauge,
2=None
+			dive_mode = *(buf + ptr++);	// 0=Deco, 1=Gauge,
2=None, 35=Rec
 			algorithm = *(buf + ptr++);	// 0=ZH-L16C+GF
 			sample_count = array_uint32_le(buf + ptr);
 		}
@@ -275,8 +310,9 @@ static void parse_dives (int log_version, const unsigned
char *buf, unsigned int
 
 			if (log_version == 3) {
 				ps_ptr += handle_event_ver3(event_code, ps,
ps_ptr, &event);
-				if (event_code != 0xf)
-					continue;	// ignore all by
pressure sensor event
+				// Ignoring the buddy sensor for now as we
cannot draw it on the profile.
+				if ((event_code != 0xf) ||
(event.pressure.sensor != 0))
+					continue;	// ignore all but
pressure sensor event
 			} else {	// version 2
 				ps_ptr += handle_event_ver2(event_code, ps,
ps_ptr, &event);
 				continue;		// ignore all events
@@ -319,6 +355,7 @@ static void parse_dives (int log_version, const unsigned
char *buf, unsigned int
 					sample->sensor =
event.pressure.sensor;
 					sample->cylinderpressure.mbar =
event.pressure.mbar;
 					finish_sample(dc);
+					d++;
 
 					break;
 				} else {	// Event is prior to sample
@@ -333,9 +370,9 @@ static void parse_dives (int log_version, const unsigned
char *buf, unsigned int
 						last_depth =
array_uint16_le(ds + (d - 1) * 2) * 10; // cm->mm
 						last_temp =
C_to_mkelvin((float) array_uint16_le(ts + (d - 1) * 2) / 10); // dC->mK
 						sample->depth.mm =
last_depth + (depth_mm - last_depth)
-							* (event.time -
last_time) / sample_interval;
+							* ((int)event.time -
last_time) / sample_interval;
 						sample->temperature.mkelvin
= last_temp + (temp_mk - last_temp)
-							* (event.time -
last_time) / sample_interval;
+							* ((int)event.time -
last_time) / sample_interval;
 					}
 					finish_sample(dc);
 
@@ -375,7 +412,7 @@ static void parse_dives (int log_version, const unsigned
char *buf, unsigned int
 				break;
 			}
 
-			while (*(ps + ps_ptr) != 0x04)
+			while (((ptr + ps_ptr + 4) < buf_size) && (*(ps +
ps_ptr) != 0x04))
 				ps_ptr++;
 		}
 
-- 
2.10.2.windows.1

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Fix-buffer-overrun-and-primary-sensor-id-issues-in-L.patch
Type: application/octet-stream
Size: 7938 bytes
Desc: not available
URL: <http://lists.subsurface-divelog.org/pipermail/subsurface/attachments/20170611/00ccd97c/attachment-0001.obj>


More information about the subsurface mailing list