Re: [PATCH master] of: fdt: fix overflowing in dt_struct_advance arguments

[Sascha Hauer <s.hauer@pengutronix.de>]

On Wed, Jun 11, 2025 at 08:39:10AM +0200, Ahmad Fatoum wrote:
> While dt_struct_advance was taking care to check its arguments don't
> overflow their type, the addition of len (that is read from the FDT)
> to a constant was already overflowing before the function was called.
> 
> Move all additions with untrusted input into the function to fix this.
> 
> This resolves crashes detected by libfuzzer when the digest functions
> were ultimately called with a length of -1 == 0xffffffff.
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  drivers/of/fdt.c | 20 ++++++++++++--------
>  1 file changed, 12 insertions(+), 8 deletions(-)

This conflicts with https://lore.kernel.org/all/20250605112607.1970520-2-a.fatoum@pengutronix.de/
which is currently not in master. So it's all or nothing.

Sascha

> 
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index 84a36c77bbf0..f2f4aa03de2d 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -33,11 +33,15 @@ static inline bool __dt_ptr_ok(const struct fdt_header *fdt, const void *p,
>  }
>  #define dt_ptr_ok(fdt, p) __dt_ptr_ok(fdt, p, sizeof(*(p)), __alignof__(*(p)))
>  
> -static inline uint32_t dt_struct_advance(struct fdt_header *f, uint32_t dt, uint32_t size)
> +static inline uint32_t dt_struct_advance(struct fdt_header *f, uint32_t dt, uint32_t size,
> +					 uint32_t increment)
>  {
>  	if (check_add_overflow(dt, size, &dt))
>  		return 0;
>  
> +	if (check_add_overflow(dt, increment, &dt))
> +		return 0;
> +
>  	dt = ALIGN(dt, 4);
>  	if (dt > f->off_dt_struct + f->size_dt_struct)
>  		return 0;
> @@ -229,7 +233,7 @@ static struct device_node *__of_unflatten_dtb(const void *infdt, int size,
>  			}
>  
>  			dt_struct = dt_struct_advance(&f, dt_struct,
> -					sizeof(struct fdt_node_header) + len + 1);
> +					sizeof(struct fdt_node_header) + 1, len);
>  			if (!dt_struct) {
>  				ret = -ESPIPE;
>  				goto err;
> @@ -262,7 +266,7 @@ static struct device_node *__of_unflatten_dtb(const void *infdt, int size,
>  
>  			node = node->parent;
>  
> -			dt_struct = dt_struct_advance(&f, dt_struct, FDT_TAGSIZE);
> +			dt_struct = dt_struct_advance(&f, dt_struct, FDT_TAGSIZE, 0);
>  			if (!dt_struct) {
>  				ret = -ESPIPE;
>  				goto err;
> @@ -287,7 +291,7 @@ static struct device_node *__of_unflatten_dtb(const void *infdt, int size,
>  			}
>  
>  			dt_struct = dt_struct_advance(&f, dt_struct,
> -					sizeof(struct fdt_property) + len);
> +					sizeof(struct fdt_property), len);
>  			if (!dt_struct) {
>  				ret = -ESPIPE;
>  				goto err;
> @@ -305,7 +309,7 @@ static struct device_node *__of_unflatten_dtb(const void *infdt, int size,
>  			break;
>  
>  		case FDT_NOP:
> -			dt_struct = dt_struct_advance(&f, dt_struct, FDT_TAGSIZE);
> +			dt_struct = dt_struct_advance(&f, dt_struct, FDT_TAGSIZE, 0);
>  			if (!dt_struct) {
>  				ret = -ESPIPE;
>  				goto err;
> @@ -776,7 +780,7 @@ int fdt_machine_is_compatible(const struct fdt_header *fdt, size_t fdt_size, con
>  				return 0;
>  
>  			dt_struct = dt_struct_advance(&f, dt_struct,
> -					sizeof(struct fdt_node_header) + 1);
> +					sizeof(struct fdt_node_header), 1);
>  			if (!dt_struct)
>  				return 0;
>  
> @@ -803,7 +807,7 @@ int fdt_machine_is_compatible(const struct fdt_header *fdt, size_t fdt_size, con
>  				return 0;
>  
>  			dt_struct = dt_struct_advance(&f, dt_struct,
> -						      sizeof(struct fdt_property) + len);
> +						      sizeof(struct fdt_property), len);
>  			if (!dt_struct)
>  				return 0;
>  
> @@ -813,7 +817,7 @@ int fdt_machine_is_compatible(const struct fdt_header *fdt, size_t fdt_size, con
>  			return fdt_string_is_compatible(fdt_prop->data, len, compat, compat_len);
>  
>  		case FDT_NOP:
> -			dt_struct = dt_struct_advance(&f, dt_struct, FDT_TAGSIZE);
> +			dt_struct = dt_struct_advance(&f, dt_struct, FDT_TAGSIZE, 0);
>  			if (!dt_struct)
>  				return 0;
>  			break;
> -- 
> 2.39.5
> 
> 
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |