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

Bug#977031: device-tree-compiler: Please backport patch to fix unaligned access on sparc64



Source: device-tree-compiler
Severity: normal
Tags: patch
User: debian-sparc@lists.debian.org
Usertags: sparc64
X-Debbugs-Cc: debian-sparc@lists.debian.org

Hi!

device-tree-compiler currently FTBFS on sparc64 due to unaligned access crashes.

Luckily, that problem has been fixed upstream already:

commit b28464a550c536296439b5785ed8852d1e15b35b
Author: David Gibson <david@gibson.dropbear.id.au>
Date:   Tue Apr 14 15:02:51 2020 +1000

    Fix some potential unaligned accesses in dtc
    
    Because of the convention of packed representations in property layouts,
    it's not uncommon to have integer values in properties which aren't
    naturally aligned.  Thus, there are several places in the dtc code where we
    cast a potentially unaligned byte pointer into an integer pointer and load
    it directly.  On a number of architectures (including sparc64 and arm) this
    won't work and will cause a fault.  In some cases it may be trapped and
    emulated by the kernel, but not always.
    
    Therefore, replace such direct unaligned reads with a helper which will
    handle unaligned data reads (a variant on the fdtXX_ld() functions already
    used in libfdt).
    
    Signed-off-by: David Gibson <david@gibson.dropbear.id.au>

Could you backport this patch which applies cleanly to 1.6.0? I'm attaching it.

Thanks,
Adrian

--
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaubitz@debian.org
`. `'   Freie Universitaet Berlin - glaubitz@physik.fu-berlin.de
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913
>From b28464a550c536296439b5785ed8852d1e15b35b Mon Sep 17 00:00:00 2001
From: David Gibson <david@gibson.dropbear.id.au>
Date: Tue, 14 Apr 2020 15:02:51 +1000
Subject: [PATCH] Fix some potential unaligned accesses in dtc

Because of the convention of packed representations in property layouts,
it's not uncommon to have integer values in properties which aren't
naturally aligned.  Thus, there are several places in the dtc code where we
cast a potentially unaligned byte pointer into an integer pointer and load
it directly.  On a number of architectures (including sparc64 and arm) this
won't work and will cause a fault.  In some cases it may be trapped and
emulated by the kernel, but not always.

Therefore, replace such direct unaligned reads with a helper which will
handle unaligned data reads (a variant on the fdtXX_ld() functions already
used in libfdt).

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 dtc.h        | 31 +++++++++++++++++++++++++++++++
 flattree.c   |  2 +-
 treesource.c |  6 +++---
 yamltree.c   |  6 +++---
 4 files changed, 38 insertions(+), 7 deletions(-)

diff --git a/dtc.h b/dtc.h
index 6e74ece..a08f415 100644
--- a/dtc.h
+++ b/dtc.h
@@ -51,6 +51,37 @@ extern int annotate;		/* annotate .dts with input source location */
 
 typedef uint32_t cell_t;
 
+static inline uint16_t dtb_ld16(const void *p)
+{
+	const uint8_t *bp = (const uint8_t *)p;
+
+	return ((uint16_t)bp[0] << 8)
+		| bp[1];
+}
+
+static inline uint32_t dtb_ld32(const void *p)
+{
+	const uint8_t *bp = (const uint8_t *)p;
+
+	return ((uint32_t)bp[0] << 24)
+		| ((uint32_t)bp[1] << 16)
+		| ((uint32_t)bp[2] << 8)
+		| bp[3];
+}
+
+static inline uint64_t dtb_ld64(const void *p)
+{
+	const uint8_t *bp = (const uint8_t *)p;
+
+	return ((uint64_t)bp[0] << 56)
+		| ((uint64_t)bp[1] << 48)
+		| ((uint64_t)bp[2] << 40)
+		| ((uint64_t)bp[3] << 32)
+		| ((uint64_t)bp[4] << 24)
+		| ((uint64_t)bp[5] << 16)
+		| ((uint64_t)bp[6] << 8)
+		| bp[7];
+}
 
 #define streq(a, b)	(strcmp((a), (b)) == 0)
 #define strstarts(s, prefix)	(strncmp((s), (prefix), strlen(prefix)) == 0)
diff --git a/flattree.c b/flattree.c
index bd6977e..07f10d2 100644
--- a/flattree.c
+++ b/flattree.c
@@ -156,7 +156,7 @@ static void asm_emit_data(void *e, struct data d)
 		emit_offset_label(f, m->ref, m->offset);
 
 	while ((d.len - off) >= sizeof(uint32_t)) {
-		asm_emit_cell(e, fdt32_to_cpu(*((fdt32_t *)(d.val+off))));
+		asm_emit_cell(e, dtb_ld32(d.val + off));
 		off += sizeof(uint32_t);
 	}
 
diff --git a/treesource.c b/treesource.c
index c9d980c..2acb920 100644
--- a/treesource.c
+++ b/treesource.c
@@ -110,13 +110,13 @@ static void write_propval_int(FILE *f, const char *p, size_t len, size_t width)
 			fprintf(f, "%02"PRIx8, *(const uint8_t*)p);
 			break;
 		case 2:
-			fprintf(f, "0x%02"PRIx16, fdt16_to_cpu(*(const fdt16_t*)p));
+			fprintf(f, "0x%02"PRIx16, dtb_ld16(p));
 			break;
 		case 4:
-			fprintf(f, "0x%02"PRIx32, fdt32_to_cpu(*(const fdt32_t*)p));
+			fprintf(f, "0x%02"PRIx32, dtb_ld32(p));
 			break;
 		case 8:
-			fprintf(f, "0x%02"PRIx64, fdt64_to_cpu(*(const fdt64_t*)p));
+			fprintf(f, "0x%02"PRIx64, dtb_ld64(p));
 			break;
 		}
 		if (p + width < end)
diff --git a/yamltree.c b/yamltree.c
index 5b6ea8e..4e93c12 100644
--- a/yamltree.c
+++ b/yamltree.c
@@ -59,10 +59,10 @@ static void yaml_propval_int(yaml_emitter_t *emitter, struct marker *markers, ch
 			sprintf(buf, "0x%"PRIx8, *(uint8_t*)(data + off));
 			break;
 		case 2:
-			sprintf(buf, "0x%"PRIx16, fdt16_to_cpu(*(fdt16_t*)(data + off)));
+			sprintf(buf, "0x%"PRIx16, dtb_ld16(data + off));
 			break;
 		case 4:
-			sprintf(buf, "0x%"PRIx32, fdt32_to_cpu(*(fdt32_t*)(data + off)));
+			sprintf(buf, "0x%"PRIx32, dtb_ld32(data + off));
 			m = markers;
 			is_phandle = false;
 			for_each_marker_of_type(m, REF_PHANDLE) {
@@ -73,7 +73,7 @@ static void yaml_propval_int(yaml_emitter_t *emitter, struct marker *markers, ch
 			}
 			break;
 		case 8:
-			sprintf(buf, "0x%"PRIx64, fdt64_to_cpu(*(fdt64_t*)(data + off)));
+			sprintf(buf, "0x%"PRIx64, dtb_ld64(data + off));
 			break;
 		}
 
-- 
2.29.2


Reply to: