qemu-block
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-block] [PATCH v2 1/2] block: vpc - prevent overflow if max_tab


From: Max Reitz
Subject: Re: [Qemu-block] [PATCH v2 1/2] block: vpc - prevent overflow if max_table_entries >= 0x40000000
Date: Wed, 22 Jul 2015 19:02:02 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.1.0

On 21.07.2015 18:13, Jeff Cody wrote:
When we allocate the pagetable based on max_table_entries, we multiply
the max table entry value by 4 to accomodate a table of 32-bit integers.
However, max_table_entries is a uint32_t, and the VPC driver accepts
ranges for that entry over 0x40000000.  So during this allocation:

s->pagetable = qemu_try_blockalign(bs->file, s->max_table_entries * 4);

The size arg overflows, allocating significantly less memory than
expected.

Since qemu_try_blockalign() size argument is size_t, cast the
multiplication correctly to prevent overflow.

The value of "max_table_entries * 4" is used elsewhere in the code as
well, so store the correct value for use in all those cases.

We also check the Max Tables Entries value, to make sure that it is <
SIZE_MAX / 4, so we know the pagetable size will fit in size_t.

Reported-by: Richard W.M. Jones <address@hidden>
Signed-off-by: Jeff Cody <address@hidden>
---
  block/vpc.c | 17 +++++++++++++----
  1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/block/vpc.c b/block/vpc.c
index 37572ba..6ef21e6 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -168,6 +168,7 @@ static int vpc_open(BlockDriverState *bs, QDict *options, 
int flags,
      uint8_t buf[HEADER_SIZE];
      uint32_t checksum;
      uint64_t computed_size;
+    uint64_t pagetable_size;
      int disk_type = VHD_DYNAMIC;
      int ret;

@@ -269,7 +270,16 @@ static int vpc_open(BlockDriverState *bs, QDict *options, 
int flags,
              goto fail;
          }

-        s->pagetable = qemu_try_blockalign(bs->file, s->max_table_entries * 4);
+        if (s->max_table_entries > SIZE_MAX / 4) {
+            error_setg(errp, "Max Table Entries too large (%" PRId32 ")",
+                        s->max_table_entries);
+            ret = -EINVAL;
+            goto fail;
+        }
+
+        pagetable_size = (uint64_t) s->max_table_entries * 4;
+
+        s->pagetable = qemu_try_blockalign(bs->file, pagetable_size);
          if (s->pagetable == NULL) {
              ret = -ENOMEM;
              goto fail;
@@ -277,14 +287,13 @@ static int vpc_open(BlockDriverState *bs, QDict *options, 
int flags,

          s->bat_offset = be64_to_cpu(dyndisk_header->table_offset);

-        ret = bdrv_pread(bs->file, s->bat_offset, s->pagetable,
-                         s->max_table_entries * 4);
+        ret = bdrv_pread(bs->file, s->bat_offset, s->pagetable, 
pagetable_size);

The last parameter of bdrv_pread() is an int, so this will still overflow if s->max_table_entries > INT_MAX / 4.

          if (ret < 0) {
              goto fail;
          }

          s->free_data_block_offset =
-            (s->bat_offset + (s->max_table_entries * 4) + 511) & ~511;
+            (s->bat_offset + pagetable_size + 511) & ~511;

Not necessary, but perhaps we should be using ROUND_UP() here...

Max

          for (i = 0; i < s->max_table_entries; i++) {
              be32_to_cpus(&s->pagetable[i]);





reply via email to

[Prev in Thread] Current Thread [Next in Thread]