[PATCH] libfdt: Fix signedness comparison warnings

[PATCH] libfdt: Fix signedness comparison warnings

[Andre Przywara]

This is a combination of upstream libfdt commits to fix warnings about
comparing signed and unsigned integers:
==========
scripts/dtc/libfdt/fdt.c: In function ?fdt_offset_ptr?:
scripts/dtc/libfdt/fdt.c:137:18: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
   if ((absoffset < offset)
...
==========

For a detailed description of the fixes, see the dtc repo:
https://git.kernel.org/pub/scm/utils/dtc/dtc.git/log/?id=73e0f143b73d808

For this patch the commits between 73e0f143b73d8088 and ca19c3db2bf62000
have been combined and adjusted for the slight differences in U-Boot's
libfdt code base.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 scripts/dtc/libfdt/fdt.c          | 15 +++++++++++----
 scripts/dtc/libfdt/fdt_overlay.c  |  3 ++-
 scripts/dtc/libfdt/fdt_ro.c       | 20 +++++++++++---------
 scripts/dtc/libfdt/fdt_strerror.c |  4 ++--
 scripts/dtc/libfdt/fdt_sw.c       | 27 +++++++++++++++------------
 scripts/dtc/libfdt/fdt_wip.c      |  2 +-
 6 files changed, 42 insertions(+), 29 deletions(-)

diff --git a/scripts/dtc/libfdt/fdt.c b/scripts/dtc/libfdt/fdt.c
index 8e4cce3b9be..28f4e1a5f15 100644
--- a/scripts/dtc/libfdt/fdt.c
+++ b/scripts/dtc/libfdt/fdt.c
@@ -131,16 +131,20 @@ int fdt_check_header(const void *fdt)
 
 const void *fdt_offset_ptr(const void *fdt, int offset, unsigned int len)
 {
-	unsigned absoffset = offset + fdt_off_dt_struct(fdt);
+	unsigned int uoffset = offset;
+	unsigned int absoffset = offset + fdt_off_dt_struct(fdt);
+
+	if (offset < 0)
+		return NULL;
 
 	if (fdt_chk_basic())
-		if ((absoffset < offset)
+		if ((absoffset < uoffset)
 		    || ((absoffset + len) < absoffset)
 		    || (absoffset + len) > fdt_totalsize(fdt))
 			return NULL;
 
 	if (!fdt_chk_version() || fdt_version(fdt) >= 0x11)
-		if (((offset + len) < offset)
+		if (((uoffset + len) < uoffset)
 		    || ((offset + len) > fdt_size_dt_struct(fdt)))
 			return NULL;
 
@@ -302,9 +306,12 @@ const char *fdt_find_string_(const char *strtab, int tabsize, const char *s)
 
 int fdt_move(const void *fdt, void *buf, int bufsize)
 {
+	if (fdt_chk_basic() && bufsize < 0)
+		return -FDT_ERR_NOSPACE;
+
 	FDT_RO_PROBE(fdt);
 
-	if (fdt_totalsize(fdt) > bufsize)
+	if (fdt_totalsize(fdt) > (unsigned int)bufsize)
 		return -FDT_ERR_NOSPACE;
 
 	memmove(buf, fdt, fdt_totalsize(fdt));
diff --git a/scripts/dtc/libfdt/fdt_overlay.c b/scripts/dtc/libfdt/fdt_overlay.c
index bd75e3dd786..7a65c35af6f 100644
--- a/scripts/dtc/libfdt/fdt_overlay.c
+++ b/scripts/dtc/libfdt/fdt_overlay.c
@@ -241,6 +241,7 @@ static int overlay_update_local_node_references(void *fdto,
 
 		if (fixup_len % sizeof(uint32_t))
 			return -FDT_ERR_BADOVERLAY;
+		fixup_len /= sizeof(uint32_t);
 
 		tree_val = fdt_getprop(fdto, tree_node, name, &tree_len);
 		if (!tree_val) {
@@ -250,7 +251,7 @@ static int overlay_update_local_node_references(void *fdto,
 			return tree_len;
 		}
 
-		for (i = 0; i < (fixup_len / sizeof(uint32_t)); i++) {
+		for (i = 0; i < fixup_len; i++) {
 			fdt32_t adj_val;
 			uint32_t poffset;
 
diff --git a/scripts/dtc/libfdt/fdt_ro.c b/scripts/dtc/libfdt/fdt_ro.c
index d9d52e0d56e..d984bab036b 100644
--- a/scripts/dtc/libfdt/fdt_ro.c
+++ b/scripts/dtc/libfdt/fdt_ro.c
@@ -53,7 +53,7 @@ const char *fdt_get_string(const void *fdt, int stroffset, int *lenp)
 
 	err = -FDT_ERR_BADOFFSET;
 	absoffset = stroffset + fdt_off_dt_strings(fdt);
-	if (absoffset >= totalsize)
+	if (absoffset >= (unsigned)totalsize)
 		goto fail;
 	len = totalsize - absoffset;
 
@@ -61,17 +61,19 @@ const char *fdt_get_string(const void *fdt, int stroffset, int *lenp)
 		if (stroffset < 0)
 			goto fail;
 		if (!fdt_chk_version() || fdt_version(fdt) >= 17) {
-			if (stroffset >= fdt_size_dt_strings(fdt))
+			if ((unsigned)stroffset >= fdt_size_dt_strings(fdt))
 				goto fail;
 			if ((fdt_size_dt_strings(fdt) - stroffset) < len)
 				len = fdt_size_dt_strings(fdt) - stroffset;
 		}
 	} else if (fdt_magic(fdt) == FDT_SW_MAGIC) {
-		if ((stroffset >= 0)
-		    || (stroffset < -fdt_size_dt_strings(fdt)))
+		unsigned int sw_stroffset = -stroffset;
+
+		if ((stroffset >= 0) ||
+		    (sw_stroffset > fdt_size_dt_strings(fdt)))
 			goto fail;
-		if ((-stroffset) < len)
-			len = -stroffset;
+		if ((sw_stroffset) < len)
+			len = sw_stroffset;
 	} else {
 		err = -FDT_ERR_INTERNAL;
 		goto fail;
@@ -157,8 +159,8 @@ int fdt_generate_phandle(const void *fdt, uint32_t *phandle)
 
 static const struct fdt_reserve_entry *fdt_mem_rsv(const void *fdt, int n)
 {
-	int offset = n * sizeof(struct fdt_reserve_entry);
-	int absoffset = fdt_off_mem_rsvmap(fdt) + offset;
+	unsigned int offset = n * sizeof(struct fdt_reserve_entry);
+	unsigned int absoffset = fdt_off_mem_rsvmap(fdt) + offset;
 
 	if (fdt_chk_extra()) {
 		if (absoffset < fdt_off_mem_rsvmap(fdt))
@@ -679,7 +681,7 @@ int fdt_node_offset_by_phandle(const void *fdt, uint32_t phandle)
 {
 	int offset;
 
-	if ((phandle == 0) || (phandle == -1))
+	if ((phandle == 0) || (phandle == ~0U))
 		return -FDT_ERR_BADPHANDLE;
 
 	FDT_RO_PROBE(fdt);
diff --git a/scripts/dtc/libfdt/fdt_strerror.c b/scripts/dtc/libfdt/fdt_strerror.c
index 768db66eada..b4356931b06 100644
--- a/scripts/dtc/libfdt/fdt_strerror.c
+++ b/scripts/dtc/libfdt/fdt_strerror.c
@@ -40,7 +40,7 @@ static struct fdt_errtabent fdt_errtable[] = {
 	FDT_ERRTABENT(FDT_ERR_NOPHANDLES),
 	FDT_ERRTABENT(FDT_ERR_BADFLAGS),
 };
-#define FDT_ERRTABSIZE	(sizeof(fdt_errtable) / sizeof(fdt_errtable[0]))
+#define FDT_ERRTABSIZE	((int)(sizeof(fdt_errtable) / sizeof(fdt_errtable[0])))
 
 const char *fdt_strerror(int errval)
 {
@@ -48,7 +48,7 @@ const char *fdt_strerror(int errval)
 		return "<valid offset/length>";
 	else if (errval == 0)
 		return "<no error>";
-	else if (errval > -FDT_ERRTABSIZE) {
+	else if (-errval < FDT_ERRTABSIZE) {
 		const char *s = fdt_errtable[-errval].str;
 
 		if (s)
diff --git a/scripts/dtc/libfdt/fdt_sw.c b/scripts/dtc/libfdt/fdt_sw.c
index a8c924675a6..d9e67fa2a61 100644
--- a/scripts/dtc/libfdt/fdt_sw.c
+++ b/scripts/dtc/libfdt/fdt_sw.c
@@ -96,8 +96,8 @@ static inline uint32_t sw_flags(void *fdt)
 
 static void *fdt_grab_space_(void *fdt, size_t len)
 {
-	int offset = fdt_size_dt_struct(fdt);
-	int spaceleft;
+	unsigned int offset = fdt_size_dt_struct(fdt);
+	unsigned int spaceleft;
 
 	spaceleft = fdt_totalsize(fdt) - fdt_off_dt_struct(fdt)
 		- fdt_size_dt_strings(fdt);
@@ -111,8 +111,8 @@ static void *fdt_grab_space_(void *fdt, size_t len)
 
 int fdt_create_with_flags(void *buf, int bufsize, uint32_t flags)
 {
-	const size_t hdrsize = FDT_ALIGN(sizeof(struct fdt_header),
-					 sizeof(struct fdt_reserve_entry));
+	const int hdrsize = FDT_ALIGN(sizeof(struct fdt_header),
+				      sizeof(struct fdt_reserve_entry));
 	void *fdt = buf;
 
 	if (bufsize < hdrsize)
@@ -155,13 +155,16 @@ int fdt_resize(void *fdt, void *buf, int bufsize)
 
 	FDT_SW_PROBE(fdt);
 
+	if (bufsize < 0)
+		return -FDT_ERR_NOSPACE;
+
 	headsize = fdt_off_dt_struct(fdt) + fdt_size_dt_struct(fdt);
 	tailsize = fdt_size_dt_strings(fdt);
 
 	if (fdt_chk_extra() && (headsize + tailsize) > fdt_totalsize(fdt))
 		return -FDT_ERR_INTERNAL;
 
-	if ((headsize + tailsize) > bufsize)
+	if ((headsize + tailsize) > (unsigned)bufsize)
 		return -FDT_ERR_NOSPACE;
 
 	oldtail = (char *)fdt + fdt_totalsize(fdt) - tailsize;
@@ -249,18 +252,18 @@ int fdt_end_node(void *fdt)
 static int fdt_add_string_(void *fdt, const char *s)
 {
 	char *strtab = (char *)fdt + fdt_totalsize(fdt);
-	int strtabsize = fdt_size_dt_strings(fdt);
-	int len = strlen(s) + 1;
-	int struct_top, offset;
+	unsigned int strtabsize = fdt_size_dt_strings(fdt);
+	unsigned int len = strlen(s) + 1;
+	unsigned int struct_top, offset;
 
-	offset = -strtabsize - len;
+	offset = strtabsize + len;
 	struct_top = fdt_off_dt_struct(fdt) + fdt_size_dt_struct(fdt);
-	if (fdt_totalsize(fdt) + offset < struct_top)
+	if (fdt_totalsize(fdt) - offset < struct_top)
 		return 0; /* no more room :( */
 
-	memcpy(strtab + offset, s, len);
+	memcpy(strtab - offset, s, len);
 	fdt_set_size_dt_strings(fdt, strtabsize + len);
-	return offset;
+	return -offset;
 }
 
 /* Must only be used to roll back in case of error */
diff --git a/scripts/dtc/libfdt/fdt_wip.c b/scripts/dtc/libfdt/fdt_wip.c
index f64139e0b3d..c2d7566a67d 100644
--- a/scripts/dtc/libfdt/fdt_wip.c
+++ b/scripts/dtc/libfdt/fdt_wip.c
@@ -23,7 +23,7 @@ int fdt_setprop_inplace_namelen_partial(void *fdt, int nodeoffset,
 	if (!propval)
 		return proplen;
 
-	if (proplen < (len + idx))
+	if ((unsigned)proplen < (len + idx))
 		return -FDT_ERR_NOSPACE;
 
 	memcpy((char *)propval + idx, val, len);
-- 
2.17.5

[PATCH] libfdt: Fix signedness comparison warnings

[Tom Rini]

On Fri, Oct 16, 2020 at 03:42:50PM +0100, Andre Przywara wrote:

> This is a combination of upstream libfdt commits to fix warnings about
> comparing signed and unsigned integers:
> ==========
> scripts/dtc/libfdt/fdt.c: In function ?fdt_offset_ptr?:
> scripts/dtc/libfdt/fdt.c:137:18: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
>    if ((absoffset < offset)
> ...
> ==========
> 
> For a detailed description of the fixes, see the dtc repo:
> https://git.kernel.org/pub/scm/utils/dtc/dtc.git/log/?id=73e0f143b73d808
> 
> For this patch the commits between 73e0f143b73d8088 and ca19c3db2bf62000
> have been combined and adjusted for the slight differences in U-Boot's
> libfdt code base.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>

So, the scripts that the Linux kernel uses to re-sync with dtc also work
for U-Boot.  Has the kernel re-synced yet?  If so, can we just re-sync
with that same commit again?  That's typically how we do this.  Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20201016/11f4c6f6/attachment.sig>

[PATCH] libfdt: Fix signedness comparison warnings

[Tom Rini]

On Fri, Oct 16, 2020 at 10:57:19AM -0400, Tom Rini wrote:
> On Fri, Oct 16, 2020 at 03:42:50PM +0100, Andre Przywara wrote:
> 
> > This is a combination of upstream libfdt commits to fix warnings about
> > comparing signed and unsigned integers:
> > ==========
> > scripts/dtc/libfdt/fdt.c: In function ?fdt_offset_ptr?:
> > scripts/dtc/libfdt/fdt.c:137:18: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
> >    if ((absoffset < offset)
> > ...
> > ==========
> > 
> > For a detailed description of the fixes, see the dtc repo:
> > https://git.kernel.org/pub/scm/utils/dtc/dtc.git/log/?id=73e0f143b73d808
> > 
> > For this patch the commits between 73e0f143b73d8088 and ca19c3db2bf62000
> > have been combined and adjusted for the slight differences in U-Boot's
> > libfdt code base.
> > 
> > Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> 
> So, the scripts that the Linux kernel uses to re-sync with dtc also work
> for U-Boot.  Has the kernel re-synced yet?  If so, can we just re-sync
> with that same commit again?  That's typically how we do this.  Thanks!

I see that it has.  So I'll give a re-sync a try and see what comes up.
Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20201016/ede407db/attachment.sig>

[PATCH] libfdt: Fix signedness comparison warnings

[Tom Rini]

On Fri, Oct 16, 2020 at 03:42:50PM +0100, Andre Przywara wrote:
> This is a combination of upstream libfdt commits to fix warnings about

> comparing signed and unsigned integers:
> ==========
> scripts/dtc/libfdt/fdt.c: In function ?fdt_offset_ptr?:
> scripts/dtc/libfdt/fdt.c:137:18: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
>    if ((absoffset < offset)
> ...
> ==========
> 
> For a detailed description of the fixes, see the dtc repo:
> https://git.kernel.org/pub/scm/utils/dtc/dtc.git/log/?id=73e0f143b73d808
> 
> For this patch the commits between 73e0f143b73d8088 and ca19c3db2bf62000
> have been combined and adjusted for the slight differences in U-Boot's
> libfdt code base.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>

So, I've applied this to u-boot/master now.  These warnings do show up
with gcc-10 and it's worthwhile to silence them.  I'm working with
upstream dtc now so that when we resync next we'll be able to avoid the
size and performance penalties of making all fdt loads unaligned safe.
A further resync will also require us to fixup a number of dts warnings
again.  These are the main reasons that I'm setting aside my suggestion
of a full resync for now.  Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20201111/34902c0f/attachment.sig>