From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57086) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YYxJK-0005eG-6T for qemu-devel@nongnu.org; Fri, 20 Mar 2015 09:50:03 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YYxJE-0005qx-8h for qemu-devel@nongnu.org; Fri, 20 Mar 2015 09:50:02 -0400 Message-ID: <550C2577.2010201@redhat.com> Date: Fri, 20 Mar 2015 09:49:43 -0400 From: Max Reitz MIME-Version: 1.0 References: <1426856744-18750-1-git-send-email-armbru@redhat.com> <1426856744-18750-2-git-send-email-armbru@redhat.com> <550C21DB.8000607@redhat.com> <87h9tf7pbi.fsf@blackfin.pond.sub.org> In-Reply-To: <87h9tf7pbi.fsf@blackfin.pond.sub.org> Content-Type: text/plain; charset=iso-8859-15; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH RFC for-2.3 1/1] block: New command line option --no-format-probing List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: kwolf@redhat.com, qemu-block@nongnu.org, qemu-devel@nongnu.org, stefanha@redhat.com On 2015-03-20 at 09:48, Markus Armbruster wrote: > Max Reitz writes: > >> 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 >>> --- >>> 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. :-) > I really should not test stuff on a hurried Friday afternoon... > > I'd appreciate opinions on whether this is wanted for 2.3. If it is, > I'll post a version that actually works. I don't have any objections because it won't break anything. But I guess it'll be mostly up to whether Eric thinks that we'll need it right now. Max