qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Iustin Pop <iusty@k1024.org>
To: Kevin Wolf <kwolf@redhat.com>
Cc: Michael Tokarev <mjt@tls.msk.ru>, qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH] block: handle filenames with colons better
Date: Fri, 17 Aug 2012 12:05:33 +0200	[thread overview]
Message-ID: <20120817100533.GA4300@teal.hq.k1024.org> (raw)
In-Reply-To: <502DF933.9010501@redhat.com>

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

  reply	other threads:[~2012-08-17 10:05 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-08-16 14:58 [Qemu-devel] [PATCH] block: handle filenames with colons better Iustin Pop
2012-08-16 19:24 ` Michael Tokarev
2012-08-17  7:15   ` Iustin Pop
2012-08-17  7:56     ` Kevin Wolf
2012-08-17 10:05       ` Iustin Pop [this message]
2012-08-17 10:15         ` Kevin Wolf
2012-08-17 22:39           ` Iustin Pop

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20120817100533.GA4300@teal.hq.k1024.org \
    --to=iusty@k1024.org \
    --cc=kwolf@redhat.com \
    --cc=mjt@tls.msk.ru \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).