* [Qemu-devel] Possible corruption on qcow2_snapshot_goto
@ 2011-08-05 7:34 Frediano Ziglio
2011-08-05 7:49 ` Kevin Wolf
0 siblings, 1 reply; 2+ messages in thread
From: Frediano Ziglio @ 2011-08-05 7:34 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-devel
This function seems to be not that safe.
The first problem is that the first thing it does if free refcount
calling qcow2_update_snapshot_refcount(bs, s->l1_table_offset,
s->l1_size, -1). Now if something goes wrong (crash, disk errors) this
could lead to refcount == 0 (depending on L1 size, cache, open flags).
If table grow and qcow2_grow_l1_table needs to resize on disk it
probably will found some reference counter set to 0 (cause we already
decremented it), fail than qcow2_snapshot_goto return -EIO leaving
refcounts deallocated.
Frediano
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [Qemu-devel] Possible corruption on qcow2_snapshot_goto
2011-08-05 7:34 [Qemu-devel] Possible corruption on qcow2_snapshot_goto Frediano Ziglio
@ 2011-08-05 7:49 ` Kevin Wolf
0 siblings, 0 replies; 2+ messages in thread
From: Kevin Wolf @ 2011-08-05 7:49 UTC (permalink / raw)
To: Frediano Ziglio; +Cc: qemu-devel
Am 05.08.2011 09:34, schrieb Frediano Ziglio:
> This function seems to be not that safe.
>
> The first problem is that the first thing it does if free refcount
> calling qcow2_update_snapshot_refcount(bs, s->l1_table_offset,
> s->l1_size, -1). Now if something goes wrong (crash, disk errors) this
> could lead to refcount == 0 (depending on L1 size, cache, open flags).
>
> If table grow and qcow2_grow_l1_table needs to resize on disk it
> probably will found some reference counter set to 0 (cause we already
> decremented it), fail than qcow2_snapshot_goto return -EIO leaving
> refcounts deallocated.
I can't say I'm surprised.
This kind of problem used to be common all over the place in qcow2. I
fixed it for most paths, but internal snapshots aren't considered
supported for RHEL, so I never got around to fixing the snapshot code.
So, yes, the order must be changed, so that in the worst case we have
cluster leaks.
Kevin
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2011-08-05 7:46 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-08-05 7:34 [Qemu-devel] Possible corruption on qcow2_snapshot_goto Frediano Ziglio
2011-08-05 7:49 ` Kevin Wolf
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.