[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-trivial] [Qemu-devel] [PATCH] device_tree: load_device_tree():
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-trivial] [Qemu-devel] [PATCH] device_tree: load_device_tree(): Allow NULL sizep |
Date: |
Sat, 11 Aug 2012 13:33:42 +0100 |
On Sat, Aug 11, 2012 at 12:11 AM, Peter Crosthwaite
<address@hidden> wrote:
> On Fri, Aug 10, 2012 at 11:42 PM, Stefan Hajnoczi <address@hidden> 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 <address@hidden>
>>> Acked-by: Alexander Graf <address@hidden>
>>> ---
>>> 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.
Stefan