[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CA+1xoqe22z9F3ZyFG_XnV9Y17HtkeSxtf1gbW4+CW1cuNQ=N5g@mail.gmail.com>
Date: Thu, 10 Nov 2011 10:23:23 +0200
From: Sasha Levin <levinsasha928@...il.com>
To: Markus Armbruster <armbru@...hat.com>
Cc: Pekka Enberg <penberg@...nel.org>,
Anthony Liguori <anthony@...emonkey.ws>,
Pekka Enberg <penberg@...helsinki.fi>,
Linus Torvalds <torvalds@...ux-foundation.org>,
Avi Kivity <avi@...hat.com>,
Andrew Morton <akpm@...ux-foundation.org>,
Ingo Molnar <mingo@...e.hu>, linux-kernel@...r.kernel.org,
kvm@...r.kernel.org, Christoph Hellwig <hch@....de>
Subject: Re: [RFC/GIT PULL] Linux KVM tool for v3.2
On Thu, Nov 10, 2011 at 9:57 AM, Markus Armbruster <armbru@...hat.com> wrote:
> Pekka Enberg <penberg@...nel.org> writes:
>
>> Hi Anthony,
>>
>>> On 11/04/2011 03:38 AM, Pekka Enberg wrote:
>>>> Hi Linus,
>>>>
>>>> Please consider pulling the latest KVM tool tree from:
>>>>
>>>> git://git.kernel.org/pub/scm/linux/kernel/git/penberg/linux.git
>>>> kvmtool/for-linus
>>>>
>>>
>>> [snip]
>>>
>>>> tools/kvm/virtio/net.c | 423 ++++++++
>>>> tools/kvm/virtio/pci.c | 319 ++++++
>>>> tools/kvm/virtio/rng.c | 185 ++++
>>>> 186 files changed, 19071 insertions(+), 179 deletions(-)
>>
>> On Wed, 9 Nov 2011, Anthony Liguori wrote:
>>> So let's assume for a moment that a tool like this should live in
>>> the kernel. What's disturbing about a PULL request like this is the
>>> lack of reviewability of it and the lack of any real review from
>>> people that understand what's going on in this code base.
>>>
>>> There are no Acked-by's by people that really understand what the
>>> code is doing or that have domain expertise in filesystems and
>>> networking.
>>>
>>> There are major functionality short comings in this code base, data
>>> corruptors, and CVEs. I'm not saying that the kvm-tool developers
>>> are bad developers, but the code is not at the appropriate quality
>>> level for the kernel. It just looks pretty on the surface to people
>>> that are used to the kernel coding style.
>>>
>>> To highlight a few of the issues:
> [...]
>>> 2) The qcow2 code is a filesystem implemented in userspace. Image
>>> formats are file systems. It really should be reviewed by the
>>> filesystem maintainers. There is absolutely no attempt made to
>>> synchronize the metadata during write operations which means that
>>> you do not have crash consistency of the meta data.
>>>
>>> If you experience a power failure or kvm-tool crashs, your image
>>> will get corrupted. I highly doubt a file system would ever be
>>> merged into Linux that was this naive about data integrity.
>>
>> The QCOW2 is lagging behind because we lost the main developer. It's
>> forced as read-only for the issues you mention. If you think it's a
>> merge blocker, we can drop it completely from the tree until the
>> issues are sorted out.
>>
>> I personally don't see the issue of having it as a read-only filesystem.
>>
>>> 3) The block probing code replicates a well known CVE from three
>>> years ago[1]. Using kvm-tool, a malicious guest could write the
>>> qcow2 signature to the zero sector and use that to attack the host.
>>
>> We don't support QCOW2 snapshots so I don't see how the "arbitrary
>> file" thing can happen.
>
> You don't need snapshots for the hole.
>
> Start with a clean read/write raw image. Probing declares it raw.
> Guest writes QCOW signature to it, with a backing file of its choice.
>
> Restart with the same image. Probing declares it QCOW2. Guest can read
> the backing file. Oops.
Thats an excellent scenario why you'd want to have 'Secure KVM' with
seccomp filters :)
I'm actually not sure why KVM tool got QCOW support in the first
place. You can have anything QCOW provides if you use btrfs (among
several other FSs).
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists