UDDF crash

Linus Torvalds torvalds at linux-foundation.org
Mon Nov 2 18:03:01 PST 2015


On Mon, Nov 2, 2015 at 4:27 PM, Lubomir I. Ivanov <neolit123 at gmail.com> wrote:
>
> here is a zip of the user file:
> https://dl.dropboxusercontent.com/u/1627980/subsurface/DiveOrganizer_2015-11-02T10.40.23.uddf.zip

So valgrind does catch something. This looks fairly interesting:

==19647== Invalid write of size 8
==19647==    at 0x6BE4F9: utf8_string (parse-xml.c:554)
==19647==    by 0x6BD7C9: match (parse-xml.c:111)
==19647==    by 0x6C1A2B: try_to_fill_dive (parse-xml.c:1405)
==19647==    by 0x6C2C3B: entry (parse-xml.c:1792)
==19647==    by 0x6C2E3E: visit_one_node (parse-xml.c:1853)
==19647==    by 0x6C2EB0: visit (parse-xml.c:1871)
==19647==    by 0x6C308B: traverse (parse-xml.c:1962)
==19647==    by 0x6C2E6E: traverse_properties (parse-xml.c:1864)
==19647==    by 0x6C2EC0: visit (parse-xml.c:1871)
==19647==    by 0x6C308B: traverse (parse-xml.c:1962)
==19647==    by 0x6C2ED4: visit (parse-xml.c:1871)
==19647==    by 0x6C308B: traverse (parse-xml.c:1962)
==19647==  Address 0x946f9530 is 16 bytes after a block of size 912 alloc'd
==19647==    at 0x4C28C50: malloc (in
/usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==19647==    by 0x6A3F66: alloc_dive (dive.c:371)
==19647==    by 0x6C24FC: dive_start (parse-xml.c:1582)
==19647==    by 0x6C32CD: parse_xml_buffer (parse-xml.c:2026)
==19647==    by 0x6B06D9: parse_file_buffer (file.c:428)
==19647==    by 0x6B09AB: parse_file (file.c:490)
==19647==    by 0x51A6D1: MainWindow::loadFiles(QStringList)
(mainwindow.cpp:1605)
==19647==    by 0x4EBBE1: main (main.cpp:80)

and that parse-xml.c:1405 is:

        if (MATCH("description.cylinder", utf8_string,
&dive->cylinder[cur_cylinder_index].type.description))
                return;

so what I *think* happens is that "cur_cylinder_index" has overflowed.

So something like the attached might be the right thing.  Not tested.

                          Linus
-------------- next part --------------
 parse-xml.c | 56 ++++++++++++++++++++++++++++++--------------------------
 1 file changed, 30 insertions(+), 26 deletions(-)

diff --git a/parse-xml.c b/parse-xml.c
index 3d86222b9c55..93ce743055fe 100644
--- a/parse-xml.c
+++ b/parse-xml.c
@@ -1398,32 +1398,36 @@ static void try_to_fill_dive(struct dive *dive, const char *name, char *buf)
 		return;
 	if (MATCH("visibility.dive", get_rating, &dive->visibility))
 		return;
-	if (MATCH("size.cylinder", cylindersize, &dive->cylinder[cur_cylinder_index].type.size))
-		return;
-	if (MATCH("workpressure.cylinder", pressure, &dive->cylinder[cur_cylinder_index].type.workingpressure))
-		return;
-	if (MATCH("description.cylinder", utf8_string, &dive->cylinder[cur_cylinder_index].type.description))
-		return;
-	if (MATCH("start.cylinder", pressure, &dive->cylinder[cur_cylinder_index].start))
-		return;
-	if (MATCH("end.cylinder", pressure, &dive->cylinder[cur_cylinder_index].end))
-		return;
-	if (MATCH("use.cylinder", cylinder_use, &dive->cylinder[cur_cylinder_index].cylinder_use))
-		return;
-	if (MATCH("description.weightsystem", utf8_string, &dive->weightsystem[cur_ws_index].description))
-		return;
-	if (MATCH("weight.weightsystem", weight, &dive->weightsystem[cur_ws_index].weight))
-		return;
-	if (MATCH("weight", weight, &dive->weightsystem[cur_ws_index].weight))
-		return;
-	if (MATCH("o2", gasmix, &dive->cylinder[cur_cylinder_index].gasmix.o2))
-		return;
-	if (MATCH("o2percent", gasmix, &dive->cylinder[cur_cylinder_index].gasmix.o2))
-		return;
-	if (MATCH("n2", gasmix_nitrogen, &dive->cylinder[cur_cylinder_index].gasmix))
-		return;
-	if (MATCH("he", gasmix, &dive->cylinder[cur_cylinder_index].gasmix.he))
-		return;
+	if (cur_ws_index < MAX_WEIGHTSYSTEMS) {
+		if (MATCH("description.weightsystem", utf8_string, &dive->weightsystem[cur_ws_index].description))
+			return;
+		if (MATCH("weight.weightsystem", weight, &dive->weightsystem[cur_ws_index].weight))
+			return;
+		if (MATCH("weight", weight, &dive->weightsystem[cur_ws_index].weight))
+			return;
+	}
+	if (cur_cylinder_index < MAX_CYLINDERS) {
+		if (MATCH("size.cylinder", cylindersize, &dive->cylinder[cur_cylinder_index].type.size))
+			return;
+		if (MATCH("workpressure.cylinder", pressure, &dive->cylinder[cur_cylinder_index].type.workingpressure))
+			return;
+		if (MATCH("description.cylinder", utf8_string, &dive->cylinder[cur_cylinder_index].type.description))
+			return;
+		if (MATCH("start.cylinder", pressure, &dive->cylinder[cur_cylinder_index].start))
+			return;
+		if (MATCH("end.cylinder", pressure, &dive->cylinder[cur_cylinder_index].end))
+			return;
+		if (MATCH("use.cylinder", cylinder_use, &dive->cylinder[cur_cylinder_index].cylinder_use))
+			return;
+		if (MATCH("o2", gasmix, &dive->cylinder[cur_cylinder_index].gasmix.o2))
+			return;
+		if (MATCH("o2percent", gasmix, &dive->cylinder[cur_cylinder_index].gasmix.o2))
+			return;
+		if (MATCH("n2", gasmix_nitrogen, &dive->cylinder[cur_cylinder_index].gasmix))
+			return;
+		if (MATCH("he", gasmix, &dive->cylinder[cur_cylinder_index].gasmix.he))
+			return;
+	}
 	if (MATCH("air.divetemperature", temperature, &dive->airtemp))
 		return;
 	if (MATCH("water.divetemperature", temperature, &dive->watertemp))


More information about the subsurface mailing list