[Qemu-devel] [PATCH] qcow2: Assert that L2 table offsets fit in the L1 table

Alberto Garcia posted 1 patch 5 years, 1 month ago
Test docker-mingw@fedora passed
Test asan passed
Test checkpatch passed
Test docker-clang@ubuntu passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20190208154453.25602-1-berto@igalia.com
Maintainers: Max Reitz <mreitz@redhat.com>, Kevin Wolf <kwolf@redhat.com>
block/qcow2-cluster.c | 3 +++
1 file changed, 3 insertions(+)
[Qemu-devel] [PATCH] qcow2: Assert that L2 table offsets fit in the L1 table
Posted by Alberto Garcia 5 years, 1 month ago
L1 table entries have a field to store the offset of an L2 table.
The rest of the bits of the entry are currently reserved except from
bit 63, which stores the COPIED flag.

The offset is always taken from the entry using L1E_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 L1 table, it is
never used when we write to it. Due to the limits set elsewhere in the
code QEMU can never produce L2 table offsets that don't fit in that
field so any such offset when allocating an L2 table would indicate a
bug in QEMU.

Signed-off-by: Alberto Garcia <berto@igalia.com>
---
 block/qcow2-cluster.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 30eca26c47..179aa2c728 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -285,6 +285,9 @@ static int l2_allocate(BlockDriverState *bs, int l1_index)
         goto fail;
     }
 
+    /* The offset must fit in the offset field of the L1 table entry */
+    assert((l2_offset & L1E_OFFSET_MASK) == l2_offset);
+
     /* If we're allocating the table at offset 0 then something is wrong */
     if (l2_offset == 0) {
         qcow2_signal_corruption(bs, true, -1, -1, "Preventing invalid "
-- 
2.11.0


Re: [Qemu-devel] [PATCH] qcow2: Assert that L2 table offsets fit in the L1 table
Posted by Eric Blake 5 years, 1 month ago
On 2/8/19 9:44 AM, Alberto Garcia wrote:
> L1 table entries have a field to store the offset of an L2 table.
> The rest of the bits of the entry are currently reserved except from
> bit 63, which stores the COPIED flag.
> 
> The offset is always taken from the entry using L1E_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 L1 table, it is
> never used when we write to it. Due to the limits set elsewhere in the
> code QEMU can never produce L2 table offsets that don't fit in that
> field so any such offset when allocating an L2 table would indicate a
> bug in QEMU.
> 
> Signed-off-by: Alberto Garcia <berto@igalia.com>
> ---
>  block/qcow2-cluster.c | 3 +++
>  1 file changed, 3 insertions(+)

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

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

Re: [Qemu-devel] [PATCH] qcow2: Assert that L2 table offsets fit in the L1 table
Posted by Kevin Wolf 5 years, 1 month ago
Am 08.02.2019 um 16:44 hat Alberto Garcia geschrieben:
> L1 table entries have a field to store the offset of an L2 table.
> The rest of the bits of the entry are currently reserved except from
> bit 63, which stores the COPIED flag.
> 
> The offset is always taken from the entry using L1E_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 L1 table, it is
> never used when we write to it. Due to the limits set elsewhere in the
> code QEMU can never produce L2 table offsets that don't fit in that
> field so any such offset when allocating an L2 table would indicate a
> bug in QEMU.
> 
> Signed-off-by: Alberto Garcia <berto@igalia.com>

Thanks, applied to the block branch.

Kevin

Re: [Qemu-devel] [PATCH] qcow2: Assert that L2 table offsets fit in the L1 table
Posted by Alberto Garcia 5 years, 1 month ago
ping

On Fri 08 Feb 2019 04:44:53 PM CET, Alberto Garcia wrote:
> L1 table entries have a field to store the offset of an L2 table.
> The rest of the bits of the entry are currently reserved except from
> bit 63, which stores the COPIED flag.
>
> The offset is always taken from the entry using L1E_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 L1 table, it is
> never used when we write to it. Due to the limits set elsewhere in the
> code QEMU can never produce L2 table offsets that don't fit in that
> field so any such offset when allocating an L2 table would indicate a
> bug in QEMU.
>
> Signed-off-by: Alberto Garcia <berto@igalia.com>
> ---
>  block/qcow2-cluster.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
> index 30eca26c47..179aa2c728 100644
> --- a/block/qcow2-cluster.c
> +++ b/block/qcow2-cluster.c
> @@ -285,6 +285,9 @@ static int l2_allocate(BlockDriverState *bs, int l1_index)
>          goto fail;
>      }
>  
> +    /* The offset must fit in the offset field of the L1 table entry */
> +    assert((l2_offset & L1E_OFFSET_MASK) == l2_offset);
> +
>      /* If we're allocating the table at offset 0 then something is wrong */
>      if (l2_offset == 0) {
>          qcow2_signal_corruption(bs, true, -1, -1, "Preventing invalid "
> -- 
> 2.11.0