block/qcow2-snapshot: Fix a null pointer dereference in qcow2_free_snapshots

[Qemu-devel] [PATCH] block/qcow2-snapshot: Fix a null pointer dereference in qcow2_free_snapshots

Hu Chaojian posted 1 patch 6 years, 8 months ago

Download mbox

Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/DM5PR17MB1451D437FA7145AEBE4688CFDB880@DM5PR17MB1451.namprd17.prod.outlook.com

Test FreeBSD passed

Test checkpatch passed

Test docker passed

Test s390x passed

block/qcow2-snapshot.c | 4 ++++
1 file changed, 4 insertions(+)

Expand all Fold all

[Qemu-devel] [PATCH] block/qcow2-snapshot: Fix a null pointer dereference in qcow2_free_snapshots

Posted by Hu Chaojian 6 years, 8 months ago

From: chaojianhu <chaojianhu@hotmail.com>

In function qcow2_do_open, if "go fail;" before calling qcow2_read_snapshots, then snapshots 
will always be NULL. When dealing with "fail:", qcow2_free_snapshots will be called, and 
s->snapshots will be dereferenced without checked.

Reported-by: chaojianhu <chaojianhu@hotmail.com>
Signed-off-by: chaojianhu <chaojianhu@hotmail.com>

---
 block/qcow2-snapshot.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c
index 44243e0..4a8128c 100644
--- a/block/qcow2-snapshot.c
+++ b/block/qcow2-snapshot.c
@@ -35,6 +35,10 @@ void qcow2_free_snapshots(BlockDriverState *bs)
     BDRVQcow2State *s = bs->opaque;
     int i;
 
+    if (NULL == s->snapshots) {
+        return;
+    }
+
     for(i = 0; i < s->nb_snapshots; i++) {
         g_free(s->snapshots[i].name);
         g_free(s->snapshots[i].id_str);
-- 
1.9.1

Re: [Qemu-devel] [PATCH] block/qcow2-snapshot: Fix a null pointer dereference in qcow2_free_snapshots

Posted by Kevin Wolf 6 years, 8 months ago

Am 10.08.2017 um 09:52 hat Hu Chaojian geschrieben:
> From: chaojianhu <chaojianhu@hotmail.com>
> 
> In function qcow2_do_open, if "go fail;" before calling qcow2_read_snapshots, then snapshots 
> will always be NULL. When dealing with "fail:", qcow2_free_snapshots will be called, and 
> s->snapshots will be dereferenced without checked.

Did you try to reproduce an actual crash?

Becasue I don't think there will be one. In this case, s->nb_snapshots
is 0, so the loop doesn't even run.

Kevin

> Reported-by: chaojianhu <chaojianhu@hotmail.com>
> Signed-off-by: chaojianhu <chaojianhu@hotmail.com>
> 
> ---
>  block/qcow2-snapshot.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c
> index 44243e0..4a8128c 100644
> --- a/block/qcow2-snapshot.c
> +++ b/block/qcow2-snapshot.c
> @@ -35,6 +35,10 @@ void qcow2_free_snapshots(BlockDriverState *bs)
>      BDRVQcow2State *s = bs->opaque;
>      int i;
>  
> +    if (NULL == s->snapshots) {
> +        return;
> +    }
> +
>      for(i = 0; i < s->nb_snapshots; i++) {
>          g_free(s->snapshots[i].name);
>          g_free(s->snapshots[i].id_str);
> -- 
> 1.9.1
>