| Message ID | 20180603202244.2c6a88e14dd9cfb67f0cee04@gmail.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | [U-Boot] fdt: Fix string property comparison overflow | expand |
On Mon, Jun 4, 2018 at 1:22 AM, Teddy Reed <teddy.reed@gmail.com> wrote: > FDT property searching can overflow when comparing strings. This will > result in undefined behavior. > > This check assures that property name lengths do not overrun the string > region or the totalsize. The lib/libfdt is mostly a sync from upstream dtc [1] so I suspect it's a problem there too and should probably sent and accepted there and it'll then be pulled back in a resync. Peter [1] https://git.kernel.org/pub/scm/utils/dtc/dtc.git > Signed-off-by: Teddy Reed <teddy.reed@gmail.com> > --- > lib/libfdt/fdt_ro.c | 5 +++++ > scripts/dtc/libfdt/fdt.c | 2 ++ > 2 files changed, 7 insertions(+) > > diff --git a/lib/libfdt/fdt_ro.c b/lib/libfdt/fdt_ro.c > index b6ca4e0..612f3ac 100644 > --- a/lib/libfdt/fdt_ro.c > +++ b/lib/libfdt/fdt_ro.c > @@ -42,6 +42,11 @@ const char *fdt_string(const void *fdt, int stroffset) > static int _fdt_string_eq(const void *fdt, int stroffset, > const char *s, int len) > { > + int total_off = fdt_off_dt_strings(fdt) + stroffset; > + if (total_off + len + 1 < total_off || > + total_off + len + 1 > fdt_totalsize(fdt)) > + return 0; > + > const char *p = fdt_string(fdt, stroffset); > > return (strnlen(p, len + 1) == len) && (memcmp(p, s, len) == 0); > diff --git a/scripts/dtc/libfdt/fdt.c b/scripts/dtc/libfdt/fdt.c > index 7855a17..dffd28d 100644 > --- a/scripts/dtc/libfdt/fdt.c > +++ b/scripts/dtc/libfdt/fdt.c > @@ -57,6 +57,8 @@ > > int fdt_check_header(const void *fdt) > { > + if (fdt == NULL) > + return -FDT_ERR_BADSTRUCTURE; > if (fdt_magic(fdt) == FDT_MAGIC) { > /* Complete tree */ > if (fdt_version(fdt) < FDT_FIRST_SUPPORTED_VERSION) > -- > 2.7.4 > > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > https://lists.denx.de/listinfo/u-boot
Ignore this patch (re: below Peter's comment). On Mon, Jun 4, 2018 at 1:42 AM, Peter Robinson <pbrobinson@gmail.com> wrote: > On Mon, Jun 4, 2018 at 1:22 AM, Teddy Reed <teddy.reed@gmail.com> wrote: >> FDT property searching can overflow when comparing strings. This will >> result in undefined behavior. >> >> This check assures that property name lengths do not overrun the string >> region or the totalsize. > > The lib/libfdt is mostly a sync from upstream dtc [1] so I suspect > it's a problem there too and should probably sent and accepted there > and it'll then be pulled back in a resync. Thanks for the heads up Peter, will do! > > Peter > > [1] https://git.kernel.org/pub/scm/utils/dtc/dtc.git > >> Signed-off-by: Teddy Reed <teddy.reed@gmail.com> >> --- >> lib/libfdt/fdt_ro.c | 5 +++++ >> scripts/dtc/libfdt/fdt.c | 2 ++ >> 2 files changed, 7 insertions(+) >> >> diff --git a/lib/libfdt/fdt_ro.c b/lib/libfdt/fdt_ro.c >> index b6ca4e0..612f3ac 100644 >> --- a/lib/libfdt/fdt_ro.c >> +++ b/lib/libfdt/fdt_ro.c >> @@ -42,6 +42,11 @@ const char *fdt_string(const void *fdt, int stroffset) >> static int _fdt_string_eq(const void *fdt, int stroffset, >> const char *s, int len) >> { >> + int total_off = fdt_off_dt_strings(fdt) + stroffset; >> + if (total_off + len + 1 < total_off || >> + total_off + len + 1 > fdt_totalsize(fdt)) >> + return 0; >> + >> const char *p = fdt_string(fdt, stroffset); >> >> return (strnlen(p, len + 1) == len) && (memcmp(p, s, len) == 0); >> diff --git a/scripts/dtc/libfdt/fdt.c b/scripts/dtc/libfdt/fdt.c >> index 7855a17..dffd28d 100644 >> --- a/scripts/dtc/libfdt/fdt.c >> +++ b/scripts/dtc/libfdt/fdt.c >> @@ -57,6 +57,8 @@ >> >> int fdt_check_header(const void *fdt) >> { >> + if (fdt == NULL) >> + return -FDT_ERR_BADSTRUCTURE; >> if (fdt_magic(fdt) == FDT_MAGIC) { >> /* Complete tree */ >> if (fdt_version(fdt) < FDT_FIRST_SUPPORTED_VERSION) >> -- >> 2.7.4 >> >> _______________________________________________ >> U-Boot mailing list >> U-Boot@lists.denx.de >> https://lists.denx.de/listinfo/u-boot
On Mon, Jun 04, 2018 at 06:42:28AM +0100, Peter Robinson wrote: > On Mon, Jun 4, 2018 at 1:22 AM, Teddy Reed <teddy.reed@gmail.com> wrote: > > FDT property searching can overflow when comparing strings. This will > > result in undefined behavior. > > > > This check assures that property name lengths do not overrun the string > > region or the totalsize. > > The lib/libfdt is mostly a sync from upstream dtc [1] so I suspect > it's a problem there too and should probably sent and accepted there > and it'll then be pulled back in a resync. > > Peter > > [1] https://git.kernel.org/pub/scm/utils/dtc/dtc.git Indeed, thanks!
diff --git a/lib/libfdt/fdt_ro.c b/lib/libfdt/fdt_ro.c index b6ca4e0..612f3ac 100644 --- a/lib/libfdt/fdt_ro.c +++ b/lib/libfdt/fdt_ro.c @@ -42,6 +42,11 @@ const char *fdt_string(const void *fdt, int stroffset) static int _fdt_string_eq(const void *fdt, int stroffset, const char *s, int len) { + int total_off = fdt_off_dt_strings(fdt) + stroffset; + if (total_off + len + 1 < total_off || + total_off + len + 1 > fdt_totalsize(fdt)) + return 0; + const char *p = fdt_string(fdt, stroffset); return (strnlen(p, len + 1) == len) && (memcmp(p, s, len) == 0); diff --git a/scripts/dtc/libfdt/fdt.c b/scripts/dtc/libfdt/fdt.c index 7855a17..dffd28d 100644 --- a/scripts/dtc/libfdt/fdt.c +++ b/scripts/dtc/libfdt/fdt.c @@ -57,6 +57,8 @@ int fdt_check_header(const void *fdt) { + if (fdt == NULL) + return -FDT_ERR_BADSTRUCTURE; if (fdt_magic(fdt) == FDT_MAGIC) { /* Complete tree */ if (fdt_version(fdt) < FDT_FIRST_SUPPORTED_VERSION)
FDT property searching can overflow when comparing strings. This will result in undefined behavior. This check assures that property name lengths do not overrun the string region or the totalsize. Signed-off-by: Teddy Reed <teddy.reed@gmail.com> --- lib/libfdt/fdt_ro.c | 5 +++++ scripts/dtc/libfdt/fdt.c | 2 ++ 2 files changed, 7 insertions(+)