qcow2: double free snapshots
PATCH 1/1 qcow2: double free snapshots
In qcow2_open(), if qcow2_read_snapshots() failed, qcow2_open() -> qcow2_free_snapshots() will be called, NULL snapshots dereference happened. because qcow2_free_snapshots has been performed before in the fail case of qcow2_read_snapshots().
shown as below callstack,qcow2_open
|- qcow2_read_snapshots |-- goto fail; |-- qcow2_free_snapshots |- goto fail; |- qcow2_free_snapshots /* on this case, NULL snapshots dereference happened. */
--- block/qcow2-snapshot.c | 4 ++++ 1 file changed, 4 insertions(+)
block/qcow2-snapshot.c +4 -0
@@ -31,6 +31,10 @@ void qcow2_free_snapshots(BlockDriverState *bs)BDRVQcowState *s = bs->opaque;int i; +if (!s || !s->snapshots) { +return ; + } +for (i =0 ; i < s->nb_snapshots; i++) {g_free (s->snapshots[i].name);g_free (s->snapshots[i].id_str);
Signature · 1 line
1.9.4.msysgit.0
Hi,
Quoted 36 lines
Subject: [Qemu-devel] [patch] qcow2: double free snapshots
In
qcow2_open(), ifqcow2_read_snapshots()failed,qcow2_open()->qcow2_free_snapshots()will be called, NULL snapshots dereference happened. becauseqcow2_free_snapshotshas been performed before in the fail case ofqcow2_read_snapshots().
shown as below callstack,qcow2_open|- qcow2_read_snapshots |-- goto fail; |-- qcow2_free_snapshots |- goto fail; |- qcow2_free_snapshots /* on this case, NULL snapshots dereferencehappened. */
Signed-off-by Zhang Haoyu <[email protected]>--- block/qcow2-snapshot.c | 4 ++++ 1 file changed, 4 insertions(+)block/qcow2-snapshot.c +4
-1@@ -31,6 +31,10 @@ void qcow2_free_snapshots(BlockDriverState *bs)BDRVQcowState *s = bs->opaque;int i; +if (!s || !s->snapshots) { +return ; + } +for (i =0 ; i < s->nb_snapshots; i++) {g_free (s->snapshots[i].name);g_free (s->snapshots[i].id_str); --
Quoted 2 lines
1.9.4.msysgit.0
Yes, the code path is right. But I don't think this check is necessary. The first free has set "s->nb_snapshots = 0", so it is impossibility go into g_free(s->snapshots[i].name), and g_free(s->snapshots) is OK, though it is NULL.
Best regards,
-Gonglei
Quoted 30 lines
Hi,
Quoted 28 lines
Subject: [Qemu-devel] [patch] qcow2: double free snapshots
In
qcow2_open(), ifqcow2_read_snapshots()failed,qcow2_open()->qcow2_free_snapshots()will be called, NULL snapshots dereference happened. becauseqcow2_free_snapshotshas been performed before in the fail case ofqcow2_read_snapshots().
shown as below callstack,qcow2_open|- qcow2_read_snapshots |-- goto fail; |-- qcow2_free_snapshots |- goto fail; |- qcow2_free_snapshots /* on this case, NULL snapshots dereferencehappened. */
Signed-off-by Zhang Haoyu <[email protected]>--- block/qcow2-snapshot.c | 4 ++++ 1 file changed, 4 insertions(+)block/qcow2-snapshot.c +0
-0@@ -31,6 +31,10 @@ void qcow2_free_snapshots(BlockDriverState *bs)BDRVQcowState *s = bs->opaque;int i;
Quoted 14 lines
Quoted 10 lines
block/qcow2-snapshot.c +4
-1+if (!s || !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.4.msysgit.0
Yes, the code path is right. But I don't think this check is necessary. The first free has set "
s->nb_snapshots= 0", so it is impossibility go intog_free(s->snapshots[i].name), andg_free(s->snapshots)is OK, though it is NULL.
Yes, you are right, ignore this patch, please.
Thanks,
Zhang Haoyu
Quoted 2 lines
Best regards,
-Gonglei