[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH for-2.0 16/47] vdi: add bounds checks for blocks
From: |
Jeff Cody |
Subject: |
Re: [Qemu-devel] [PATCH for-2.0 16/47] vdi: add bounds checks for blocks_in_image and disk_size header fields (CVE-2014-0144) |
Date: |
Fri, 28 Mar 2014 08:52:37 -0400 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
On Fri, Mar 28, 2014 at 10:07:22AM +0100, Stefan Hajnoczi wrote:
> On Thu, Mar 27, 2014 at 08:58:38PM +0100, Stefan Weil wrote:
> > Am 27.03.2014 19:52, schrieb Jeff Cody:
> > >> Do we need this patch for QEMU 2.0? For 32 bit systems, the image size
> > >> limit is 1000 TB, and that image would need 4 GB for the block cache in
> > >> memory. Are such image sizes used anywhere? For 64 bit systems, the
> > >> limit will be much higher.
> > >>
> > >
> > > I don't know what systems are in use in the wild. But since we
> > > allocate block cache to fit the entire cache in RAM currently, that
> > > does open us up to potentially allocating a lot of memory, based on
> > > what the image file header specifies.
> > >
> > > That is a reason to keep the maximum blocks_in_image size bounded to
> > > the size of 0x3fffffff. With an unbound blocks_in_image size (except
> > > to UINT32_MAX), the driver would potentially attempt to allocate 16GB
> > > of RAM, if qemu attempts to open a VDI image file with such a header.
> >
> > Either we crash because of the 0x3fffffff limit, or we might crash
> > because a memory allocation fails (but it might also be successful). I
> > prefer this 2nd variant.
>
> From a user perspective, hotplugging a disk should never kill the VM.
> Instead the hotplug command should fail for invalid image files.
>
> If a valid image can cause QEMU abort then the block driver
> implementation needs to use a metadata cache to avoid putting everything
> in RAM.
>
We also don't know what a valid image really is. Regardless, it is
legitimate for QEMU to fail to open a valid image as 'unsupported', if
the current code is not able to reasonably handle the image.
On that note, I wonder if the disk size / blocks in image limit in my
original patch is too generous - it would still allow the allocation
of 4G of data for the block cache at the upper limit.
- Re: [Qemu-devel] [PATCH for-2.0 14/47] vpc/vhd: add bounds check for max_table_entries and block_size (CVE-2014-0144), (continued)
[Qemu-devel] [PATCH for-2.0 17/47] vhdx: Bounds checking for block_size and logical_sector_size (CVE-2014-0148), Stefan Hajnoczi, 2014/03/26
[Qemu-devel] [PATCH for-2.0 18/47] curl: check data size before memcpy to local buffer. (CVE-2014-0144), Stefan Hajnoczi, 2014/03/26
[Qemu-devel] [PATCH for-2.0 20/47] qcow2: Check backing_file_offset (CVE-2014-0144), Stefan Hajnoczi, 2014/03/26
[Qemu-devel] [PATCH for-2.0 22/47] qcow2: Validate refcount table offset, Stefan Hajnoczi, 2014/03/26
[Qemu-devel] [PATCH for-2.0 21/47] qcow2: Check refcount table size (CVE-2014-0144), Stefan Hajnoczi, 2014/03/26