From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=38043 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1OZ5wL-0003Eo-TW for qemu-devel@nongnu.org; Wed, 14 Jul 2010 13:40:15 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1OZ5wJ-0007yt-T8 for qemu-devel@nongnu.org; Wed, 14 Jul 2010 13:40:13 -0400 Received: from mail-vw0-f45.google.com ([209.85.212.45]:47774) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1OZ5wJ-0007y6-Q9 for qemu-devel@nongnu.org; Wed, 14 Jul 2010 13:40:11 -0400 Received: by vws13 with SMTP id 13so9626vws.4 for ; Wed, 14 Jul 2010 10:40:10 -0700 (PDT) Message-ID: <4C3DF681.7080304@codemonkey.ws> Date: Wed, 14 Jul 2010 12:40:17 -0500 From: Anthony Liguori MIME-Version: 1.0 Subject: Re: [Qemu-devel] Re: [PATCH] Make default invocation of block drivers safer References: <1279123952-1576-1-git-send-email-aliguori@us.ibm.com> <4C3DE8E3.8080709@redhat.com> In-Reply-To: <4C3DE8E3.8080709@redhat.com> Content-Type: text/plain; charset=ISO-8859-15; format=flowed Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: Anthony Liguori , qemu-devel@nongnu.org, Stefan Hajnoczi On 07/14/2010 11:42 AM, Kevin Wolf wrote: > Am 14.07.2010 18:12, schrieb Anthony Liguori: > >> CVE-2008-2004 described a vulnerability in QEMU whereas a malicious user could >> trick the block probing code into accessing arbitrary files in a guest. To >> mitigate this, we added an explicit format parameter to -drive which disabling >> block probing. >> >> Fast forward to today, and the vast majority of users do not use this parameter. >> libvirt does not use this by default nor does virt-manager. >> >> Most users want block probing so we should try to make it safer. >> >> This patch adds some logic to the raw device which attempts to detect a write >> operation to the beginning of a raw device. If the first 4 bytes happen to >> match an image file that has a backing file that we support, it scrubs the >> signature to all zeros. If a user specifies an explicit format parameter, this >> behavior is disabled. >> >> I contend that while a legitimate guest could write such a signature to the >> header, we would behave incorrectly anyway upon the next invocation of QEMU. >> This simply changes the incorrect behavior to not involve a security >> vulnerability. >> >> I've tested this pretty extensively both in the positive and negative case. I'm >> not 100% confident in the block layer's ability to deal with zero sized writes >> particularly with respect to the aio functions so some additional eyes would be >> appreciated. >> >> Even in the case of a single sector write, we have to make sure to invoked the >> completion from a bottom half so just removing the zero sized write is not an >> option. >> >> Signed-off-by: Anthony Liguori >> > I guess something like this makes sense, and the approach looks okay in > general. With the check that we have really probed the format, we still > allow legitimate use cases (whatever they might be). > > However, I wonder why you even bother with adjusting buffers and > requests and stuff instead of just returning a straight -EIO. Doing so > would have the additional advantage that the expectation of the guest OS > matches what is really on the disk (garbage) instead of silently > corrupting things. > I started with that approach. My concern is that it would trigger the stop-on-error behavior and the result would be far too difficult for a management tool/person to deal with. Scrubbing seemed like a easier-to-use solution. >> static BlockDriverAIOCB *raw_aio_writev(BlockDriverState *bs, >> int64_t sector_num, QEMUIOVector *qiov, int nb_sectors, >> BlockDriverCompletionFunc *cb, void *opaque) >> { >> + if (check_write_unsafe(bs, sector_num, qiov->iov[0].iov_base, nb_sectors)) { >> > Have you checked that the bad value is always in iov[0]? Could a guest > construct a zero-length iov[0] and do the bad access in iov[1]? Or use > two two-byte buffers to write the magic number? > > I'm not saying that any of these work, I honestly don't know, but did > you consider them? > No, and it's certainly worth being a bit more paranoid. Regards, Anthony Liguori > Kevin > >