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

[Markus Armbruster]

Eric Blake <address@hidden> writes:

> On 10/30/2014 06:49 AM, Markus Armbruster wrote:
>
>> You either have to prevent *any* writing of the first 2048 bytes (the
>> part that can be examined by a bdrv_probe() method, or your have to
>> prevent writing anything a probe recognizes, or the user has to specify
>> the format explicitly.
>> 
>> If you do the former, you're way outside the realm of "theoretical".
>> 
>> If you do the latter, the degree of "theoreticalness" depends on the
>> union of the patterns you prevent.  Issues:
>> 
>> 1. Anthony's method of checking a few known signatures is fragile.  The
>> only obviously robust way to test "is probing going to find something
>> other than 'raw'?" is running the probes.  Feasible.
>> 
>> 2. Whether the union of patterns qualifies as "theoretical" for all our
>> targets is not obvious, and whether it'll remain "theoretical" for all
>> future formats and target machines even less.
>
> This one scares me.  The proof of concept patch you posted tests whether
> a write to the first sector would result in the sector matching a
> _currently known probe_ for the file formats that were compiled in at
> configure time to the currently running binary.  But this is NOT the set
> of all possible binary formats that may be introduced in the future.  By
> extrapolation, if we pursue write blocking, then the only SAFE way to is
> to reject ALL writes to the first sector, because we can't know which
> writes will match some theoretical future probe -

Yes!

>                                                   but by the time you
> get to that point, then we no longer match bare metal (rejecting ALL
> writes to the first sector is ridiculous).
>
>> 3. A write access that works just fine in one QEMU version can be
>> rejected by another version that recognizes more formats.  Or probes the
>> same formats differently.
>
> This is just a manifestation of my observation above, which can be
> easily triggered now by having two qemu binaries built from the same
> source but different compile options.

Throw in loadable modules, and it may even depend on runtime
configuration.

>> 4. Rejecting a write fails *late*, and looks like hardware failure to
>> the guest.  With imperfect guest software, this risks data corruption.
>> 
>> (4) is a deal breaker for me.
>
> It is mitigated somewhat by the fact that the proposed patch will NEVER
> reject writes for a management app that always correctly passes in a
> format and thus avoids probes.  But that is still fragile.

If I was content with a "specify format= explicitly and you'll be fine"
argument, we wouldn't have this discussion :)