Re: [PATCH] fdt: change splicing offset detection

[Simon Glass <sjg@chromium.org>]

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