Re: [Qemu-devel] [PATCH] block: handle filenames with colons better

[Kevin Wolf]

Am 17.08.2012 09:15, schrieb Iustin Pop:
> On Thu, Aug 16, 2012 at 11:24:11PM +0400, Michael Tokarev wrote:
>> On 16.08.2012 18:58, Iustin Pop wrote:
>>> Commit 947995c (block: protect path_has_protocol from filenames with
>>> colons) introduced a way to handle filenames with colons based on
>>> whether the path contains a slash or not. IMHO this is not optimal,
>>> since we shouldn't rely on the contents of the path but rather on
>>> whether the given path exists as a file or not.
>>>
>>> As such, this patch tries to handle both files with and without
>>> slashes by falling back to opening them as files if no drivers
>>> supporting the protocol has been identified.
>>
>> I for one dislike this idea entirely: I think there should be a
>> way to stop qemu from trying to open something as a file.  It
>> opens a security hole after all, "what if" such a file will actually
>> exist?
> 
> I'm not sure I understand the concern here. You pass what is a file path
> (and not an existing protocol path), and you want qemu not to open it?
> Or are you worried that a typo in the protocol name can lead to attacks?

That, or just the fact that you don't know exactly which protocols are
supported by this qemu binary and expect one that isn't there.

The other concern I have is about the opposite direction: When a new
qemu version introduces a new protocol, the same string that meant a
file before would start meaning the protocol, which makes it an
incompatible change. Essentially, that would mean that we can't add any
new protocols any more. Doesn't sound like a great idea to me.

>> If I can vote, I'm voting against this with both hands.
> 
> It's fine to have a way to stop QEMU opening something as a file, but
> please tell me how I can make it so that "qemu -hda x:0" works for both
> regular files and block/char devices. Right now, it behaves differently
> for these two, and from the code it looks like this difference is rather
> accidental than intentional.

It was probably accidental in the beginning, but it's now a well-known
misfeature that we can't get rid of for compatibility reasons. People
rely on devices with colons being accessible this way. (We once changed
that, but had to take this part back)

Kevin