[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#981339: marked as done (buster-pu: package device-tree-compiler/1.4.7-4)



Your message dated Sat, 06 Feb 2021 10:39:26 +0000
with message-id <6425525e38201ecf9a2d3e0f1e63c0d3b08e0fc0.camel@adam-barratt.org.uk>
and subject line Closing p-u bugs for updates in 10.8
has caused the Debian Bug report #981339,
regarding buster-pu: package device-tree-compiler/1.4.7-4
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
Bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact owner@bugs.debian.org
immediately.)


-- 
981339: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=981339
Debian Bug Tracking System
Contact owner@bugs.debian.org with problems
--- Begin Message ---
Package: release.debian.org
Severity: normal
Tags: buster
User: release.debian.org@packages.debian.org
Usertags: pu

Hi,

I'd like to propose a fix for a dtc segfault in buster, which can
prevent people from checking what their device-tree looks like,
making it harder to debug why device-tree overlays don't work:
  https://bugs.debian.org/981033

Case in point, I was following this upstream doc and thought maybe
I should be using the in-tree (linux.git) dtc executable instead,
which was a red herring:
  https://www.raspberrypi.org/documentation/configuration/device-tree.md

As Héctor pointed out, there's an unaffected package available in
buster-backports, but I thought it would be worth it to fix the
package in buster proper as well, and Héctor is fine with my
handling that.

I've tested the patched package successfully, in a buster/arm64
environment (Pi3-like environments).

Changelog entry (there was no -4 upload to the archive):

    device-tree-compiler (1.4.7-4) buster; urgency=medium
    
      * Fix segfault on “dtc -I fs /proc/device-tree” by backporting
        9619c8619c, first released in 1.5.0 (Closes: #981033). With huge
        thanks to Uwe Kleine-König for the debugging and general guidance:
         - 03-Kill-bogus-TYPE_BLOB-marker-type.patch
      * Adjust gbp configuration for the buster branch.
    
     -- Cyril Brulebois <cyril@debamax.com>  Wed, 27 Jan 2021 19:53:55 +0100

Full source debdiff attached.

Thanks for your time!


Cheers,
-- 
Cyril Brulebois -- Debian Consultant @ DEBAMAX -- https://debamax.com/
diff -Nru device-tree-compiler-1.4.7/debian/changelog device-tree-compiler-1.4.7/debian/changelog
--- device-tree-compiler-1.4.7/debian/changelog	2018-09-11 09:51:07.000000000 +0200
+++ device-tree-compiler-1.4.7/debian/changelog	2021-01-27 19:53:55.000000000 +0100
@@ -1,3 +1,13 @@
+device-tree-compiler (1.4.7-4) buster; urgency=medium
+
+  * Fix segfault on “dtc -I fs /proc/device-tree” by backporting
+    9619c8619c, first released in 1.5.0 (Closes: #981033). With huge
+    thanks to Uwe Kleine-König for the debugging and general guidance:
+     - 03-Kill-bogus-TYPE_BLOB-marker-type.patch
+  * Adjust gbp configuration for the buster branch.
+
+ -- Cyril Brulebois <cyril@debamax.com>  Wed, 27 Jan 2021 19:53:55 +0100
+
 device-tree-compiler (1.4.7-3) unstable; urgency=medium
 
   * Add Build-Depends on pkg-config, which is used to check for valgrind.
diff -Nru device-tree-compiler-1.4.7/debian/gbp.conf device-tree-compiler-1.4.7/debian/gbp.conf
--- device-tree-compiler-1.4.7/debian/gbp.conf	2018-09-11 07:47:55.000000000 +0200
+++ device-tree-compiler-1.4.7/debian/gbp.conf	2021-01-27 19:50:38.000000000 +0100
@@ -1,6 +1,6 @@
 [DEFAULT]
 pristine-tar = True
 debian-tag = debian/%(version)s
-debian-branch = debian/master
+debian-branch = debian/buster
 upstream-branch = upstream/latest
 patch-numbers = False
diff -Nru device-tree-compiler-1.4.7/debian/patches/03-Kill-bogus-TYPE_BLOB-marker-type.patch device-tree-compiler-1.4.7/debian/patches/03-Kill-bogus-TYPE_BLOB-marker-type.patch
--- device-tree-compiler-1.4.7/debian/patches/03-Kill-bogus-TYPE_BLOB-marker-type.patch	1970-01-01 01:00:00.000000000 +0100
+++ device-tree-compiler-1.4.7/debian/patches/03-Kill-bogus-TYPE_BLOB-marker-type.patch	2021-01-27 19:49:45.000000000 +0100
@@ -0,0 +1,139 @@
+From ec8c8cd0fd71d33da07d388d391e6211bef5d757 Mon Sep 17 00:00:00 2001
+From: Greg Kurz <groug@kaod.org>
+Date: Thu, 30 Aug 2018 12:01:59 +0200
+Subject: [PATCH] Kill bogus TYPE_BLOB marker type
+
+Since commit 32b9c6130762 "Preserve datatype markers when emitting dts
+format", we no longer try to guess the value type. Instead, we reuse
+the type of the datatype markers when they are present, if the type
+is either TYPE_UINT* or TYPE_STRING.
+
+This causes 'dtc -I fs' to crash:
+
+Starting program: /root/dtc -q -f -O dts -I fs /proc/device-tree
+/dts-v1/;
+
+/ {
+
+Program received signal SIGSEGV, Segmentation fault.
+__strlen_power8 () at ../sysdeps/powerpc/powerpc64/power8/strlen.S:47
+47              ld      r12,0(r4)     /* Load doubleword from memory.  */
+(gdb) bt
+#0  __strlen_power8 () at ../sysdeps/powerpc/powerpc64/power8/strlen.S:47
+#1  0x00007ffff7de3d10 in __GI__IO_fputs (str=<optimized out>,
+    fp=<optimized out>) at iofputs.c:33
+#2  0x000000001000c7a0 in write_propval (prop=0x100525e0,
+    f=0x7ffff7f718a0 <_IO_2_1_stdout_>) at treesource.c:245
+
+The offending line is:
+
+                fprintf(f, "%s", delim_start[emit_type]);
+
+where emit_type is TYPE_BLOB and:
+
+static const char *delim_start[] = {
+        [TYPE_UINT8] = "[",
+        [TYPE_UINT16] = "/bits/ 16 <",
+        [TYPE_UINT32] = "<",
+        [TYPE_UINT64] = "/bits/ 64 <",
+        [TYPE_STRING] = "",
+};
+
+/* Data blobs */
+enum markertype {
+        TYPE_NONE,
+        REF_PHANDLE,
+        REF_PATH,
+        LABEL,
+        TYPE_UINT8,
+        TYPE_UINT16,
+        TYPE_UINT32,
+        TYPE_UINT64,
+        TYPE_BLOB,
+        TYPE_STRING,
+};
+
+Because TYPE_BLOB < TYPE_STRING and delim_start[] is a static array,
+delim_start[emit_type] is 0x0. The glibc usually prints out "(null)"
+when one passes 0x0 to %s, but it seems to call fputs() internally if
+the format is exactly "%s", hence the crash.
+
+TYPE_BLOB basically means the data comes from a file and we don't know
+its type. We don't care for the former, and the latter is TYPE_NONE.
+
+So let's drop TYPE_BLOB completely and use TYPE_NONE instead when reading
+the file. Then, try to guess the data type at emission time, like the
+code already does for refs and labels.
+
+Instead of adding yet another check for TYPE_NONE, an helper is introduced
+to check if the data marker has type information, ie, >= TYPE_UINT8.
+
+Fixes: 32b9c61307629ac76c6ac0bead6f926d579b3d2c
+Suggested-by: David Gibson <david@gibson.dropbear.id.au>
+Signed-off-by: Greg Kurz <groug@kaod.org>
+Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
+(cherry picked from commit 9619c8619c37b9aea98100bcc15c51a5642e877e)
+Signed-off-by: Cyril Brulebois <cyril@debamax.com>
+---
+ data.c       | 2 +-
+ dtc.h        | 1 -
+ treesource.c | 9 +++++++--
+ 3 files changed, 8 insertions(+), 4 deletions(-)
+
+diff --git a/data.c b/data.c
+index accdfae..4a20414 100644
+--- a/data.c
++++ b/data.c
+@@ -95,7 +95,7 @@ struct data data_copy_file(FILE *f, size_t maxlen)
+ {
+ 	struct data d = empty_data;
+ 
+-	d = data_add_marker(d, TYPE_BLOB, NULL);
++	d = data_add_marker(d, TYPE_NONE, NULL);
+ 	while (!feof(f) && (d.len < maxlen)) {
+ 		size_t chunksize, ret;
+ 
+diff --git a/dtc.h b/dtc.h
+index 303c2a6..51c03ef 100644
+--- a/dtc.h
++++ b/dtc.h
+@@ -82,7 +82,6 @@ enum markertype {
+ 	TYPE_UINT16,
+ 	TYPE_UINT32,
+ 	TYPE_UINT64,
+-	TYPE_BLOB,
+ 	TYPE_STRING,
+ };
+ extern const char *markername(enum markertype markertype);
+diff --git a/treesource.c b/treesource.c
+index f99544d..53e6203 100644
+--- a/treesource.c
++++ b/treesource.c
+@@ -133,9 +133,14 @@ static void write_propval_int(FILE *f, const char *p, size_t len, size_t width)
+ 	}
+ }
+ 
++static bool has_data_type_information(struct marker *m)
++{
++	return m->type >= TYPE_UINT8;
++}
++
+ static struct marker *next_type_marker(struct marker *m)
+ {
+-	while (m && (m->type == LABEL || m->type == REF_PHANDLE || m->type == REF_PATH))
++	while (m && !has_data_type_information(m))
+ 		m = m->next;
+ 	return m;
+ }
+@@ -225,7 +230,7 @@ static void write_propval(FILE *f, struct property *prop)
+ 		size_t chunk_len;
+ 		const char *p = &prop->val.val[m->offset];
+ 
+-		if (m->type < TYPE_UINT8)
++		if (!has_data_type_information(m))
+ 			continue;
+ 
+ 		chunk_len = type_marker_length(m);
+-- 
+2.11.0
+
diff -Nru device-tree-compiler-1.4.7/debian/patches/series device-tree-compiler-1.4.7/debian/patches/series
--- device-tree-compiler-1.4.7/debian/patches/series	2018-09-11 07:49:08.000000000 +0200
+++ device-tree-compiler-1.4.7/debian/patches/series	2021-01-27 19:43:01.000000000 +0100
@@ -1,2 +1,3 @@
 01_build_doc.patch
 02-Make-valgrind-optional.patch
+03-Kill-bogus-TYPE_BLOB-marker-type.patch

--- End Message ---
--- Begin Message ---
Package: release.debian.org
Version: 10.8

Hi,

Each of the updates referenced by these bugs was included in today's
10.8 point release.

Regards,

Adam

--- End Message ---

Reply to: