[U-Boot] [PATCH] fdt: Fix string property comparison overflow
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(+)
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)
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
Indeed, thanks!
participants (3)
-
Peter Robinson -
Teddy Reed -
Tom Rini