| Message ID | 20181113164544.9906-1-berto@igalia.com |
|---|---|
| State | New |
| Headers | show |
| Series | qcow2: Assert that refcount block offsets fit in the refcount table | expand |
On 11/13/18 10:45 AM, Alberto Garcia wrote: > Refcount table entries have a field to store the offset of the > refcount block. The rest of the bits of the entry are currently > reserved. > > The offset is always taken from the entry using REFT_OFFSET_MASK to > ensure that we only use the bits that belong to that field. > > While that mask is used every time we read from the refcount table, it > is never used when we write to it. Due to the other constraints of the > qcow2 format QEMU can never produce refcount block offsets that don't > fit in that field so any such offset when allocating a refcount block > would indicate a bug in QEMU. > --- > block/qcow2-refcount.c | 3 +++ > 1 file changed, 3 insertions(+) > Reviewed-by: Eric Blake <eblake@redhat.com>
On Tue 13 Nov 2018 06:06:54 PM CET, Eric Blake <eblake@redhat.com> wrote: >> Refcount table entries have a field to store the offset of the >> refcount block. The rest of the bits of the entry are currently >> reserved. >> >> The offset is always taken from the entry using REFT_OFFSET_MASK to >> ensure that we only use the bits that belong to that field. >> >> While that mask is used every time we read from the refcount table, it >> is never used when we write to it. Due to the other constraints of the >> qcow2 format QEMU can never produce refcount block offsets that don't >> fit in that field so any such offset when allocating a refcount block >> would indicate a bug in QEMU. >> --- >> block/qcow2-refcount.c | 3 +++ >> 1 file changed, 3 insertions(+) >> > > Reviewed-by: Eric Blake <eblake@redhat.com> Yes, for 3.1, shall I resend it with the updated subject message? Berto
Am 14.11.2018 um 08:10 hat Alberto Garcia geschrieben: > On Tue 13 Nov 2018 06:06:54 PM CET, Eric Blake <eblake@redhat.com> wrote: > > >> Refcount table entries have a field to store the offset of the > >> refcount block. The rest of the bits of the entry are currently > >> reserved. > >> > >> The offset is always taken from the entry using REFT_OFFSET_MASK to > >> ensure that we only use the bits that belong to that field. > >> > >> While that mask is used every time we read from the refcount table, it > >> is never used when we write to it. Due to the other constraints of the > >> qcow2 format QEMU can never produce refcount block offsets that don't > >> fit in that field so any such offset when allocating a refcount block > >> would indicate a bug in QEMU. Missing S-o-b. > >> block/qcow2-refcount.c | 3 +++ > >> 1 file changed, 3 insertions(+) > >> > > > > Reviewed-by: Eric Blake <eblake@redhat.com> > > Yes, for 3.1, shall I resend it with the updated subject message? Honestly, I don't see why an additional assertion should qualify as a fix? If it changes the behaviour, it's a bug. You wouldn't have to resend for the updated subject message, but you do for the missing S-o-b. Kevin
On Wed 14 Nov 2018 11:49:34 AM CET, Kevin Wolf wrote: >> > Reviewed-by: Eric Blake <eblake@redhat.com> >> >> Yes, for 3.1, shall I resend it with the updated subject message? > > Honestly, I don't see why an additional assertion should qualify as a > fix? If it changes the behaviour, it's a bug. We can leave it for after 3.1 if you think it's more appropriate. I'll resend it with the S-o-b anyway. Berto
diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c index 46082aeac1..31a2e1f845 100644 --- a/block/qcow2-refcount.c +++ b/block/qcow2-refcount.c @@ -367,6 +367,9 @@ static int alloc_refcount_block(BlockDriverState *bs, return new_block; } + /* The offset must fit in the offset field of the refcount table entry */ + assert((new_block & REFT_OFFSET_MASK) == new_block); + /* If we're allocating the block at offset 0 then something is wrong */ if (new_block == 0) { qcow2_signal_corruption(bs, true, -1, -1, "Preventing invalid "