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

[Kevin Wolf]

Am 17.08.2012 12:05, schrieb Iustin Pop:
> On Fri, Aug 17, 2012 at 09:56:35AM +0200, Kevin Wolf wrote:
>> 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.
> 
> So in this sense, you prefer to have it remain so that files with colons
> are not accepted by qemu, if I understand you correctly, so that new
> procotol can be added, right?

The current rule is that protocols never contain slashes (and we won't
add such protocols), and therefore anything containing a slash is a file
name. I already felt a bit uncomfortable with that, but at least it
introduces no ambiguity.

You're implying that you can't use files that contain a colon. That's
not true because you can include a slash in every file name. Instead of
"foo:bar" write "./foo:bar" and you get it interpreted as you wanted.

>>>> 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)
> 
> OK, then I think what is needed is to update the documentation then.
> I was not able to find any restrictions on the file names, hence this
> patch.

Sure, that makes sense. Care to send a patch?

> I think what is needed is to actually add a new 'file:' protocol, that
> can open paths containing no matter what characters they contain (as
> long as the filesystem supports them, of course); as the current
> situation where files have no protocol but all the other do seems
> asymmetric.

This is what I've been advocating before commit 947995c, but I think
other people had concerns (that I can't remember now).

Kevin