qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Kevin Wolf <kwolf@redhat.com>
To: Stefan Hajnoczi <stefanha@redhat.com>
Cc: qemu-devel@nongnu.org, jcody@redhat.com,
	Markus Armbruster <armbru@redhat.com>,
	Max Reitz <mreitz@redhat.com>
Subject: Re: [Qemu-devel] [PATCH RFC 2/2] block: Warn on insecure format probing
Date: Fri, 31 Oct 2014 12:56:39 +0100	[thread overview]
Message-ID: <20141031115639.GD4496@noname.str.redhat.com> (raw)
In-Reply-To: <20141031112423.GE10332@stefanha-thinkpad.redhat.com>

[-- Attachment #1: Type: text/plain, Size: 2082 bytes --]

Am 31.10.2014 um 12:24 hat Stefan Hajnoczi geschrieben:
> On Thu, Oct 30, 2014 at 10:36:35AM +0100, Kevin Wolf wrote:
> > Am 30.10.2014 um 10:27 hat Stefan Hajnoczi geschrieben:
> > > The guest may legitimately use raw devices that contain image format
> > > data.  Imagine tools similar to libguestfs.
> > > 
> > > It's perfectly okay for them to lay out image format data onto a raw
> > > device.
> > > 
> > > Probing is the problem, not putting image format data onto a raw device.
> > 
> > Agreed, that's why any restrictions only apply when probing was used to
> > detect a raw image. If you want to do anything exotic like storing a
> > qcow2 image for nested virt on a disk that is a raw image in the host,
> > then making sure to pass format=raw shouldn't be too much.
> 
> Because at that point the solution is way over-engineered.
> 
> Probing checks should be in the QEMU command-line code, not sprinkled
> across the codebase and even at run-time.
> 
> Isn't Markus approach much simpler and cleaner?

I don't think so. My code isn't "sprinkled across the codebase", it has
the checks right where the problem arises, in the raw block driver.

It's with Markus's approach that we'll have to have code in many
different places as I showed. Its fundamental assumption that there is
always a filename string and the filename isn't passed in some QDict
option is simply wrong. Specifying the image is driver-dependent and
therefore you'd have to add functionality to each driver in order to get
the filename extension (or the information that there isn't anything
close enough to a filename).

The only argument brought up so far that I can reasonably buy is that
in the unlikely case of the restrictions applying it may be surprising
for the user to see requests failing. To address this, we could print
a warning when an image is opened in the "restricted raw" mode. This
way the user knows what's going on, and at the same time we still
effectively protect them instead of only printing a warning without real
protection.

Kevin

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

  reply	other threads:[~2014-10-31 15:04 UTC|newest]

Thread overview: 63+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-10-28 16:03 [Qemu-devel] [PATCH RFC 0/2] block: Warn on insecure format probing Markus Armbruster
2014-10-28 16:03 ` [Qemu-devel] [PATCH RFC 1/2] block: Factor bdrv_probe_all() out of find_image_format() Markus Armbruster
2014-10-28 16:34   ` Eric Blake
2014-10-28 16:41   ` Jeff Cody
2014-10-29 15:22   ` Stefan Hajnoczi
2014-10-28 16:03 ` [Qemu-devel] [PATCH RFC 2/2] block: Warn on insecure format probing Markus Armbruster
2014-10-28 17:02   ` Eric Blake
2014-10-28 18:29     ` Jeff Cody
2014-10-28 18:56       ` Eric Blake
2014-10-28 19:42         ` Jeff Cody
2014-10-29  7:36           ` Markus Armbruster
2014-10-29  8:25             ` Max Reitz
2014-10-29  7:22         ` Markus Armbruster
2014-10-30 13:58           ` Jeff Cody
2014-11-03  8:07             ` Markus Armbruster
2014-10-29  7:03     ` Markus Armbruster
2014-10-28 18:33   ` Jeff Cody
2014-10-29  6:37     ` Markus Armbruster
2014-10-30 13:52       ` Jeff Cody
2014-11-03  8:11         ` Markus Armbruster
2014-10-29  1:16   ` Fam Zheng
2014-10-29  6:32     ` Markus Armbruster
2014-10-29 10:12   ` Kevin Wolf
2014-10-29 13:54     ` Markus Armbruster
2014-10-29 15:34       ` Stefan Hajnoczi
2014-10-30  9:07         ` Markus Armbruster
2014-10-30  9:24           ` Stefan Hajnoczi
2014-10-30 12:19             ` Markus Armbruster
2014-10-30  9:30       ` Kevin Wolf
2014-10-30 12:49         ` Markus Armbruster
2014-10-31 11:19           ` Stefan Hajnoczi
2014-10-31 22:45           ` Eric Blake
2014-11-03  8:15             ` Markus Armbruster
2014-11-03 11:13             ` Kevin Wolf
2014-11-04  9:36               ` Markus Armbruster
2014-11-04 10:32                 ` Kevin Wolf
2014-11-05  7:58                   ` Markus Armbruster
2014-11-03 11:00           ` Kevin Wolf
2014-11-04  9:39             ` Markus Armbruster
2014-11-04 16:09               ` Jeff Cody
2014-11-05  8:05                 ` Markus Armbruster
2014-11-05  8:09                   ` Max Reitz
2014-10-30  9:08   ` Max Reitz
2014-10-30  9:27     ` Stefan Hajnoczi
2014-10-30  9:36       ` Kevin Wolf
2014-10-31 11:24         ` Stefan Hajnoczi
2014-10-31 11:56           ` Kevin Wolf [this message]
2014-11-03  8:54             ` Markus Armbruster
2014-11-03  9:11               ` Max Reitz
2014-11-04  9:34                 ` Markus Armbruster
2014-11-03 10:25               ` Kevin Wolf
2014-11-03 15:05                 ` Stefan Hajnoczi
2014-11-03 15:13                   ` Max Reitz
2014-11-04  9:42                   ` Markus Armbruster
2014-11-04 10:11                   ` Kevin Wolf
2014-11-04 15:25                     ` Stefan Hajnoczi
2014-11-04 15:37                       ` Kevin Wolf
2014-11-05  8:39                         ` Markus Armbruster
2014-11-05 10:18                           ` Eric Blake
2014-10-30 13:02     ` Markus Armbruster
2014-11-03  8:44       ` Max Reitz
2014-10-30 20:45   ` Richard W.M. Jones
2014-11-03  8:18     ` Markus Armbruster

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=20141031115639.GD4496@noname.str.redhat.com \
    --to=kwolf@redhat.com \
    --cc=armbru@redhat.com \
    --cc=jcody@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    /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).