From: Connor Kuehl <ckuehl@redhat.com>
To: kwolf@redhat.com, mreitz@redhat.com
Cc: qemu-devel@nongnu.org, qemu-block@nongnu.org
Subject: Re: [PATCH] block: Raise an error when backing file parameter is an empty string
Date: Wed, 1 Jul 2020 15:42:17 -0700 [thread overview]
Message-ID: <47f5b1fe-e6cd-2302-e36f-5ad071cd3374@redhat.com> (raw)
In-Reply-To: <20200617182725.951119-1-ckuehl@redhat.com>
Hi Kevin & Max,
Just pinging this patch for your consideration.
Thank you,
Connor
On 6/17/20 11:27 AM, Connor Kuehl wrote:
> Providing an empty string for the backing file parameter like so:
>
> qemu-img create -f qcow2 -b '' /tmp/foo
>
> allows the flow of control to reach and subsequently fail an assert
> statement because passing an empty string to
>
> bdrv_get_full_backing_filename_from_filename()
>
> simply results in NULL being returned without an error being raised.
>
> To fix this, let's check for an empty string when getting the value from
> the opts list.
>
> Reported-by: Attila Fazekas <afazekas@redhat.com>
> Fixes: https://bugzilla.redhat.com/1809553
> Signed-off-by: Connor Kuehl <ckuehl@redhat.com>
> ---
> block.c | 4 ++++
> tests/qemu-iotests/298 | 47 ++++++++++++++++++++++++++++++++++++++
> tests/qemu-iotests/298.out | 5 ++++
> tests/qemu-iotests/group | 1 +
> 4 files changed, 57 insertions(+)
> create mode 100755 tests/qemu-iotests/298
> create mode 100644 tests/qemu-iotests/298.out
>
> diff --git a/block.c b/block.c
> index 6dbcb7e083..b335d6bcb2 100644
> --- a/block.c
> +++ b/block.c
> @@ -6116,6 +6116,10 @@ void bdrv_img_create(const char *filename, const char *fmt,
> "same filename as the backing file");
> goto out;
> }
> + if (backing_file[0] == '\0') {
> + error_setg(errp, "Expected backing file name, got empty string");
> + goto out;
> + }
> }
>
> backing_fmt = qemu_opt_get(opts, BLOCK_OPT_BACKING_FMT);
> diff --git a/tests/qemu-iotests/298 b/tests/qemu-iotests/298
> new file mode 100755
> index 0000000000..1e30caebec
> --- /dev/null
> +++ b/tests/qemu-iotests/298
> @@ -0,0 +1,47 @@
> +#!/usr/bin/env python3
> +#
> +# Copyright (C) 2020 Red Hat, Inc.
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 2 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program. If not, see <http://www.gnu.org/licenses/>.
> +
> +
> +
> +# Regression test for avoiding an assertion when the 'backing file'
> +# parameter (-b) is set to an empty string for qemu-img create
> +#
> +# qemu-img create -f qcow2 -b '' /tmp/foo
> +#
> +# This ensures the invalid parameter is handled with a user-
> +# friendly message instead of a failed assertion.
> +
> +import iotests
> +
> +class TestEmptyBackingFilename(iotests.QMPTestCase):
> +
> +
> + def test_empty_backing_file_name(self):
> + actual = iotests.qemu_img_pipe(
> + 'create',
> + '-f', 'qcow2',
> + '-b', '',
> + '/tmp/foo'
> + )
> + expected = 'qemu-img: /tmp/foo: Expected backing file name,' \
> + ' got empty string'
> +
> + self.assertEqual(actual.strip(), expected.strip())
> +
> +
> +if __name__ == '__main__':
> + iotests.main(supported_fmts=['raw', 'qcow2'])
> diff --git a/tests/qemu-iotests/298.out b/tests/qemu-iotests/298.out
> new file mode 100644
> index 0000000000..ae1213e6f8
> --- /dev/null
> +++ b/tests/qemu-iotests/298.out
> @@ -0,0 +1,5 @@
> +.
> +----------------------------------------------------------------------
> +Ran 1 tests
> +
> +OK
> diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
> index d886fa0cb3..4bca2d9e05 100644
> --- a/tests/qemu-iotests/group
> +++ b/tests/qemu-iotests/group
> @@ -302,3 +302,4 @@
> 291 rw quick
> 292 rw auto quick
> 297 meta
> +298 img auto quick
>
next prev parent reply other threads:[~2020-07-01 22:45 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-06-17 18:27 [PATCH] block: Raise an error when backing file parameter is an empty string Connor Kuehl
2020-06-17 18:50 ` no-reply
2020-06-17 19:11 ` Eric Blake
2020-06-18 10:48 ` Alberto Garcia
2020-07-01 22:42 ` Connor Kuehl [this message]
2020-07-10 15:57 ` Connor Kuehl
2020-07-13 10:10 ` Max Reitz
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=47f5b1fe-e6cd-2302-e36f-5ad071cd3374@redhat.com \
--to=ckuehl@redhat.com \
--cc=kwolf@redhat.com \
--cc=mreitz@redhat.com \
--cc=qemu-block@nongnu.org \
--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).