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

[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?

> >> 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.

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.

thanks,
iustin