Re: [Qemu-devel] [PATCH RFC for-2.3 1/1] block: New command line option --no-format-probing

[Max Reitz <mreitz@redhat.com>]

On 2015-03-20 at 09:05, 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.
>
> Unfortunately, the new option is not available with -readconfig.
> There's no obvious option group to take it.  I think we could use a
> "miscellaneous" option group.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>   block.c               |  9 ++++++++-
>   include/block/block.h |  2 +-
>   qemu-options.hx       | 12 ++++++++++++
>   vl.c                  |  6 +++++-
>   4 files changed, 26 insertions(+), 3 deletions(-)
>
> diff --git a/block.c b/block.c
> index 0fe97de..5865309 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;
> +static 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 "
> @@ -4909,9 +4915,10 @@ void bdrv_init(void)
>       module_call_init(MODULE_INIT_BLOCK);
>   }
>   
> -void bdrv_init_with_whitelist(void)
> +void bdrv_init_with_whitelist(bool no_format_probing)
>   {
>       use_bdrv_whitelist = 1;
> +    bdrv_image_probing_disabled = no_format_probing;
>       bdrv_init();
>   }
>   
> diff --git a/include/block/block.h b/include/block/block.h
> index 4c57d63..b5a8b23 100644
> --- a/include/block/block.h
> +++ b/include/block/block.h
> @@ -177,7 +177,7 @@ void bdrv_io_limits_enable(BlockDriverState *bs);
>   void bdrv_io_limits_disable(BlockDriverState *bs);
>   
>   void bdrv_init(void);
> -void bdrv_init_with_whitelist(void);
> +void bdrv_init_with_whitelist(bool no_format_probing);
>   BlockDriver *bdrv_find_protocol(const char *filename,
>                                   bool allow_protocol_prefix,
>                                   Error **errp);
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 319d971..8aa4d7b 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -963,6 +963,18 @@ STEXI
>   Disable SDL window close capability.
>   ETEXI
>   
> +DEF("no-format-probing", 0, QEMU_OPTION_no_format_probing,
> +    "-no-format-probing\n"
> +    "                disable block image format probing\n", QEMU_ARCH_ALL)
> +STEXI
> +@item -no-format-probing
> +@findex -no-format-probing
> +Disable block image format probing.  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.
> +ETEXI
> +
>   DEF("sdl", 0, QEMU_OPTION_sdl,
>       "-sdl            enable SDL\n", QEMU_ARCH_ALL)
>   STEXI
> diff --git a/vl.c b/vl.c
> index 75ec292..94d5e15 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2754,6 +2754,7 @@ int main(int argc, char **argv, char **envp)
>   #endif
>       bool defconfig = true;
>       bool userconfig = true;
> +    bool no_format_probing = false;
>       const char *log_mask = NULL;
>       const char *log_file = NULL;
>       GMemVTable mem_trace = {
> @@ -2823,7 +2824,7 @@ int main(int argc, char **argv, char **envp)
>   
>       nb_nics = 0;
>   
> -    bdrv_init_with_whitelist();
> +    bdrv_init_with_whitelist(no_format_probing);
>   
>       autostart = 1;
>   
> @@ -3381,6 +3382,9 @@ int main(int argc, char **argv, char **envp)
>               case QEMU_OPTION_no_quit:
>                   no_quit = 1;
>                   break;
> +            case QEMU_OPTION_no_format_probing:
> +                no_format_probing = true;
> +                break;
>               case QEMU_OPTION_sdl:
>   #ifdef CONFIG_SDL
>                   display_type = DT_SDL;

You're setting no_format_probing after you're using it, so it doesn't 
work very well. :-)

Max