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

[Ahmad Fatoum <a.fatoum@pengutronix.de>]

Hi Sascha,

On 6/11/25 09:01, Sascha Hauer wrote:
> 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.

Please apply both to master.

Thanks,
Ahmad

> 
> 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 |