[PATCH 0/3] block: Introduce the 'zeroes-co' driver to help security reports

[PATCH 0/3] block: Introduce the 'zeroes-co' driver to help security reports

[Philippe Mathieu-Daudé]

Hi,

This is an alternative approach to changing null-co driver
default 'read-zeroes' option to true:
https://www.mail-archive.com/qemu-block@nongnu.org/msg80873.html

Instead we introduce yet another block driver with an explicit
name: 'zeroes-co'. We then clarify in secure-coding-practices.rst
that security reports have to be sent using this new driver.

The 2nd patch is RFC because I won't spend time converting the
tests until the first patch is discussed, as I already spent enough
time doing that in the previous mentioned series.

Regards,

Phil.

Philippe Mathieu-Daudé (3):
  block: Introduce the 'zeroes-co' driver
  tests/test-blockjob: Use zeroes-co instead of null-co,read-zeroes=on
  docs/secure-coding-practices: Describe null-co/zeroes-co block drivers

 docs/devel/secure-coding-practices.rst |   7 +
 block/zeroes.c                         | 306 +++++++++++++++++++++++++
 tests/test-blockjob.c                  |   4 +-
 block/meson.build                      |   1 +
 4 files changed, 315 insertions(+), 3 deletions(-)
 create mode 100644 block/zeroes.c

-- 
2.26.2

[PATCH 1/3] block: Introduce the 'zeroes-co' driver

[Philippe Mathieu-Daudé]

The 'zeroes-co' block driver is almost a copy of the 'null-co'
block driver designed for performance testing, but targets
security needs, by always zero-initializing read accesses.
Write accesses are discarded.

Suggested-by: Fam Zheng <fam@euphon.net>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 block/zeroes.c    | 306 ++++++++++++++++++++++++++++++++++++++++++++++
 block/meson.build |   1 +
 2 files changed, 307 insertions(+)
 create mode 100644 block/zeroes.c

diff --git a/block/zeroes.c b/block/zeroes.c
new file mode 100644
index 00000000000..7256b6d02ee
--- /dev/null
+++ b/block/zeroes.c
@@ -0,0 +1,306 @@
+/*
+ * Zeroes block driver
+ *
+ * Based on block/null.c
+ *
+ * Copyright (C) 2021 Red Hat, Inc.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "qapi/qmp/qdict.h"
+#include "qapi/qmp/qstring.h"
+#include "qemu/module.h"
+#include "qemu/option.h"
+#include "block/block_int.h"
+#include "sysemu/replay.h"
+
+#define NULL_OPT_LATENCY "latency-ns"
+
+typedef struct {
+    int64_t length;
+    int64_t latency_ns;
+} BDRVZeroesState;
+
+static QemuOptsList runtime_opts = {
+    .name = "zeroes",
+    .head = QTAILQ_HEAD_INITIALIZER(runtime_opts.head),
+    .desc = {
+        {
+            .name = BLOCK_OPT_SIZE,
+            .type = QEMU_OPT_SIZE,
+            .help = "size of the zeroes block",
+        },
+        {
+            .name = NULL_OPT_LATENCY,
+            .type = QEMU_OPT_NUMBER,
+            .help = "nanoseconds (approximated) to wait "
+                    "before completing request",
+        },
+        { /* end of list */ }
+    },
+};
+
+static void zeroes_co_parse_filename(const char *filename, QDict *options,
+                                     Error **errp)
+{
+    /*
+     * This functions only exists so that a zeroes-co:// filename
+     * is accepted with the zeroes-co driver.
+     */
+    if (strcmp(filename, "zeroes-co://")) {
+        error_setg(errp, "The only allowed filename for this driver is "
+                         "'zeroes-co://'");
+        return;
+    }
+}
+
+static void zeroes_aio_parse_filename(const char *filename, QDict *options,
+                                      Error **errp)
+{
+    /*
+     * This functions only exists so that a zeroes-aio:// filename
+     * is accepted with the zeroes-aio driver.
+     */
+    if (strcmp(filename, "zeroes-aio://")) {
+        error_setg(errp, "The only allowed filename for this driver is "
+                         "'zeroes-aio://'");
+        return;
+    }
+}
+
+static int zeroes_file_open(BlockDriverState *bs, QDict *options,
+                            int flags, Error **errp)
+{
+    QemuOpts *opts;
+    BDRVZeroesState *s = bs->opaque;
+    int ret = 0;
+
+    opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort);
+    qemu_opts_absorb_qdict(opts, options, &error_abort);
+    s->length = qemu_opt_get_size(opts, BLOCK_OPT_SIZE, 0);
+    if (s->length < 0) {
+        error_setg(errp, "%s is invalid", BLOCK_OPT_SIZE);
+        ret = -EINVAL;
+    }
+    s->latency_ns = qemu_opt_get_number(opts, NULL_OPT_LATENCY, 0);
+    if (s->latency_ns < 0) {
+        error_setg(errp, "%s is invalid", NULL_OPT_LATENCY);
+        ret = -EINVAL;
+    }
+    qemu_opts_del(opts);
+    bs->supported_write_flags = BDRV_REQ_FUA;
+    return ret;
+}
+
+static int64_t zeroes_getlength(BlockDriverState *bs)
+{
+    BDRVZeroesState *s = bs->opaque;
+    return s->length;
+}
+
+static coroutine_fn int zeroes_co_common(BlockDriverState *bs)
+{
+    BDRVZeroesState *s = bs->opaque;
+
+    if (s->latency_ns) {
+        qemu_co_sleep_ns(QEMU_CLOCK_REALTIME, s->latency_ns);
+    }
+    return 0;
+}
+
+static coroutine_fn int zeroes_co_preadv(BlockDriverState *bs,
+                                         uint64_t offset, uint64_t bytes,
+                                         QEMUIOVector *qiov, int flags)
+{
+    qemu_iovec_memset(qiov, 0, 0, bytes);
+
+    return zeroes_co_common(bs);
+}
+
+static coroutine_fn int zeroes_co_pwritev(BlockDriverState *bs,
+                                          uint64_t offset, uint64_t bytes,
+                                          QEMUIOVector *qiov, int flags)
+{
+    return zeroes_co_common(bs);
+}
+
+static coroutine_fn int zeroes_co_flush(BlockDriverState *bs)
+{
+    return zeroes_co_common(bs);
+}
+
+typedef struct {
+    BlockAIOCB common;
+    QEMUTimer timer;
+} ZeroesAIOCB;
+
+static const AIOCBInfo zeroes_aiocb_info = {
+    .aiocb_size = sizeof(ZeroesAIOCB),
+};
+
+static void zeroes_bh_cb(void *opaque)
+{
+    ZeroesAIOCB *acb = opaque;
+    acb->common.cb(acb->common.opaque, 0);
+    qemu_aio_unref(acb);
+}
+
+static void zeroes_timer_cb(void *opaque)
+{
+    ZeroesAIOCB *acb = opaque;
+    acb->common.cb(acb->common.opaque, 0);
+    timer_deinit(&acb->timer);
+    qemu_aio_unref(acb);
+}
+
+static inline BlockAIOCB *zeroes_aio_common(BlockDriverState *bs,
+                                            BlockCompletionFunc *cb,
+                                            void *opaque)
+{
+    ZeroesAIOCB *acb;
+    BDRVZeroesState *s = bs->opaque;
+
+    acb = qemu_aio_get(&zeroes_aiocb_info, bs, cb, opaque);
+    /* Only emulate latency after vcpu is running. */
+    if (s->latency_ns) {
+        aio_timer_init(bdrv_get_aio_context(bs), &acb->timer,
+                       QEMU_CLOCK_REALTIME, SCALE_NS,
+                       zeroes_timer_cb, acb);
+        timer_mod_ns(&acb->timer,
+                     qemu_clock_get_ns(QEMU_CLOCK_REALTIME) + s->latency_ns);
+    } else {
+        replay_bh_schedule_oneshot_event(bdrv_get_aio_context(bs),
+                                         zeroes_bh_cb, acb);
+    }
+    return &acb->common;
+}
+
+static BlockAIOCB *zeroes_aio_preadv(BlockDriverState *bs,
+                                   uint64_t offset, uint64_t bytes,
+                                   QEMUIOVector *qiov, int flags,
+                                   BlockCompletionFunc *cb,
+                                   void *opaque)
+{
+    qemu_iovec_memset(qiov, 0, 0, bytes);
+
+    return zeroes_aio_common(bs, cb, opaque);
+}
+
+static BlockAIOCB *zeroes_aio_pwritev(BlockDriverState *bs,
+                                      uint64_t offset, uint64_t bytes,
+                                      QEMUIOVector *qiov, int flags,
+                                      BlockCompletionFunc *cb,
+                                      void *opaque)
+{
+    return zeroes_aio_common(bs, cb, opaque);
+}
+
+static BlockAIOCB *zeroes_aio_flush(BlockDriverState *bs,
+                                    BlockCompletionFunc *cb,
+                                    void *opaque)
+{
+    return zeroes_aio_common(bs, cb, opaque);
+}
+
+static int zeroes_reopen_prepare(BDRVReopenState *reopen_state,
+                                 BlockReopenQueue *queue, Error **errp)
+{
+    return 0;
+}
+
+static int coroutine_fn zeroes_co_block_status(BlockDriverState *bs,
+                                               bool want_zero, int64_t offset,
+                                               int64_t bytes, int64_t *pnum,
+                                               int64_t *map,
+                                               BlockDriverState **file)
+{
+    *pnum = bytes;
+    *map = offset;
+    *file = bs;
+
+    return BDRV_BLOCK_OFFSET_VALID | BDRV_BLOCK_ZERO;
+}
+
+static void zeroes_refresh_filename(BlockDriverState *bs)
+{
+    const QDictEntry *e;
+
+    for (e = qdict_first(bs->full_open_options); e;
+         e = qdict_next(bs->full_open_options, e))
+    {
+        /* These options can be ignored */
+        if (strcmp(qdict_entry_key(e), "filename") &&
+            strcmp(qdict_entry_key(e), "driver") &&
+            strcmp(qdict_entry_key(e), NULL_OPT_LATENCY))
+        {
+            return;
+        }
+    }
+
+    snprintf(bs->exact_filename, sizeof(bs->exact_filename),
+             "%s://", bs->drv->format_name);
+}
+
+static int64_t zeroes_allocated_file_size(BlockDriverState *bs)
+{
+    return 0;
+}
+
+static const char *const zeroes_strong_runtime_opts[] = {
+    BLOCK_OPT_SIZE,
+
+    NULL
+};
+
+static BlockDriver bdrv_zeroes_co = {
+    .format_name            = "zeroes-co",
+    .protocol_name          = "zeroes-co",
+    .instance_size          = sizeof(BDRVZeroesState),
+
+    .bdrv_file_open         = zeroes_file_open,
+    .bdrv_parse_filename    = zeroes_co_parse_filename,
+    .bdrv_getlength         = zeroes_getlength,
+    .bdrv_get_allocated_file_size = zeroes_allocated_file_size,
+
+    .bdrv_co_preadv         = zeroes_co_preadv,
+    .bdrv_co_pwritev        = zeroes_co_pwritev,
+    .bdrv_co_flush_to_disk  = zeroes_co_flush,
+    .bdrv_reopen_prepare    = zeroes_reopen_prepare,
+
+    .bdrv_co_block_status   = zeroes_co_block_status,
+
+    .bdrv_refresh_filename  = zeroes_refresh_filename,
+    .strong_runtime_opts    = zeroes_strong_runtime_opts,
+};
+
+static BlockDriver bdrv_zeroes_aio = {
+    .format_name            = "zeroes-aio",
+    .protocol_name          = "zeroes-aio",
+    .instance_size          = sizeof(BDRVZeroesState),
+
+    .bdrv_file_open         = zeroes_file_open,
+    .bdrv_parse_filename    = zeroes_aio_parse_filename,
+    .bdrv_getlength         = zeroes_getlength,
+    .bdrv_get_allocated_file_size = zeroes_allocated_file_size,
+
+    .bdrv_aio_preadv        = zeroes_aio_preadv,
+    .bdrv_aio_pwritev       = zeroes_aio_pwritev,
+    .bdrv_aio_flush         = zeroes_aio_flush,
+    .bdrv_reopen_prepare    = zeroes_reopen_prepare,
+
+    .bdrv_co_block_status   = zeroes_co_block_status,
+
+    .bdrv_refresh_filename  = zeroes_refresh_filename,
+    .strong_runtime_opts    = zeroes_strong_runtime_opts,
+};
+
+static void bdrv_zeroes_init(void)
+{
+    bdrv_register(&bdrv_zeroes_co);
+    bdrv_register(&bdrv_zeroes_aio);
+}
+
+block_init(bdrv_zeroes_init);
diff --git a/block/meson.build b/block/meson.build
index d21990ec95a..661d84118fb 100644
--- a/block/meson.build
+++ b/block/meson.build
@@ -40,6 +40,7 @@
   'vmdk.c',
   'vpc.c',
   'write-threshold.c',
+  'zeroes.c',
 ), zstd, zlib, gnutls)
 
 softmmu_ss.add(when: 'CONFIG_TCG', if_true: files('blkreplay.c'))
-- 
2.26.2

[RFC PATCH 2/3] tests/test-blockjob: Use zeroes-co instead of null-co, read-zeroes=on

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
RFC because I won't convert the rest of the tests until the
previous patch is reviewed.
---
 tests/test-blockjob.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/tests/test-blockjob.c b/tests/test-blockjob.c
index 75198479120..ae2ea7028ee 100644
--- a/tests/test-blockjob.c
+++ b/tests/test-blockjob.c
@@ -70,9 +70,7 @@ static BlockBackend *create_blk(const char *name)
     BlockBackend *blk = blk_new(qemu_get_aio_context(), 0, BLK_PERM_ALL);
     BlockDriverState *bs;
 
-    QDict *opt = qdict_new();
-    qdict_put_str(opt, "file.read-zeroes", "on");
-    bs = bdrv_open("null-co://", NULL, opt, 0, &error_abort);
+    bs = bdrv_open("zeroes-co://", NULL, NULL, 0, &error_abort);
     g_assert_nonnull(bs);
 
     blk_insert_bs(blk, bs, &error_abort);
-- 
2.26.2

[PATCH 3/3] docs/secure-coding-practices: Describe null-co/zeroes-co block drivers

[Philippe Mathieu-Daudé]

Document that security reports must not use the 'null-co' block
driver, as it leaves memory uninitialized on purposed (this is
a performance feature).
Reports must be send using the 'zeroes-co' driver.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 docs/devel/secure-coding-practices.rst | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/docs/devel/secure-coding-practices.rst b/docs/devel/secure-coding-practices.rst
index cbfc8af67e6..64d61085804 100644
--- a/docs/devel/secure-coding-practices.rst
+++ b/docs/devel/secure-coding-practices.rst
@@ -104,3 +104,10 @@ structures and only process the local copy.  This prevents
 time-of-check-to-time-of-use (TOCTOU) race conditions that could cause QEMU to
 crash when a vCPU thread modifies guest RAM while device emulation is
 processing it.
+
+Use of null-co / zeroes-co block drivers
+----------------------------------------
+
+When reporting security issues, the null-co block driver must not be used,
+as it is designed for performance and its read accesses are not initialized.
+The zeroes-co block driver must be used instead.
-- 
2.26.2

Re: [PATCH 3/3] docs/secure-coding-practices: Describe null-co/zeroes-co block drivers

[Vladimir Sementsov-Ogievskiy]

10.03.2021 14:43, Philippe Mathieu-Daudé wrote:
> Document that security reports must not use the 'null-co' block
> driver, as it leaves memory uninitialized on purposed (this is
> a performance feature).
> Reports must be send using the 'zeroes-co' driver.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>   docs/devel/secure-coding-practices.rst | 7 +++++++
>   1 file changed, 7 insertions(+)
> 
> diff --git a/docs/devel/secure-coding-practices.rst b/docs/devel/secure-coding-practices.rst
> index cbfc8af67e6..64d61085804 100644
> --- a/docs/devel/secure-coding-practices.rst
> +++ b/docs/devel/secure-coding-practices.rst
> @@ -104,3 +104,10 @@ structures and only process the local copy.  This prevents
>   time-of-check-to-time-of-use (TOCTOU) race conditions that could cause QEMU to
>   crash when a vCPU thread modifies guest RAM while device emulation is
>   processing it.
> +
> +Use of null-co / zeroes-co block drivers
> +----------------------------------------
> +
> +When reporting security issues, the null-co block driver must not be used,
> +as it is designed for performance and its read accesses are not initialized.
> +The zeroes-co block driver must be used instead.
> 

How much it differs from just document that when reporting security issues the null-co block driver must be used with read-zeroes=true?

-- 
Best regards,
Vladimir

Re: [PATCH 0/3] block: Introduce the 'zeroes-co' driver to help security reports

[Daniel P. Berrangé]

On Wed, Mar 10, 2021 at 12:43:11PM +0100, Philippe Mathieu-Daudé wrote:
> Hi,
> 
> This is an alternative approach to changing null-co driver
> default 'read-zeroes' option to true:
> https://www.mail-archive.com/qemu-block@nongnu.org/msg80873.html
> 
> Instead we introduce yet another block driver with an explicit
> name: 'zeroes-co'. We then clarify in secure-coding-practices.rst
> that security reports have to be sent using this new driver.

IMHO introducing a new block driver, when this can be solved by
simply adding a property to the existing driver, just feels mad
Your previous series made much more sense, and despite the long
thread, I didn't see anyone suggest a real world blocker with
making it read zeros by default.

I think Max's last mail sums it up pretty well

  https://lists.gnu.org/archive/html/qemu-devel/2021-02/msg07173.html

[quote]
In cases where we have a test that just wants a simple block node that
doesn’t use disk space, the memset() can’t be noticeable. But it’s just
a test, so do we even need the memset()? Strictly speaking, perhaps not,
but if someone is to run it via Valgrind or something, they may get
false positives, so just doing the memset() is the right thing to do.

For performance tests, it must be possible to set read-zeroes=off,
because even though “that memset() isn’t noticeable in a functional
test”, in a hard-core performance test, it will be.

So we need a switch. It should default to memset(), because (1) making
tools like Valgrind happy seems like a reasonable objective to me, and
(2) in the majority of cases, the memset() cannot have a noticeable
impact.
[/quote]


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH 0/3] block: Introduce the 'zeroes-co' driver to help security reports

[Philippe Mathieu-Daudé]

On 3/10/21 12:55 PM, Daniel P. Berrangé wrote:
> On Wed, Mar 10, 2021 at 12:43:11PM +0100, Philippe Mathieu-Daudé wrote:
>> Hi,
>>
>> This is an alternative approach to changing null-co driver
>> default 'read-zeroes' option to true:
>> https://www.mail-archive.com/qemu-block@nongnu.org/msg80873.html
>>
>> Instead we introduce yet another block driver with an explicit
>> name: 'zeroes-co'. We then clarify in secure-coding-practices.rst
>> that security reports have to be sent using this new driver.
> 
> IMHO introducing a new block driver, when this can be solved by
> simply adding a property to the existing driver, just feels mad
> Your previous series made much more sense, and despite the long
> thread, I didn't see anyone suggest a real world blocker with
> making it read zeros by default.
> 
> I think Max's last mail sums it up pretty well
> 
>   https://lists.gnu.org/archive/html/qemu-devel/2021-02/msg07173.html
> 
> [quote]
> In cases where we have a test that just wants a simple block node that
> doesn’t use disk space, the memset() can’t be noticeable. But it’s just
> a test, so do we even need the memset()? Strictly speaking, perhaps not,
> but if someone is to run it via Valgrind or something, they may get
> false positives, so just doing the memset() is the right thing to do.
> 
> For performance tests, it must be possible to set read-zeroes=off,
> because even though “that memset() isn’t noticeable in a functional
> test”, in a hard-core performance test, it will be.
> 
> So we need a switch. It should default to memset(), because (1) making
> tools like Valgrind happy seems like a reasonable objective to me, and
> (2) in the majority of cases, the memset() cannot have a noticeable
> impact.
> [/quote]

Yes I totally agree with Max, but Fam is the maintainer.

I'll keep looking for alternatives. Maybe simply documentation.

Thanks,

Phil.

Re: [PATCH 0/3] block: Introduce the 'zeroes-co' driver to help security reports

[Fam Zheng]

[-- Attachment #1: Type: text/plain, Size: 1451 bytes --]

On Wed, 10 Mar 2021 at 11:44, Philippe Mathieu-Daudé <philmd@redhat.com>
wrote:

> Hi,
>
> This is an alternative approach to changing null-co driver
> default 'read-zeroes' option to true:
> https://www.mail-archive.com/qemu-block@nongnu.org/msg80873.html
>
> Instead we introduce yet another block driver with an explicit
> name: 'zeroes-co'. We then clarify in secure-coding-practices.rst
> that security reports have to be sent using this new driver.
>
> The 2nd patch is RFC because I won't spend time converting the
> tests until the first patch is discussed, as I already spent enough
> time doing that in the previous mentioned series.
>
> Regards,
>
> Phil.
>
> Philippe Mathieu-Daudé (3):
>   block: Introduce the 'zeroes-co' driver
>   tests/test-blockjob: Use zeroes-co instead of null-co,read-zeroes=on
>   docs/secure-coding-practices: Describe null-co/zeroes-co block drivers
>
>  docs/devel/secure-coding-practices.rst |   7 +
>  block/zeroes.c                         | 306 +++++++++++++++++++++++++
>


Why not add another BlockDriver struct to block/null.c and set the
read_zeroes field in the .bdrv_file_open callback? It would make the patch
much simpler.

Fam


>  tests/test-blockjob.c                  |   4 +-
>  block/meson.build                      |   1 +
>  4 files changed, 315 insertions(+), 3 deletions(-)
>  create mode 100644 block/zeroes.c
>
> --
> 2.26.2
>
>
>
>

[-- Attachment #2: Type: text/html, Size: 2285 bytes --]

Re: [PATCH 0/3] block: Introduce the 'zeroes-co' driver to help security reports

[Philippe Mathieu-Daudé]

On 3/10/21 1:32 PM, Fam Zheng wrote:
> On Wed, 10 Mar 2021 at 11:44, Philippe Mathieu-Daudé <philmd@redhat.com
> <mailto:philmd@redhat.com>> wrote:
> 
>     Hi,
> 
>     This is an alternative approach to changing null-co driver
>     default 'read-zeroes' option to true:
>     https://www.mail-archive.com/qemu-block@nongnu.org/msg80873.html
>     <https://www.mail-archive.com/qemu-block@nongnu.org/msg80873.html>
> 
>     Instead we introduce yet another block driver with an explicit
>     name: 'zeroes-co'. We then clarify in secure-coding-practices.rst
>     that security reports have to be sent using this new driver.
> 
>     The 2nd patch is RFC because I won't spend time converting the
>     tests until the first patch is discussed, as I already spent enough
>     time doing that in the previous mentioned series.
> 
>     Regards,
> 
>     Phil.
> 
>     Philippe Mathieu-Daudé (3):
>       block: Introduce the 'zeroes-co' driver
>       tests/test-blockjob: Use zeroes-co instead of null-co,read-zeroes=on
>       docs/secure-coding-practices: Describe null-co/zeroes-co block drivers
> 
>      docs/devel/secure-coding-practices.rst |   7 +
>      block/zeroes.c                         | 306 +++++++++++++++++++++++++
> 
> Why not add another BlockDriver struct to block/null.c and set the
> read_zeroes field in the .bdrv_file_open callback? It would make the
> patch much simpler.

This is the first patch I wrote but then realized you are listed as
null-co maintainer, so you might be uninterested in having this new
driver in the same file. And declaring the prototypes public to
reuse them seemed overkill.

Anyway I'll try to find a simpler outcome by simply improving
documentation.

Re: [PATCH 0/3] block: Introduce the 'zeroes-co' driver to help security reports

[Fam Zheng]

[-- Attachment #1: Type: text/plain, Size: 2352 bytes --]

On Wed, 10 Mar 2021 at 12:38, Philippe Mathieu-Daudé <philmd@redhat.com>
wrote:

> On 3/10/21 1:32 PM, Fam Zheng wrote:
> > On Wed, 10 Mar 2021 at 11:44, Philippe Mathieu-Daudé <philmd@redhat.com
> > <mailto:philmd@redhat.com>> wrote:
> >
> >     Hi,
> >
> >     This is an alternative approach to changing null-co driver
> >     default 'read-zeroes' option to true:
> >     https://www.mail-archive.com/qemu-block@nongnu.org/msg80873.html
> >     <https://www.mail-archive.com/qemu-block@nongnu.org/msg80873.html>
> >
> >     Instead we introduce yet another block driver with an explicit
> >     name: 'zeroes-co'. We then clarify in secure-coding-practices.rst
> >     that security reports have to be sent using this new driver.
> >
> >     The 2nd patch is RFC because I won't spend time converting the
> >     tests until the first patch is discussed, as I already spent enough
> >     time doing that in the previous mentioned series.
> >
> >     Regards,
> >
> >     Phil.
> >
> >     Philippe Mathieu-Daudé (3):
> >       block: Introduce the 'zeroes-co' driver
> >       tests/test-blockjob: Use zeroes-co instead of
> null-co,read-zeroes=on
> >       docs/secure-coding-practices: Describe null-co/zeroes-co block
> drivers
> >
> >      docs/devel/secure-coding-practices.rst |   7 +
> >      block/zeroes.c                         | 306
> +++++++++++++++++++++++++
> >
> > Why not add another BlockDriver struct to block/null.c and set the
> > read_zeroes field in the .bdrv_file_open callback? It would make the
> > patch much simpler.
>
> This is the first patch I wrote but then realized you are listed as
> null-co maintainer, so you might be uninterested in having this new
> driver in the same file. And declaring the prototypes public to
> reuse them seemed overkill.
>
>
I posted a patch for block/null.c to add the same interface, just as an
alternative to the first patch.

I think we all agree that both zeroed and non-zeroed read mode will be
supported going forward, the divergence is on approach:

a) -blockdev null-co:// | -blockdev null-co://,read-zeroes=off,

if we make read-zeroes=on the default.

b) -blockdev null-co:// | -blockdev zero-co://,

if we keep the current default of null-co://, but introduce zero-co://
which has a clearer interface.

Fam

[-- Attachment #2: Type: text/html, Size: 3439 bytes --]