diff mbox series

[1/2] libfdt: Fix undefined behaviour in fdt_offset_ptr()

Message ID e2734a4d-fb92-55e7-c08b-423f38049776@suse.com (mailing list archive)
State New, archived
Headers show
Series libfdt: eliminate UB pointer validation | expand

Commit Message

Jan Beulich March 13, 2020, 7:35 a.m. UTC 
From: David Gibson <david@gibson.dropbear.id.au>

Using pointer arithmetic to generate a pointer outside a known object is,
technically, undefined behaviour in C.  Unfortunately, we were using that
in fdt_offset_ptr() to detect overflows.

To fix this we need to do our bounds / overflow checking on the offsets
before constructing pointers from them.

Reported-by: David Binderman <dcb314@hotmail.com>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
[upstream commit d0b3ab0a0f46ac929b4713da46f7fdcd893dd3bd]
Signed-off-by: Jan Beulich <jbeulich@suse.com>

Comments

Julien Grall March 17, 2020, 2:51 p.m. UTC | #1 
Hi Jan,

On 13/03/2020 07:35, Jan Beulich wrote:
> From: David Gibson <david@gibson.dropbear.id.au>
> 
> Using pointer arithmetic to generate a pointer outside a known object is,
> technically, undefined behaviour in C.  Unfortunately, we were using that
> in fdt_offset_ptr() to detect overflows.
> 
> To fix this we need to do our bounds / overflow checking on the offsets
> before constructing pointers from them.
> 
> Reported-by: David Binderman <dcb314@hotmail.com>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> [upstream commit d0b3ab0a0f46ac929b4713da46f7fdcd893dd3bd]
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Acked-by: Julien Grall <jgrall@amazon.com>

Cheers,

> 
> --- a/xen/common/libfdt/fdt.c
> +++ b/xen/common/libfdt/fdt.c
> @@ -74,18 +74,19 @@ int fdt_check_header(const void *fdt)
>   
>   const void *fdt_offset_ptr(const void *fdt, int offset, unsigned int len)
>   {
> -	const char *p;
> +	unsigned absoffset = offset + fdt_off_dt_struct(fdt);
> +
> +	if ((absoffset < offset)
> +	    || ((absoffset + len) < absoffset)
> +	    || (absoffset + len) > fdt_totalsize(fdt))
> +		return NULL;
>   
>   	if (fdt_version(fdt) >= 0x11)
>   		if (((offset + len) < offset)
>   		    || ((offset + len) > fdt_size_dt_struct(fdt)))
>   			return NULL;
>   
> -	p = _fdt_offset_ptr(fdt, offset);
> -
> -	if (p + len < p)
> -		return NULL;
> -	return p;
> +	return _fdt_offset_ptr(fdt, offset);
>   }
>   
>   uint32_t fdt_next_tag(const void *fdt, int startoffset, int *nextoffset)
>
diff mbox series

Patch

--- a/xen/common/libfdt/fdt.c
+++ b/xen/common/libfdt/fdt.c
@@ -74,18 +74,19 @@  int fdt_check_header(const void *fdt)
 
 const void *fdt_offset_ptr(const void *fdt, int offset, unsigned int len)
 {
-	const char *p;
+	unsigned absoffset = offset + fdt_off_dt_struct(fdt);
+
+	if ((absoffset < offset)
+	    || ((absoffset + len) < absoffset)
+	    || (absoffset + len) > fdt_totalsize(fdt))
+		return NULL;
 
 	if (fdt_version(fdt) >= 0x11)
 		if (((offset + len) < offset)
 		    || ((offset + len) > fdt_size_dt_struct(fdt)))
 			return NULL;
 
-	p = _fdt_offset_ptr(fdt, offset);
-
-	if (p + len < p)
-		return NULL;
-	return p;
+	return _fdt_offset_ptr(fdt, offset);
 }
 
 uint32_t fdt_next_tag(const void *fdt, int startoffset, int *nextoffset)