Anthony Liguori<address@hidden> writes:
On 07/15/2010 10:19 AM, Markus Armbruster wrote:
Anthony Liguori<address@hidden> writes:
On 07/14/2010 01:43 PM, Christoph Hellwig wrote:
Err, strong NACK. Please don't start messing with the contents of the
data plane, we're getting into real trouble there. It's perfectly
valid for a guest to create an image inside an image, and with hardware
support for nested virtualization I guess this use case will become
rather common, just as it already is on S/390 with VM.
Then we have to remove block format probing.
The two things are fundamentally incompatible.
I agree with Christoph: changing guest writes is a big no-no, and
changing them silently is even worse.
I do sympathize. The problem is we're already doing this. This patch
simply changes the behavior to not be a security problem. I've
committed it to attempt to resolve that security problem. However, we
still have a problem and I don't consider the issue closed.
I could perhaps accept EIO. Elsewhere in this thread you wrote that you
rejected that approach because "it would trigger the stop-on-error
behavior and the result would be far too difficult for a management
tool/person to deal with." I think that would be *far* superior in
fact: it fails spectacularly, immediately and safely instead of silently
corrupting disk contents.
There's really nothing wrong with this type of write, so EIO doesn't
solve the problem. While we can argue whether writing zeros or EIO is
a "better bad" solution, let's try to figure out a good solution.
The real problem in need of fixing is the unsafe default. You wrote
that "most users want block probing". I disagree. Users want to set up
drives with as little hassle as possible. If format is optional, and
appears to work, why bother specifying it?
I really think specifying the format is a burden that is nice to avoid.
Yes, users don't like having to specify the "obvious".
I have another idea that I hope will solve the problem in a more
complete way. The fundamental issue is that it's impossible to probe
raw images reliably. We can probe qcow2, vmdk, etc but not raw.
So, let's do the following: have raw_probe() always fail. Probing
shouldn't be a heuristic, it should be an absolute. We can't prove
it's a raw image, so we should always fail.
Note: if we stop right here, the security hole is patched, but use of
raw images requires explicit specification of format.
To accomodate current use-cases with raw, let's introduce a new format
called "probed_raw". probed_raw's semantics will be the following:
The signature of a probed_raw will be ~{'QFI\xfb', 'VMDK', 'COWD',
OOOM', ...}. If the signature is 'QRAW', then instead of reading the
first sector at offset 0, we read the first sector at offset LENGTH.
If the signature is 'QRAW', LENGTH is computed by calculating
FILE_SIZE - 512.
For probed_raw, write requests to sector 0 are checked. If the first
four bytes is an invalid probed_raw signature or QRAW, we write a QRAW
signature to file offset 0 and copy the first sector to the end of the
file redirecting reads and writes to the end of file.
Doesn't this require an image that can grow? What about host block
devices?
An approach like this has the following properties:
1) We can make the bdrv_probe check 100% reliable and return a boolean.
2) In the cases where we known format=raw, none of this code is ever
invoked.
3) probed_raw images usually look exactly like raw images in most cases
4) In the degenerate cases, probe_raw images are still mountable in
the normal way.
5) Even after the QRAW signature is applied, if the guest writes a
valid signature, we can truncate the file and make it appear as a
normal raw image.
Christoph/Markus/Stefan, does this seem like a more reasonable approach?
I'm not convinced it's a good idea. It's clearly a less bad idea,
though :)
It avoids guest-visible lossage, and that's good.
There's still host-visible lossage: as soon as we redirect sector 0, the
image isn't raw anymore, and accessing it with non-qemu tools (say
losetup + kpartx) no longer works. You need to know what QEMU did to
your no-longer-raw image to work around the lossage (say losetup -o
512).