[PATCH 05/17] block: Don't advertise zero_init_truncate with encryption

[Eric Blake]

Commit 38841dcd correctly argued that having qcow2 blindly return 1
for .bdrv_has_zero_init() is wrong for preallocated images built on
block devices, while .bdrv_has_zero_init_truncate() can still return 1
because it is only relied on when changing size with PREALLOC_MODE_OFF
(and this is true even for v2 images which lack the notion of an
explicit zero cluster, since the block layer already filters out the
case of a larger backing file leaking through).  However, it missed
the fact that encrypted images do not default to reading as zero in
any case.

However, instead of changing qcow2's .bdrv_has_zero_init_truncate() to
point to a one-off function that special-cases bs->encryption, it is
smarter to just move the logic about encryption directly to the block
layer (that is, the driver callbacks will never be invoked for
encrypted images, just like they are already not called when a backing
file is present).  This solution fixes the qcow2 issue, has no effect
on the crypto driver (which already lacks .bdrv_has_zero_init*
callbacks), and no other driver currently uses bs->encrypted.

One other reason to fix this at the block layer: any information we
expose about an encrypted image that in turn may alter timing of
algorithms run on that image can be considered a (slight) information
leak; refusing to optimize zero handling of encrypted images thus
avoids the possibility of that being a security concern.

Signed-off-by: Eric Blake <address@hidden>
---
 block.c       | 19 ++++++++++++++++---
 block/qcow2.c |  2 --
 2 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/block.c b/block.c
index 6c2b2bd2e292..296845040e59 100644
--- a/block.c
+++ b/block.c
@@ -5077,9 +5077,12 @@ int bdrv_has_zero_init(BlockDriverState *bs)
         return 0;
     }

-    /* If BS is a copy on write image, it is initialized to
-       the contents of the base image, which may not be zeroes.  */
-    if (bs->backing) {
+    /*
+     * If BS is a copy on write image, it is initialized to the
+     * contents of the base image, which may not be zeroes.  Likewise,
+     * encrypted images do not read as zero.
+     */
+    if (bs->backing || bs->encrypted) {
         return 0;
     }
     if (bs->drv->bdrv_has_zero_init) {
@@ -5099,6 +5102,16 @@ int bdrv_has_zero_init_truncate(BlockDriverState *bs)
         return 0;
     }

+    /*
+     * Encrypted images never default to reading all zero; and even if
+     * they did, advertising that fact might lead to an information
+     * leak based on timing comparisons of algorithms that change if
+     * our result were dynamic.
+     */
+    if (bs->encrypted) {
+        return 0;
+    }
+
     if (bs->backing) {
         /* Depends on the backing image length, but better safe than sorry */
         return 0;
diff --git a/block/qcow2.c b/block/qcow2.c
index 6ea06dbdf48a..40aa751d1de7 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -4934,8 +4934,6 @@ static int qcow2_has_zero_init(BlockDriverState *bs)

     if (!preallocated) {
         return 1;
-    } else if (bs->encrypted) {
-        return 0;
     } else {
         return bdrv_has_zero_init(s->data_file->bs);
     }
-- 
2.24.1