Re: [Qemu-devel] [PATCH RFC 2/2] block: Warn on insecure format probing

[Kevin Wolf]

Am 29.10.2014 um 14:54 hat Markus Armbruster geschrieben:
> Kevin Wolf <address@hidden> writes:
> > Instead, let me try once more to sell my old proposal [1] from the
> > thread you mentioned:
> >
> >> What if we let the raw driver know that it was probed and then it
> >> enables a check that returns -EIO for any write on the first 2k if that
> >> write would make the image look like a different format?
> >
> > Attacks the problem where it arises instead of trying to detect the
> > outcome of it, and works in whatever way it is nested in the BDS graph
> > and whatever way is used to address the image file.
> >
> > Kevin
> >
> > [1] https://lists.nongnu.org/archive/html/qemu-devel/2014-08/msg02548.html
> 
> Arbitrarily breaking the virtual hardware that way is incompatible, too.
> It's a bigger no-no for me than changing user interface corner cases or
> deciding what to do with a file based on its file name extension.

It is arbitrarily breaking theoretical cases, while your patch is less
arbitrarily breaking common cases. I strongly prefer the former.

Nobody will ever want to write a qcow2 header into their boot sector for
just running a guest:

0:   51                      push   %cx
1:   46                      inc    %si
2:   49                      dec    %cx
3:   fb                      sti

This code doesn't make any sense. It's similar for other formats. And if
they really have some odd reason to do that, the fix is easy: Either
don't use raw, or pass an explicit format=raw.

> Anthony tried something similar (commit 79368c8), but couldn't get it
> right (commit 8b33d9e).

The discussion back then: http://patchwork.ozlabs.org/patch/58980/

The problem with Anthony's code was that he didn't handle a qiov
correctly that had unaligned members. With today's block layer, this is
not a big deal to implement correctly. We're running coroutines instead
of AIO callbacks and we don't have to do all the manual qiov fixing
magic that Anthony had in his patch, util/iov.c provides all you need.

I'll send out an RFC series that implements this.

Kevin