[PATCH] fdt: change splicing offset detection

[PATCH] fdt: change splicing offset detection

[Elijah Conners]

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

Re: [PATCH] fdt: change splicing offset detection

[Simon Glass]

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