On Sat, Aug 11, 2012 at 01:33:42PM +0100, Stefan Hajnoczi wrote:
> On Sat, Aug 11, 2012 at 12:11 AM, Peter Crosthwaite
> <peter.crosthwa...@petalogix.com> wrote:
> > On Fri, Aug 10, 2012 at 11:42 PM, Stefan Hajnoczi <stefa...@gmail.com>
> > wrote:
> >> On Fri, Aug 10, 2012 at 01:54:26PM +1000, Peter A. G. Crosthwaite wrote:
> >>> The sizep arg is populated with the size of the loaded device tree. Since
> >>> this
> >>> is one of those informational "please populate" type arguments it should
> >>> be
> >>> optional. Guarded writes to *sizep against NULL accordingly.
> >>>
> >>> Signed-off-by: Peter A. G. Crosthwaite <peter.crosthwa...@petalogix.com>
> >>> Acked-by: Alexander Graf <ag...@suse.de>
> >>> ---
> >>> device_tree.c | 8 ++++++--
> >>> 1 files changed, 6 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/device_tree.c b/device_tree.c
> >>> index d7a9b6b..641a48a 100644
> >>> --- a/device_tree.c
> >>> +++ b/device_tree.c
> >>> @@ -71,7 +71,9 @@ void *load_device_tree(const char *filename_path, int
> >>> *sizep)
> >>> int ret;
> >>> void *fdt = NULL;
> >>>
> >>> - *sizep = 0;
> >>> + if (sizep) {
> >>> + *sizep = 0;
> >>> + }
> >>> dt_size = get_image_size(filename_path);
> >>> if (dt_size < 0) {
> >>> printf("Unable to get size of device tree file '%s'\n",
> >>> @@ -104,7 +106,9 @@ void *load_device_tree(const char *filename_path, int
> >>> *sizep)
> >>> filename_path);
> >>> goto fail;
> >>> }
> >>> - *sizep = dt_size;
> >>> + if (sizep) {
> >>> + *sizep = dt_size;
> >>> + }
> >>
> >> What can the caller do with this void* buffer without knowing its size?
> >>
> >
> > Sanity check the machine:
> >
> > dtb = load_device_tree( ... ); //dont care how big it is
> > foo = fdt_gep_prop( dtb, ... );
> > if (foo != object_get_prop(foo_device, foo_prop, ... )) {
> > hw_error("your dtb is bad because ... !\n", ... );
> > }
>
> What happens if the fdt is corrupt or malicious? I guess we'll access
> memory beyond the end of blob.
>
> This seems to be libfdt's fault. I didn't see an API to validate the
> blob's size.
>
> I'm "happy" with this patch but if fdt's can ever come from untrusted
> sources then we're in trouble.
Jon/David, can you confirm that libfdt has no way of check the size of
the fdt blob?
For example, if I pass a corrupt or malicious blob to libfdt, is there a
way to detect that or will it access memory beyond the end of the blob
as we query the device tree?
Stefan
