[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [RFC PATCH] block/null: Use 'read-zeroes' mode by default
From: |
Eric Blake |
Subject: |
Re: [RFC PATCH] block/null: Use 'read-zeroes' mode by default |
Date: |
Tue, 9 Feb 2021 11:11:03 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.7.0 |
On 2/9/21 11:01 AM, Philippe Mathieu-Daudé wrote:
> The null-co driver is meant for (performance) testing.
> By default, read operation does nothing, the provided buffer
> is not filled with zero values and its content is unchanged.
>
> This can confuse security experts. For example, using the default
> null-co driver, buf[] is uninitialized, the blk_pread() call
> succeeds and we then access uninitialized memory:
>
> static int guess_disk_lchs(BlockBackend *blk,
> int *pcylinders, int *pheads,
> int *psectors)
> {
> uint8_t buf[BDRV_SECTOR_SIZE];
> ...
>
> if (blk_pread(blk, 0, buf, BDRV_SECTOR_SIZE) < 0) {
> return -1;
> }
> /* test msdos magic */
> if (buf[510] != 0x55 || buf[511] != 0xaa) {
> return -1;
> }
>
> We could audit all the uninitialized buffers and the
> bdrv_co_preadv() handlers, but it is simpler to change the
> default of this testing driver. Performance tests will have
> to adapt and use 'null-co,read-zeroes=on'.
Wouldn't this rather be read-zeroes=off when doing performance testing?
>
> Suggested-by: Max Reitz <mreitz@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> RFC maybe a stricter approach is required?
Since the null driver is only for testing in the first place, opting in
to speed over security seems like a reasonable tradeoff. But I consider
the patch incomplete without an audit of the iotests that will want to
use explicit read-zeroes=off.
> ---
> block/null.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/block/null.c b/block/null.c
> index cc9b1d4ea72..f9658fd70ac 100644
> --- a/block/null.c
> +++ b/block/null.c
> @@ -93,7 +93,7 @@ static int null_file_open(BlockDriverState *bs, QDict
> *options, int flags,
> error_setg(errp, "latency-ns is invalid");
> ret = -EINVAL;
> }
> - s->read_zeroes = qemu_opt_get_bool(opts, NULL_OPT_ZEROES, false);
> + s->read_zeroes = qemu_opt_get_bool(opts, NULL_OPT_ZEROES, true);
> qemu_opts_del(opts);
> bs->supported_write_flags = BDRV_REQ_FUA;
> return ret;
>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org