Re: [PATCH] block: Raise an error when backing file parameter is an empty string

[Connor Kuehl <ckuehl@redhat.com>]

Ping

On 7/1/20 3:42 PM, Connor Kuehl wrote:
> 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
>>
>