[Qemu-devel] [PATCH for-2.3 0/1] block: New command line option --misc format-probing=off

[Qemu-devel] [PATCH for-2.3 0/1] block: New command line option --misc format-probing=off

[Markus Armbruster]

First of all, my apologies for being so late with this.  I realized
part way through the current development cycle that I couldn't do both
the error work and my half of the block probing work we discussed back
in November, so I punted the latter to the next cycle, missing the one
little feature I quite obviously could do.

Why 2.3?

1. libvirt wants it, the sooner, the better.  See the libvirt RFC PATCH
   https://lists.nongnu.org/archive/html/qemu-devel/2015-03/msg04457.html

2. The patch is simple, and quite obviously does nothing unless you
   run with --misc format-probing=off.

Markus Armbruster (1):
  block: New command line option --misc format-probing=off

 block.c               |  6 ++++++
 include/block/block.h |  2 ++
 qemu-options.hx       | 15 +++++++++++++++
 util/qemu-config.c    |  2 +-
 vl.c                  | 22 ++++++++++++++++++++++
 5 files changed, 46 insertions(+), 1 deletion(-)

-- 
1.9.3

[Qemu-devel] [PATCH for-2.3 1/1] block: New command line option --misc format-probing=off

[Markus Armbruster]

Probing is convenient, but probing untrusted raw images is insecure
(CVE-2008-2004).  To avoid it, users should always specify raw format
explicitly.  This isn't trivial, and even sophisticated users have
gotten it wrong (libvirt CVE-2010-2237, CVE-2010-2238, CVE-2010-2239,
plus more recent variations of the theme that didn't get CVEs because
they were caught before they could hurt users).

Disabling probing entirely is a (hamfisted) way to ensure you always
specify the format.

Instead of creating yet another simple option that doesn't work with
-readconfig, create a "misc" option group and --misc command line
option.  We're out of space in vm_config_groups[], so double it.

This will let us make existing miscellaneous non-QemeOpts options
sugar for --misc, so they become available with -readconfig.  Left for
another day.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 block.c               |  6 ++++++
 include/block/block.h |  2 ++
 qemu-options.hx       | 15 +++++++++++++++
 util/qemu-config.c    |  2 +-
 vl.c                  | 22 ++++++++++++++++++++++
 5 files changed, 46 insertions(+), 1 deletion(-)

diff --git a/block.c b/block.c
index 0fe97de..fe65aeb 100644
--- a/block.c
+++ b/block.c
@@ -103,6 +103,7 @@ static void bdrv_reset_dirty(BlockDriverState *bs, int64_t cur_sector,
                              int nr_sectors);
 /* If non-zero, use only whitelisted block drivers */
 static int use_bdrv_whitelist;
+bool bdrv_image_probing_disabled;
 
 #ifdef _WIN32
 static int is_windows_drive_prefix(const char *filename)
@@ -751,6 +752,11 @@ static int find_image_format(BlockDriverState *bs, const char *filename,
         return ret;
     }
 
+    if (bdrv_image_probing_disabled) {
+        error_setg(errp, "Format not specified and image probing disabled");
+        return -EINVAL;
+    }
+
     ret = bdrv_pread(bs, 0, buf, sizeof(buf));
     if (ret < 0) {
         error_setg_errno(errp, -ret, "Could not read image for determining its "
diff --git a/include/block/block.h b/include/block/block.h
index 4c57d63..3485b9b 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -162,6 +162,8 @@ typedef enum BlockOpType {
     BLOCK_OP_TYPE_MAX,
 } BlockOpType;
 
+extern bool bdrv_image_probing_disabled;
+
 void bdrv_iostatus_enable(BlockDriverState *bs);
 void bdrv_iostatus_reset(BlockDriverState *bs);
 void bdrv_iostatus_disable(BlockDriverState *bs);
diff --git a/qemu-options.hx b/qemu-options.hx
index 319d971..b6cdae2 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -963,6 +963,21 @@ STEXI
 Disable SDL window close capability.
 ETEXI
 
+DEF("misc", HAS_ARG, QEMU_OPTION_misc,
+    "-misc [format-probing=on|off]\n", QEMU_ARCH_ALL)
+STEXI
+@item -misc
+@findex -misc @var{name}[=@var{value},...
+Miscellaneous settings:
+@table @option
+@item format-probing=on|off
+Enable or disable block image format probing.  Default is enable.
+Probing is convenient, but probing untrusted raw images is insecure.
+To avoid it, always specify raw format explicitly.  Disabling probing
+entirely is a (hamfisted) way to ensure you do.
+@end table
+ETEXI
+
 DEF("sdl", 0, QEMU_OPTION_sdl,
     "-sdl            enable SDL\n", QEMU_ARCH_ALL)
 STEXI
diff --git a/util/qemu-config.c b/util/qemu-config.c
index f3463df..a35cb32 100644
--- a/util/qemu-config.c
+++ b/util/qemu-config.c
@@ -7,7 +7,7 @@
 #include "qapi/error.h"
 #include "qmp-commands.h"
 
-static QemuOptsList *vm_config_groups[32];
+static QemuOptsList *vm_config_groups[64];
 static QemuOptsList *drive_config_groups[4];
 
 static QemuOptsList *find_list(QemuOptsList **lists, const char *group,
diff --git a/vl.c b/vl.c
index 75ec292..991d86c 100644
--- a/vl.c
+++ b/vl.c
@@ -490,6 +490,18 @@ static QemuOptsList qemu_semihosting_config_opts = {
     },
 };
 
+static QemuOptsList qemu_misc_opts = {
+    .name = "misc",
+    .head = QTAILQ_HEAD_INITIALIZER(qemu_misc_opts.head),
+    .desc = {
+        {
+            .name = "format-probing",
+            .type = QEMU_OPT_BOOL,
+        },
+        { /* end of list */ }
+    },
+};
+
 /**
  * Get machine options
  *
@@ -2806,6 +2818,7 @@ int main(int argc, char **argv, char **envp)
     qemu_add_opts(&qemu_numa_opts);
     qemu_add_opts(&qemu_icount_opts);
     qemu_add_opts(&qemu_semihosting_config_opts);
+    qemu_add_opts(&qemu_misc_opts);
 
     runstate_init();
 
@@ -3381,6 +3394,12 @@ int main(int argc, char **argv, char **envp)
             case QEMU_OPTION_no_quit:
                 no_quit = 1;
                 break;
+            case QEMU_OPTION_misc:
+                opts = qemu_opts_parse(qemu_find_opts("misc"), optarg, 0);
+                if (!opts) {
+                    exit(1);
+                }
+                break;
             case QEMU_OPTION_sdl:
 #ifdef CONFIG_SDL
                 display_type = DT_SDL;
@@ -4158,6 +4177,9 @@ int main(int argc, char **argv, char **envp)
     }
 
     /* open the virtual block devices */
+    bdrv_image_probing_disabled =
+        !qemu_opt_get_bool(qemu_opts_find(qemu_find_opts("misc"), NULL),
+                           "format-probing", true);
     if (snapshot)
         qemu_opts_foreach(qemu_find_opts("drive"), drive_enable_snapshot, NULL, 0);
     if (qemu_opts_foreach(qemu_find_opts("drive"), drive_init_func,
-- 
1.9.3

Re: [Qemu-devel] [PATCH for-2.3 1/1] block: New command line option --misc format-probing=off

[Eric Blake]

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

On 03/23/2015 04:04 AM, Markus Armbruster wrote:
> Probing is convenient, but probing untrusted raw images is insecure
> (CVE-2008-2004).  To avoid it, users should always specify raw format
> explicitly.  This isn't trivial, and even sophisticated users have
> gotten it wrong (libvirt CVE-2010-2237, CVE-2010-2238, CVE-2010-2239,
> plus more recent variations of the theme that didn't get CVEs because
> they were caught before they could hurt users).
> 
> Disabling probing entirely is a (hamfisted) way to ensure you always
> specify the format.
> 
> Instead of creating yet another simple option that doesn't work with
> -readconfig, create a "misc" option group and --misc command line
> option.  We're out of space in vm_config_groups[], so double it.
> 
> This will let us make existing miscellaneous non-QemeOpts options

s/Qeme/Qemu/
maintainer can fix this

> sugar for --misc, so they become available with -readconfig.  Left for
> another day.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  block.c               |  6 ++++++
>  include/block/block.h |  2 ++
>  qemu-options.hx       | 15 +++++++++++++++
>  util/qemu-config.c    |  2 +-
>  vl.c                  | 22 ++++++++++++++++++++++
>  5 files changed, 46 insertions(+), 1 deletion(-)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>

Also, I made sure that query-command-line-options includes this
(slightly prettified):

 {"parameters": [{"name": "format-probing",
 "type": "boolean"}],
 "option": "misc"},

which I then paired with my pending libvirt patch; as well as
experimentation with giving images without a format, all to give:

Tested-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH for-2.3 1/1] block: New command line option --misc format-probing=off

[Paolo Bonzini]

On 23/03/2015 11:04, Markus Armbruster wrote:
> Probing is convenient, but probing untrusted raw images is insecure
> (CVE-2008-2004).  To avoid it, users should always specify raw format
> explicitly.  This isn't trivial, and even sophisticated users have
> gotten it wrong (libvirt CVE-2010-2237, CVE-2010-2238, CVE-2010-2239,
> plus more recent variations of the theme that didn't get CVEs because
> they were caught before they could hurt users).
> 
> Disabling probing entirely is a (hamfisted) way to ensure you always
> specify the format.
> 
> Instead of creating yet another simple option that doesn't work with
> -readconfig, create a "misc" option group and --misc command line
> option.  We're out of space in vm_config_groups[], so double it.
> 
> This will let us make existing miscellaneous non-QemeOpts options
> sugar for --misc, so they become available with -readconfig.  Left for
> another day.

Which exactly?  Could they fit into another scheme?  (See how
-mem-prealloc was replaced and generalized by memory-backend-* objects).

For example, -win2k-install-hack should really be an IDE disk property
that can be set with -global, and many other options could be machine or
display options.

I don't think it's the right solution.  Libvirt knows where to add a
format=raw option, and it can do it without waiting for QEMU to
implement this.  Direct command-line users are not going to use the
option anyway.

So for today we're 1-1 on NACKs. :D

Paolo

Re: [Qemu-devel] [PATCH for-2.3 1/1] block: New command line option --misc format-probing=off

[Markus Armbruster]

Paolo Bonzini <pbonzini@redhat.com> writes:

> On 23/03/2015 11:04, Markus Armbruster wrote:
>> Probing is convenient, but probing untrusted raw images is insecure
>> (CVE-2008-2004).  To avoid it, users should always specify raw format
>> explicitly.  This isn't trivial, and even sophisticated users have
>> gotten it wrong (libvirt CVE-2010-2237, CVE-2010-2238, CVE-2010-2239,
>> plus more recent variations of the theme that didn't get CVEs because
>> they were caught before they could hurt users).
>> 
>> Disabling probing entirely is a (hamfisted) way to ensure you always
>> specify the format.
>> 
>> Instead of creating yet another simple option that doesn't work with
>> -readconfig, create a "misc" option group and --misc command line
>> option.  We're out of space in vm_config_groups[], so double it.
>> 
>> This will let us make existing miscellaneous non-QemeOpts options
>> sugar for --misc, so they become available with -readconfig.  Left for
>> another day.
>
> Which exactly?  Could they fit into another scheme?  (See how
> -mem-prealloc was replaced and generalized by memory-backend-* objects).
>
> For example, -win2k-install-hack should really be an IDE disk property
> that can be set with -global, and many other options could be machine or
> display options.
>
> I don't think it's the right solution.  Libvirt knows where to add a
> format=raw option, and it can do it without waiting for QEMU to
> implement this.  Direct command-line users are not going to use the
> option anyway.

Two separate bones of contention here:

1. Do we want to give libvirt the bug insurance it wants?

2. Is --misc sane?

We're discussing 1. elsewhere already.

Regarding 2.: if anyone has a better idea on how to do the command line
switch, I'm all ears.

Eyeballing vl.c, I suspect these options don't use QemuOpts, thus don't
support -readconfig:

    nodefconfig
    nouserconfig
    cpu
    snapshot
    display
    nographic
    curses
    portrait
    rotate
    no-fd-bootchk
    tftp
    bootp
    redir
    audio_help
    soundhw
    help
    version
    mempath
    mem-prealloc
    d
    D
    s
    L
    singlestep
    S
    k
    localtime
    vga
    g
    echr
    watchdog
    watchdog-action
    loadvm
    full-screen
    no-frame
    alt-grab
    ctrl-grab
    no-quit
    sdl
    pidfile
    win2k-hack
    rtc-td-hack
    no-kvm-pit-reinjection
    no-acpi
    no-hpet
    no-reboot
    no-shutdown
    show-cursor
    uuid
    semihosting
    prom-env
    startdate
    tb-size
    incoming
    nodefaults
    xen-domid
    xen-attach
    qtest
    qtest-log
    dump-vmstate
    smb
    runas
    chroot
    daemonize
    enable-fips

Unless we stop adding more, we'll never get --readconfig reasonably
complete.

>
> So for today we're 1-1 on NACKs. :D

I NACKed something today?

All I remember is advising to disable sdhci-pci instead of changing how
it's hacked up.

Re: [Qemu-devel] [PATCH for-2.3 0/1] block: New command line option --misc format-probing=off

[Peter Maydell]

On 23 March 2015 at 10:04, Markus Armbruster <armbru@redhat.com> wrote:
> First of all, my apologies for being so late with this.  I realized
> part way through the current development cycle that I couldn't do both
> the error work and my half of the block probing work we discussed back
> in November, so I punted the latter to the next cycle, missing the one
> little feature I quite obviously could do.
>
> Why 2.3?
>
> 1. libvirt wants it, the sooner, the better.  See the libvirt RFC PATCH
>    https://lists.nongnu.org/archive/html/qemu-devel/2015-03/msg04457.html
>
> 2. The patch is simple, and quite obviously does nothing unless you
>    run with --misc format-probing=off.

I'm really dubious about adding new commandline ABI at this point
in the release cycle, especially a whole new option group...

-- PMM

Re: [Qemu-devel] [PATCH for-2.3 1/1] block: New command line option --misc format-probing=off

[Eric Blake]

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

On 03/23/2015 02:42 PM, Markus Armbruster wrote:

>>
>> I don't think it's the right solution.  Libvirt knows where to add a
>> format=raw option, and it can do it without waiting for QEMU to
>> implement this.  Direct command-line users are not going to use the
>> option anyway.
> 
> Two separate bones of contention here:
> 
> 1. Do we want to give libvirt the bug insurance it wants?

If we add this in 2.3, libvirt will use it.  If we wait until 2.4,
libvirt will manage without (as it has already managed without for all
earlier releases); it is just a little bit harder to ensure that formats
are being uniformly used.  I'm okay if this topic proves too
controversial to add into 2.3 at this late in the cycle, even though I'm
in favor of adding it.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]