* [PATCH master] of: fdt: fix overflowing in dt_struct_advance arguments
@ 2025-06-11 6:39 Ahmad Fatoum
2025-06-11 7:01 ` Sascha Hauer
2025-06-12 7:44 ` Sascha Hauer
0 siblings, 2 replies; 4+ messages in thread
From: Ahmad Fatoum @ 2025-06-11 6:39 UTC (permalink / raw)
To: barebox; +Cc: Ahmad Fatoum
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(-)
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
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH master] of: fdt: fix overflowing in dt_struct_advance arguments
2025-06-11 6:39 [PATCH master] of: fdt: fix overflowing in dt_struct_advance arguments Ahmad Fatoum
@ 2025-06-11 7:01 ` Sascha Hauer
2025-06-11 7:51 ` Ahmad Fatoum
2025-06-12 7:44 ` Sascha Hauer
1 sibling, 1 reply; 4+ messages in thread
From: Sascha Hauer @ 2025-06-11 7:01 UTC (permalink / raw)
To: Ahmad Fatoum; +Cc: barebox
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 |
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH master] of: fdt: fix overflowing in dt_struct_advance arguments
2025-06-11 7:01 ` Sascha Hauer
@ 2025-06-11 7:51 ` Ahmad Fatoum
0 siblings, 0 replies; 4+ messages in thread
From: Ahmad Fatoum @ 2025-06-11 7:51 UTC (permalink / raw)
To: Sascha Hauer; +Cc: barebox
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 |
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH master] of: fdt: fix overflowing in dt_struct_advance arguments
2025-06-11 6:39 [PATCH master] of: fdt: fix overflowing in dt_struct_advance arguments Ahmad Fatoum
2025-06-11 7:01 ` Sascha Hauer
@ 2025-06-12 7:44 ` Sascha Hauer
1 sibling, 0 replies; 4+ messages in thread
From: Sascha Hauer @ 2025-06-12 7:44 UTC (permalink / raw)
To: barebox, Ahmad Fatoum
On Wed, 11 Jun 2025 08:39:10 +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.
>
> [...]
Applied, thanks!
[1/1] of: fdt: fix overflowing in dt_struct_advance arguments
https://git.pengutronix.de/cgit/barebox/commit/?id=26136fd068d7 (link may not be stable)
Best regards,
--
Sascha Hauer <s.hauer@pengutronix.de>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-06-12 8:10 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-06-11 6:39 [PATCH master] of: fdt: fix overflowing in dt_struct_advance arguments Ahmad Fatoum
2025-06-11 7:01 ` Sascha Hauer
2025-06-11 7:51 ` Ahmad Fatoum
2025-06-12 7:44 ` Sascha Hauer
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox