[PATCH v2] fdtdump.c: make sure size_t argument to memchr is always unsigned.

[PATCH v2] fdtdump.c: make sure size_t argument to memchr is always unsigned.

[Jean-Christophe Dubois]

CID 132817 (#1 of 1): Integer overflowed argument (INTEGER_OVERFLOW)
15. overflow_sink: Overflowed or truncated value (or a value computed from an overflowed or truncated value) endp - p - 4L used as critical argument to function.

Signed-off-by: Jean-Christophe Dubois <jcd-WBS85hRCVJbxB9160cZjhg@public.gmane.org>
---

Changes since v1:
 * fix the post loop test.

 fdtdump.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/fdtdump.c b/fdtdump.c
index 95a6a20..a9a2484 100644
--- a/fdtdump.c
+++ b/fdtdump.c
@@ -15,6 +15,8 @@
 
 #include "util.h"
 
+#define FDT_MAGIC_SIZE	4
+
 #define ALIGN(x, a)	(((x) + ((a) - 1)) & ~((a) - 1))
 #define PALIGN(p, a)	((void *)(ALIGN((unsigned long)(p), (a))))
 #define GET_CELL(p)	(p += 4, *((const uint32_t *)(p-4)))
@@ -188,15 +190,15 @@ int main(int argc, char *argv[])
 
 	/* try and locate an embedded fdt in a bigger blob */
 	if (scan) {
-		unsigned char smagic[4];
+		unsigned char smagic[FDT_MAGIC_SIZE];
 		char *p = buf;
 		char *endp = buf + len;
 
 		fdt_set_magic(smagic, FDT_MAGIC);
 
 		/* poor man's memmem */
-		while (true) {
-			p = memchr(p, smagic[0], endp - p - 4);
+		while ((endp - p) >= FDT_MAGIC_SIZE) {
+			p = memchr(p, smagic[0], endp - p - FDT_MAGIC_SIZE);
 			if (!p)
 				break;
 			if (fdt_magic(p) == FDT_MAGIC) {
@@ -215,7 +217,7 @@ int main(int argc, char *argv[])
 			}
 			++p;
 		}
-		if (!p)
+		if (!p || ((endp - p) < FDT_MAGIC_SIZE))
 			die("%s: could not locate fdt magic\n", file);
 		printf("%s: found fdt at offset %#zx\n", file, p - buf);
 		buf = p;
-- 
2.7.4

Re: [PATCH v2] fdtdump.c: make sure size_t argument to memchr is always unsigned.

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 2146 bytes --]

On Wed, Jul 13, 2016 at 02:31:13AM +0200, Jean-Christophe Dubois wrote:
> CID 132817 (#1 of 1): Integer overflowed argument (INTEGER_OVERFLOW)
> 15. overflow_sink: Overflowed or truncated value (or a value computed from an overflowed or truncated value) endp - p - 4L used as critical argument to function.
> 
> Signed-off-by: Jean-Christophe Dubois <jcd-WBS85hRCVJbxB9160cZjhg@public.gmane.org>

Applied.

fdtdump is a hacky tool, so I don't particularly care about bugs in
it, but I guess we might as well shut up the semantic checker.

> ---
> 
> Changes since v1:
>  * fix the post loop test.
> 
>  fdtdump.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/fdtdump.c b/fdtdump.c
> index 95a6a20..a9a2484 100644
> --- a/fdtdump.c
> +++ b/fdtdump.c
> @@ -15,6 +15,8 @@
>  
>  #include "util.h"
>  
> +#define FDT_MAGIC_SIZE	4
> +
>  #define ALIGN(x, a)	(((x) + ((a) - 1)) & ~((a) - 1))
>  #define PALIGN(p, a)	((void *)(ALIGN((unsigned long)(p), (a))))
>  #define GET_CELL(p)	(p += 4, *((const uint32_t *)(p-4)))
> @@ -188,15 +190,15 @@ int main(int argc, char *argv[])
>  
>  	/* try and locate an embedded fdt in a bigger blob */
>  	if (scan) {
> -		unsigned char smagic[4];
> +		unsigned char smagic[FDT_MAGIC_SIZE];
>  		char *p = buf;
>  		char *endp = buf + len;
>  
>  		fdt_set_magic(smagic, FDT_MAGIC);
>  
>  		/* poor man's memmem */
> -		while (true) {
> -			p = memchr(p, smagic[0], endp - p - 4);
> +		while ((endp - p) >= FDT_MAGIC_SIZE) {
> +			p = memchr(p, smagic[0], endp - p - FDT_MAGIC_SIZE);
>  			if (!p)
>  				break;
>  			if (fdt_magic(p) == FDT_MAGIC) {
> @@ -215,7 +217,7 @@ int main(int argc, char *argv[])
>  			}
>  			++p;
>  		}
> -		if (!p)
> +		if (!p || ((endp - p) < FDT_MAGIC_SIZE))
>  			die("%s: could not locate fdt magic\n", file);
>  		printf("%s: found fdt at offset %#zx\n", file, p - buf);
>  		buf = p;

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]