On Tue, Dec 20, 2016 at 11:32:16PM +0100, Heinrich Schuchardt wrote:
> Running fdtdump without scan mode for an invalid file often
> results in a segmentation fault because the fdt header is
> not checked.
>
> With the patch the header is checked both in scanning as
> well as in non-scanning mode.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@xxxxxx>
This seems to be more complex than it needs to me.
The idea of validating the header in non-scan mode seems sensible, but
this also appears to scan in non-scan mode.
It would be more sensible to just pull the header validation out of
the scanning block.
That said, fdtdump is pretty much a deliberately dumb tool. In
general the preferred way for decompiling a dtb is to use dtc -I dtb
-O dts.
> ---
> fdtdump.c | 57 +++++++++++++++++++++++++++++----------------------------
> 1 file changed, 29 insertions(+), 28 deletions(-)
>
> diff --git a/fdtdump.c b/fdtdump.c
> index a9a2484..717fef5 100644
> --- a/fdtdump.c
> +++ b/fdtdump.c
> @@ -189,39 +189,40 @@ int main(int argc, char *argv[])
> die("could not read: %s\n", file);
>
> /* try and locate an embedded fdt in a bigger blob */
> - if (scan) {
> - unsigned char smagic[FDT_MAGIC_SIZE];
> - char *p = buf;
> - char *endp = buf + len;
> + unsigned char smagic[FDT_MAGIC_SIZE];
> + char *p = buf;
> + char *endp = buf + len;
>
> - fdt_set_magic(smagic, FDT_MAGIC);
> + fdt_set_magic(smagic, FDT_MAGIC);
>
> - /* poor man's memmem */
> - while ((endp - p) >= FDT_MAGIC_SIZE) {
> - p = memchr(p, smagic[0], endp - p - FDT_MAGIC_SIZE);
> - if (!p)
> + /* poor man's memmem */
> + while ((endp - p) >= FDT_MAGIC_SIZE) {
> + p = memchr(p, smagic[0], endp - p - FDT_MAGIC_SIZE);
> + if (!p)
> + break;
> + if (fdt_magic(p) == FDT_MAGIC) {
> + /* try and validate the main struct */
> + off_t this_len = endp - p;
> + fdt32_t max_version = 17;
> + if (fdt_version(p) <= max_version &&
> + fdt_last_comp_version(p) < max_version &&
> + fdt_totalsize(p) < this_len &&
> + fdt_off_dt_struct(p) < this_len &&
> + fdt_off_dt_strings(p) < this_len)
> break;
> - if (fdt_magic(p) == FDT_MAGIC) {
> - /* try and validate the main struct */
> - off_t this_len = endp - p;
> - fdt32_t max_version = 17;
> - if (fdt_version(p) <= max_version &&
> - fdt_last_comp_version(p) < max_version &&
> - fdt_totalsize(p) < this_len &&
> - fdt_off_dt_struct(p) < this_len &&
> - fdt_off_dt_strings(p) < this_len)
> - break;
> - if (debug)
> - printf("%s: skipping fdt magic at offset %#zx\n",
> - file, p - buf);
> - }
> - ++p;
> + if (debug)
> + printf("%s: skipping fdt magic at offset %#zx\n",
> + file, p - buf);
> }
> - if (!p || ((endp - p) < FDT_MAGIC_SIZE))
> - die("%s: could not locate fdt magic\n", file);
> - printf("%s: found fdt at offset %#zx\n", file, p - buf);
> - buf = p;
> + if (!scan)
> + die("%s: fdt missing\n", file);
> + ++p;
> }
> + if (!p || ((endp - p) < FDT_MAGIC_SIZE))
> + die("%s: could not locate fdt magic\n", file);
> + if (scan)
> + printf("%s: found fdt at offset %#zx\n", file, p - buf);
> + buf = p;
>
> dump_blob(buf, debug);
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
Attachment:
signature.asc
Description: PGP signature
