Re: [PATCH v2 0/2] block: Use 'read-zeroes=true' mode by default with 'null-co' driver

[Fam Zheng <fam@euphon.net>]

On 2021-02-11 15:26, Philippe Mathieu-Daudé wrote:
> The null-co driver doesn't zeroize buffer in its default config,
> because it is designed for testing and tests want to run fast.
> However this confuses security researchers (access to uninit
> buffers).

I'm a little surprised.

Is changing default the only way to fix this? I'm not opposed to
changing the default but I'm not convinced this is the easiest way.
block/nvme.c also doesn't touch the memory, but defers to the device
DMA, why doesn't that confuse the security checker?

Cannot we just somehow annotate it in a way that the checker can
understand (akin to how we provide coverity models) and be done?

Thanks,
Fam

> 
> A one-line patch supposed which became a painful one, because
> there is so many different syntax to express the same usage:
> 
>  opt = qdict_new();
>  qdict_put_str(opt, "read-zeroes", "off");
>  null_bs = bdrv_open("null-co://", NULL, opt, BDRV_O_RDWR | BDRV_O_PROTOCOL,
>                      &error_abort);
> 
> vm.qmp('blockdev-add', driver='null-co', read_zeroes=False, ...)
> 
> vm.add_drive_raw("id=drive0,driver=null-co,read-zeroes=off,if=none")
> 
>     blk0 = { 'node-name': 'src',
>         'driver': 'null-co',
>         'read-zeroes': 'off' }
> 
>     'file': {
>         'driver': 'null-co',
>         'read-zeroes': False,
>     }
> 
>     "file": {
>         "driver": "null-co",
>         "read-zeroes": "off"
>     }
> 
>     { "execute": "blockdev-add",
>       "arguments": {
>           "driver": "null-co",
>           "read-zeroes": false,
>           "node-name": "disk0"
>         }
>     }
> 
> opts = {'driver': 'null-co,read-zeroes=off', 'node-name': 'root', 'size': 1024}
> 
> qemu -drive driver=null-co,read-zeroes=off
> 
> qemu-io ... "json:{'driver': 'null-co', 'read-zeroes': false, 'size': 65536}"
> 
> qemu-img null-co://,read-zeroes=off
> 
> qemu-img ... -o data_file="json:{'driver':'null-co',,'read-zeroes':false,,'size':'4294967296'}"
> 
> There are probably more.
> 
> Anyhow, the iotests I am not sure and should be audited are 056, 155
> (I don't understand the syntax) and 162.
> 
> Please review,
> 
> Phil.
> 
> Philippe Mathieu-Daud=C3=A9 (2):
>   block: Explicit null-co uses 'read-zeroes=3Dfalse'
>   block/null: Enable 'read-zeroes' mode by default
> 
>  docs/devel/testing.rst                     | 14 +++++++-------
>  tests/qtest/fuzz/generic_fuzz_configs.h    | 11 ++++++-----
>  block/null.c                               |  2 +-
>  tests/test-bdrv-drain.c                    | 10 ++++++++--
>  tests/acceptance/virtio_check_params.py    |  2 +-
>  tests/perf/block/qcow2/convert-blockstatus |  6 +++---
>  tests/qemu-iotests/040                     |  2 +-
>  tests/qemu-iotests/041                     | 12 ++++++++----
>  tests/qemu-iotests/051                     |  2 +-
>  tests/qemu-iotests/051.out                 |  2 +-
>  tests/qemu-iotests/051.pc.out              |  4 ++--
>  tests/qemu-iotests/087                     |  6 ++++--
>  tests/qemu-iotests/118                     |  2 +-
>  tests/qemu-iotests/133                     |  2 +-
>  tests/qemu-iotests/153                     |  8 ++++----
>  tests/qemu-iotests/184                     |  2 ++
>  tests/qemu-iotests/184.out                 | 10 +++++-----
>  tests/qemu-iotests/218                     |  3 +++
>  tests/qemu-iotests/224                     |  3 ++-
>  tests/qemu-iotests/224.out                 |  8 ++++----
>  tests/qemu-iotests/225                     |  2 +-
>  tests/qemu-iotests/227                     |  4 ++--
>  tests/qemu-iotests/227.out                 |  4 ++--
>  tests/qemu-iotests/228                     |  2 +-
>  tests/qemu-iotests/235                     |  1 +
>  tests/qemu-iotests/245                     |  2 +-
>  tests/qemu-iotests/270                     |  2 +-
>  tests/qemu-iotests/283                     |  3 ++-
>  tests/qemu-iotests/283.out                 |  4 ++--
>  tests/qemu-iotests/299                     |  1 +
>  tests/qemu-iotests/299.out                 |  2 +-
>  tests/qemu-iotests/300                     |  4 ++--
>  32 files changed, 82 insertions(+), 60 deletions(-)
> 
> --=20
> 2.26.2
> 
> 
>