[PATCH] fdt: change splicing offset detection

In fdt_rw.c, -FDT_ERR_BADOFFSET is returned when either the sum of the old length and the splice point are less than the splice point, or when the sum of the old length and the splice point exceed the end of the pointer. Adding an int and a pointer may result in a pointer overflow, an undefined behavior, which means that the result of this if statement can't be recovered from. Checking if the old length exceeds the end of the pointer minus the pointer is a much safer check.
Signed-off-by: Elijah Conners business@elijahpepe.com --- scripts/dtc/libfdt/fdt_rw.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/dtc/libfdt/fdt_rw.c b/scripts/dtc/libfdt/fdt_rw.c index 2eb2b38703..672b74ae7a 100644 --- a/scripts/dtc/libfdt/fdt_rw.c +++ b/scripts/dtc/libfdt/fdt_rw.c @@ -58,7 +58,7 @@ static int fdt_splice_(void *fdt, void *splicepoint, int oldlen, int newlen) char *p = splicepoint; char *end = (char *)fdt + fdt_data_size_(fdt);
- if (((p + oldlen) < p) || ((p + oldlen) > end)) + if (oldlen >= (end - p)) return -FDT_ERR_BADOFFSET; if ((p < (char *)fdt) || ((end - oldlen + newlen) < (char *)fdt)) return -FDT_ERR_BADOFFSET;

Hi Elijah,
On Sun, 25 Sept 2022 at 22:30, Elijah Conners business@elijahpepe.com wrote:
In fdt_rw.c, -FDT_ERR_BADOFFSET is returned when either the sum of the old length and the splice point are less than the splice point, or when the sum of the old length and the splice point exceed the end of the pointer. Adding an int and a pointer may result in a pointer overflow, an undefined behavior, which means that the result of this if statement can't be recovered from. Checking if the old length exceeds the end of the pointer minus the pointer is a much safer check.
Signed-off-by: Elijah Conners business@elijahpepe.com
scripts/dtc/libfdt/fdt_rw.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/dtc/libfdt/fdt_rw.c b/scripts/dtc/libfdt/fdt_rw.c index 2eb2b38703..672b74ae7a 100644 --- a/scripts/dtc/libfdt/fdt_rw.c +++ b/scripts/dtc/libfdt/fdt_rw.c @@ -58,7 +58,7 @@ static int fdt_splice_(void *fdt, void *splicepoint, int oldlen, int newlen) char *p = splicepoint; char *end = (char *)fdt + fdt_data_size_(fdt);
if (((p + oldlen) < p) || ((p + oldlen) > end))
if (oldlen >= (end - p)) return -FDT_ERR_BADOFFSET; if ((p < (char *)fdt) || ((end - oldlen + newlen) < (char *)fdt)) return -FDT_ERR_BADOFFSET;
-- 2.29.2.windows.2
Granted this function is not documented but I don't think the new check is correct. I did not know that pointer overflow was undefined, but perhaps there is another way to solve this? This breaks CI.
I suggest sending this to the devicetreee-compiler mailing list and resolving it there first.
Regards, Simon
participants (2)
-
Elijah Conners
-
Simon Glass