From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53175) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WTWHB-0004he-St for qemu-devel@nongnu.org; Fri, 28 Mar 2014 08:52:55 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WTWH5-0000n6-LH for qemu-devel@nongnu.org; Fri, 28 Mar 2014 08:52:49 -0400 Date: Fri, 28 Mar 2014 08:52:37 -0400 From: Jeff Cody Message-ID: <20140328125237.GH21833@localhost.localdomain> References: <1395835569-21193-1-git-send-email-stefanha@redhat.com> <1395835569-21193-17-git-send-email-stefanha@redhat.com> <53331AA9.5040506@weilnetz.de> <20140327185242.GE21833@localhost.localdomain> <533482EE.2060609@weilnetz.de> <20140328090722.GD11688@stefanha-thinkpad.redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20140328090722.GD11688@stefanha-thinkpad.redhat.com> 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) List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: Kevin Wolf , Stefan Weil , pmatouse@redhat.com, qemu-devel@nongnu.org, qemu-stable@nongnu.org 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.