[v2] off-by-one and NULL pointer accesses detected by static analysis

[Qemu-devel] [PATCH v2 6/8] block: dump_qlist() may dereference a Null pointer

Posted by Liam Merwick 7 years, 5 months ago

A NULL 'list' passed into function dump_qlist() isn't correctly
validated and can be passed to qlist_first() where it is dereferenced.

Given that  dump_qlist() is static, and callers already do the right
thing, just add an assert to catch future potential bugs.

Signed-off-by: Liam Merwick <Liam.Merwick@oracle.com>
---
 block/qapi.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/block/qapi.c b/block/qapi.c
index c66f949db839..e81be604217c 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -740,6 +740,8 @@ static void dump_qlist(fprintf_function func_fprintf, void *f, int indentation,
     const QListEntry *entry;
     int i = 0;
 
+    assert(list);
+
     for (entry = qlist_first(list); entry; entry = qlist_next(entry), i++) {
         QType type = qobject_type(entry->value);
         bool composite = (type == QTYPE_QDICT || type == QTYPE_QLIST);
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v2 6/8] block: dump_qlist() may dereference a Null pointer

Posted by Eric Blake 7 years, 5 months ago

On 08/31/2018 11:36 AM, Liam Merwick wrote:
> A NULL 'list' passed into function dump_qlist() isn't correctly
> validated and can be passed to qlist_first() where it is dereferenced.
> 
> Given that  dump_qlist() is static, and callers already do the right

Double space looks odd.

> thing, just add an assert to catch future potential bugs.
> 
> Signed-off-by: Liam Merwick <Liam.Merwick@oracle.com>
> ---
>   block/qapi.c | 2 ++
>   1 file changed, 2 insertions(+)

Reviewed-by: Eric Blake <eblake@redhat.com>

> 
> diff --git a/block/qapi.c b/block/qapi.c
> index c66f949db839..e81be604217c 100644
> --- a/block/qapi.c
> +++ b/block/qapi.c
> @@ -740,6 +740,8 @@ static void dump_qlist(fprintf_function func_fprintf, void *f, int indentation,
>       const QListEntry *entry;
>       int i = 0;
>   
> +    assert(list);
> +
>       for (entry = qlist_first(list); entry; entry = qlist_next(entry), i++) {
>           QType type = qobject_type(entry->value);
>           bool composite = (type == QTYPE_QDICT || type == QTYPE_QLIST);
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org