Package: release.debian.org
Severity: normal
Tags: buster
User: release.debian....@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 <cy...@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 <cy...@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 <gr...@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 <da...@gibson.dropbear.id.au>
+Signed-off-by: Greg Kurz <gr...@kaod.org>
+Signed-off-by: David Gibson <da...@gibson.dropbear.id.au>
+(cherry picked from commit 9619c8619c37b9aea98100bcc15c51a5642e877e)
+Signed-off-by: Cyril Brulebois <cy...@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
