[pve-devel] [PATCH qemu 8/8] cherry-pick stable fixes to avoid crash in IO error scenarios

[Fiona Ebner <f.ebner@proxmox.com>]

Upstream report of the issue [0]. I ran into it too by chance by
filling up my NFS storage with the VM's qcow2 disk.

[0]: https://bugzilla.redhat.com/show_bug.cgi?id=2234374

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 ...ile-posix-Clear-bs-bl.zoned-on-error.patch | 87 +++++++++++++++++++
 ...osix-Check-bs-bl.zoned-for-zone-info.patch | 59 +++++++++++++
 ...ix-Fix-zone-update-in-I-O-error-path.patch | 33 +++++++
 ...-Simplify-raw_co_prw-s-out-zone-code.patch | 58 +++++++++++++
 ...k-file-change-locking-default-to-off.patch |  2 +-
 ...le-posix-make-locking-optiono-on-cre.patch | 14 +--
 debian/patches/series                         |  4 +
 7 files changed, 249 insertions(+), 8 deletions(-)
 create mode 100644 debian/patches/extra/0008-file-posix-Clear-bs-bl.zoned-on-error.patch
 create mode 100644 debian/patches/extra/0009-file-posix-Check-bs-bl.zoned-for-zone-info.patch
 create mode 100644 debian/patches/extra/0010-file-posix-Fix-zone-update-in-I-O-error-path.patch
 create mode 100644 debian/patches/extra/0011-file-posix-Simplify-raw_co_prw-s-out-zone-code.patch

diff --git a/debian/patches/extra/0008-file-posix-Clear-bs-bl.zoned-on-error.patch b/debian/patches/extra/0008-file-posix-Clear-bs-bl.zoned-on-error.patch
new file mode 100644
index 0000000..0e3dc3e
--- /dev/null
+++ b/debian/patches/extra/0008-file-posix-Clear-bs-bl.zoned-on-error.patch
@@ -0,0 +1,87 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Hanna Czenczek <hreitz@redhat.com>
+Date: Thu, 24 Aug 2023 17:53:40 +0200
+Subject: [PATCH] file-posix: Clear bs->bl.zoned on error
+
+bs->bl.zoned is what indicates whether the zone information is present
+and valid; it is the only thing that raw_refresh_zoned_limits() sets if
+CONFIG_BLKZONED is not defined, and it is also the only thing that it
+sets if CONFIG_BLKZONED is defined, but there are no zones.
+
+Make sure that it is always set to BLK_Z_NONE if there is an error
+anywhere in raw_refresh_zoned_limits() so that we do not accidentally
+announce zones while our information is incomplete or invalid.
+
+This also fixes a memory leak in the last error path in
+raw_refresh_zoned_limits().
+
+Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
+Message-Id: <20230824155345.109765-2-hreitz@redhat.com>
+Reviewed-by: Sam Li <faithilikerun@gmail.com>
+(cherry-picked from commit 56d1a022a77ea2125564913665eeadf3e303a671)
+Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
+---
+ block/file-posix.c | 21 ++++++++++++---------
+ 1 file changed, 12 insertions(+), 9 deletions(-)
+
+diff --git a/block/file-posix.c b/block/file-posix.c
+index b16e9c21a1..2b88b9eefa 100644
+--- a/block/file-posix.c
++++ b/block/file-posix.c
+@@ -1412,11 +1412,9 @@ static void raw_refresh_zoned_limits(BlockDriverState *bs, struct stat *st,
+     BlockZoneModel zoned;
+     int ret;
+ 
+-    bs->bl.zoned = BLK_Z_NONE;
+-
+     ret = get_sysfs_zoned_model(st, &zoned);
+     if (ret < 0 || zoned == BLK_Z_NONE) {
+-        return;
++        goto no_zoned;
+     }
+     bs->bl.zoned = zoned;
+ 
+@@ -1437,10 +1435,10 @@ static void raw_refresh_zoned_limits(BlockDriverState *bs, struct stat *st,
+     if (ret < 0) {
+         error_setg_errno(errp, -ret, "Unable to read chunk_sectors "
+                                      "sysfs attribute");
+-        return;
++        goto no_zoned;
+     } else if (!ret) {
+         error_setg(errp, "Read 0 from chunk_sectors sysfs attribute");
+-        return;
++        goto no_zoned;
+     }
+     bs->bl.zone_size = ret << BDRV_SECTOR_BITS;
+ 
+@@ -1448,10 +1446,10 @@ static void raw_refresh_zoned_limits(BlockDriverState *bs, struct stat *st,
+     if (ret < 0) {
+         error_setg_errno(errp, -ret, "Unable to read nr_zones "
+                                      "sysfs attribute");
+-        return;
++        goto no_zoned;
+     } else if (!ret) {
+         error_setg(errp, "Read 0 from nr_zones sysfs attribute");
+-        return;
++        goto no_zoned;
+     }
+     bs->bl.nr_zones = ret;
+ 
+@@ -1472,10 +1470,15 @@ static void raw_refresh_zoned_limits(BlockDriverState *bs, struct stat *st,
+     ret = get_zones_wp(bs, s->fd, 0, bs->bl.nr_zones, 0);
+     if (ret < 0) {
+         error_setg_errno(errp, -ret, "report wps failed");
+-        bs->wps = NULL;
+-        return;
++        goto no_zoned;
+     }
+     qemu_co_mutex_init(&bs->wps->colock);
++    return;
++
++no_zoned:
++    bs->bl.zoned = BLK_Z_NONE;
++    g_free(bs->wps);
++    bs->wps = NULL;
+ }
+ #else /* !defined(CONFIG_BLKZONED) */
+ static void raw_refresh_zoned_limits(BlockDriverState *bs, struct stat *st,
diff --git a/debian/patches/extra/0009-file-posix-Check-bs-bl.zoned-for-zone-info.patch b/debian/patches/extra/0009-file-posix-Check-bs-bl.zoned-for-zone-info.patch
new file mode 100644
index 0000000..a505816
--- /dev/null
+++ b/debian/patches/extra/0009-file-posix-Check-bs-bl.zoned-for-zone-info.patch
@@ -0,0 +1,59 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Hanna Czenczek <hreitz@redhat.com>
+Date: Thu, 24 Aug 2023 17:53:41 +0200
+Subject: [PATCH] file-posix: Check bs->bl.zoned for zone info
+
+Instead of checking bs->wps or bs->bl.zone_size for whether zone
+information is present, check bs->bl.zoned.  That is the flag that
+raw_refresh_zoned_limits() reliably sets to indicate zone support.  If
+it is set to something other than BLK_Z_NONE, other values and objects
+like bs->wps and bs->bl.zone_size must be non-null/zero and valid; if it
+is not, we cannot rely on their validity.
+
+Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
+Message-Id: <20230824155345.109765-3-hreitz@redhat.com>
+Reviewed-by: Sam Li <faithilikerun@gmail.com>
+(cherry-picked from commit 4b5d80f3d02096a9bb1f651f6b3401ba40877159)
+Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
+---
+ block/file-posix.c | 12 +++++++-----
+ 1 file changed, 7 insertions(+), 5 deletions(-)
+
+diff --git a/block/file-posix.c b/block/file-posix.c
+index 2b88b9eefa..46e22403fe 100644
+--- a/block/file-posix.c
++++ b/block/file-posix.c
+@@ -2455,9 +2455,10 @@ static int coroutine_fn raw_co_prw(BlockDriverState *bs, uint64_t offset,
+     if (fd_open(bs) < 0)
+         return -EIO;
+ #if defined(CONFIG_BLKZONED)
+-    if ((type & (QEMU_AIO_WRITE | QEMU_AIO_ZONE_APPEND)) && bs->wps) {
++    if ((type & (QEMU_AIO_WRITE | QEMU_AIO_ZONE_APPEND)) &&
++        bs->bl.zoned != BLK_Z_NONE) {
+         qemu_co_mutex_lock(&bs->wps->colock);
+-        if (type & QEMU_AIO_ZONE_APPEND && bs->bl.zone_size) {
++        if (type & QEMU_AIO_ZONE_APPEND) {
+             int index = offset / bs->bl.zone_size;
+             offset = bs->wps->wp[index];
+         }
+@@ -2508,8 +2509,8 @@ out:
+ {
+     BlockZoneWps *wps = bs->wps;
+     if (ret == 0) {
+-        if ((type & (QEMU_AIO_WRITE | QEMU_AIO_ZONE_APPEND))
+-            && wps && bs->bl.zone_size) {
++        if ((type & (QEMU_AIO_WRITE | QEMU_AIO_ZONE_APPEND)) &&
++            bs->bl.zoned != BLK_Z_NONE) {
+             uint64_t *wp = &wps->wp[offset / bs->bl.zone_size];
+             if (!BDRV_ZT_IS_CONV(*wp)) {
+                 if (type & QEMU_AIO_ZONE_APPEND) {
+@@ -2529,7 +2530,8 @@ out:
+         }
+     }
+ 
+-    if ((type & (QEMU_AIO_WRITE | QEMU_AIO_ZONE_APPEND)) && wps) {
++    if ((type & (QEMU_AIO_WRITE | QEMU_AIO_ZONE_APPEND)) &&
++        bs->blk.zoned != BLK_Z_NONE) {
+         qemu_co_mutex_unlock(&wps->colock);
+     }
+ }
diff --git a/debian/patches/extra/0010-file-posix-Fix-zone-update-in-I-O-error-path.patch b/debian/patches/extra/0010-file-posix-Fix-zone-update-in-I-O-error-path.patch
new file mode 100644
index 0000000..fb32c62
--- /dev/null
+++ b/debian/patches/extra/0010-file-posix-Fix-zone-update-in-I-O-error-path.patch
@@ -0,0 +1,33 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Hanna Czenczek <hreitz@redhat.com>
+Date: Thu, 24 Aug 2023 17:53:42 +0200
+Subject: [PATCH] file-posix: Fix zone update in I/O error path
+
+We must check that zone information is present before running
+update_zones_wp().
+
+Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=2234374
+Fixes: Coverity CID 1512459
+Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
+Message-Id: <20230824155345.109765-4-hreitz@redhat.com>
+Reviewed-by: Sam Li <faithilikerun@gmail.com>
+(cherry-picked from commit deab5c9a4ed74f76a713008a42527762b30a7e84)
+Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
+---
+ block/file-posix.c | 3 ++-
+ 1 file changed, 2 insertions(+), 1 deletion(-)
+
+diff --git a/block/file-posix.c b/block/file-posix.c
+index 46e22403fe..a050682e97 100644
+--- a/block/file-posix.c
++++ b/block/file-posix.c
+@@ -2525,7 +2525,8 @@ out:
+             }
+         }
+     } else {
+-        if (type & (QEMU_AIO_WRITE | QEMU_AIO_ZONE_APPEND)) {
++        if ((type & (QEMU_AIO_WRITE | QEMU_AIO_ZONE_APPEND)) &&
++            bs->bl.zoned != BLK_Z_NONE) {
+             update_zones_wp(bs, s->fd, 0, 1);
+         }
+     }
diff --git a/debian/patches/extra/0011-file-posix-Simplify-raw_co_prw-s-out-zone-code.patch b/debian/patches/extra/0011-file-posix-Simplify-raw_co_prw-s-out-zone-code.patch
new file mode 100644
index 0000000..6164160
--- /dev/null
+++ b/debian/patches/extra/0011-file-posix-Simplify-raw_co_prw-s-out-zone-code.patch
@@ -0,0 +1,58 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Hanna Czenczek <hreitz@redhat.com>
+Date: Thu, 24 Aug 2023 17:53:43 +0200
+Subject: [PATCH] file-posix: Simplify raw_co_prw's 'out' zone code
+
+We duplicate the same condition three times here, pull it out to the top
+level.
+
+Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
+Message-Id: <20230824155345.109765-5-hreitz@redhat.com>
+Reviewed-by: Sam Li <faithilikerun@gmail.com>
+(cherry-picked from commit d31b50a15dd25a560749b25fc40b6484fd1a57b7)
+Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
+---
+ block/file-posix.c | 18 +++++-------------
+ 1 file changed, 5 insertions(+), 13 deletions(-)
+
+diff --git a/block/file-posix.c b/block/file-posix.c
+index a050682e97..aa89789737 100644
+--- a/block/file-posix.c
++++ b/block/file-posix.c
+@@ -2506,11 +2506,10 @@ static int coroutine_fn raw_co_prw(BlockDriverState *bs, uint64_t offset,
+ 
+ out:
+ #if defined(CONFIG_BLKZONED)
+-{
+-    BlockZoneWps *wps = bs->wps;
+-    if (ret == 0) {
+-        if ((type & (QEMU_AIO_WRITE | QEMU_AIO_ZONE_APPEND)) &&
+-            bs->bl.zoned != BLK_Z_NONE) {
++    if ((type & (QEMU_AIO_WRITE | QEMU_AIO_ZONE_APPEND)) &&
++        bs->bl.zoned != BLK_Z_NONE) {
++        BlockZoneWps *wps = bs->wps;
++        if (ret == 0) {
+             uint64_t *wp = &wps->wp[offset / bs->bl.zone_size];
+             if (!BDRV_ZT_IS_CONV(*wp)) {
+                 if (type & QEMU_AIO_ZONE_APPEND) {
+@@ -2523,19 +2522,12 @@ out:
+                     *wp = offset + bytes;
+                 }
+             }
+-        }
+-    } else {
+-        if ((type & (QEMU_AIO_WRITE | QEMU_AIO_ZONE_APPEND)) &&
+-            bs->bl.zoned != BLK_Z_NONE) {
++        } else {
+             update_zones_wp(bs, s->fd, 0, 1);
+         }
+-    }
+ 
+-    if ((type & (QEMU_AIO_WRITE | QEMU_AIO_ZONE_APPEND)) &&
+-        bs->blk.zoned != BLK_Z_NONE) {
+         qemu_co_mutex_unlock(&wps->colock);
+     }
+-}
+ #endif
+     return ret;
+ }
diff --git a/debian/patches/pve/0001-PVE-Config-block-file-change-locking-default-to-off.patch b/debian/patches/pve/0001-PVE-Config-block-file-change-locking-default-to-off.patch
index 1c3d08c..3d8785c 100644
--- a/debian/patches/pve/0001-PVE-Config-block-file-change-locking-default-to-off.patch
+++ b/debian/patches/pve/0001-PVE-Config-block-file-change-locking-default-to-off.patch
@@ -14,7 +14,7 @@ Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
  1 file changed, 2 insertions(+), 2 deletions(-)
 
 diff --git a/block/file-posix.c b/block/file-posix.c
-index b16e9c21a1..26aa0172b4 100644
+index aa89789737..0db366a851 100644
 --- a/block/file-posix.c
 +++ b/block/file-posix.c
 @@ -564,7 +564,7 @@ static QemuOptsList raw_runtime_opts = {
diff --git a/debian/patches/pve/0022-PVE-Up-Config-file-posix-make-locking-optiono-on-cre.patch b/debian/patches/pve/0022-PVE-Up-Config-file-posix-make-locking-optiono-on-cre.patch
index 904f76e..766c4f9 100644
--- a/debian/patches/pve/0022-PVE-Up-Config-file-posix-make-locking-optiono-on-cre.patch
+++ b/debian/patches/pve/0022-PVE-Up-Config-file-posix-make-locking-optiono-on-cre.patch
@@ -13,10 +13,10 @@ Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
  2 files changed, 42 insertions(+), 20 deletions(-)
 
 diff --git a/block/file-posix.c b/block/file-posix.c
-index 26aa0172b4..504c6ed316 100644
+index 0db366a851..46f1ee38ae 100644
 --- a/block/file-posix.c
 +++ b/block/file-posix.c
-@@ -2872,6 +2872,7 @@ raw_co_create(BlockdevCreateOptions *options, Error **errp)
+@@ -2870,6 +2870,7 @@ raw_co_create(BlockdevCreateOptions *options, Error **errp)
      int fd;
      uint64_t perm, shared;
      int result = 0;
@@ -24,7 +24,7 @@ index 26aa0172b4..504c6ed316 100644
  
      /* Validate options and set default values */
      assert(options->driver == BLOCKDEV_DRIVER_FILE);
-@@ -2912,19 +2913,22 @@ raw_co_create(BlockdevCreateOptions *options, Error **errp)
+@@ -2910,19 +2911,22 @@ raw_co_create(BlockdevCreateOptions *options, Error **errp)
      perm = BLK_PERM_WRITE | BLK_PERM_RESIZE;
      shared = BLK_PERM_ALL & ~BLK_PERM_RESIZE;
  
@@ -59,7 +59,7 @@ index 26aa0172b4..504c6ed316 100644
      }
  
      /* Clear the file by truncating it to 0 */
-@@ -2978,13 +2982,15 @@ raw_co_create(BlockdevCreateOptions *options, Error **errp)
+@@ -2976,13 +2980,15 @@ raw_co_create(BlockdevCreateOptions *options, Error **errp)
      }
  
  out_unlock:
@@ -82,7 +82,7 @@ index 26aa0172b4..504c6ed316 100644
      }
  
  out_close:
-@@ -3008,6 +3014,7 @@ raw_co_create_opts(BlockDriver *drv, const char *filename,
+@@ -3006,6 +3012,7 @@ raw_co_create_opts(BlockDriver *drv, const char *filename,
      PreallocMode prealloc;
      char *buf = NULL;
      Error *local_err = NULL;
@@ -90,7 +90,7 @@ index 26aa0172b4..504c6ed316 100644
  
      /* Skip file: protocol prefix */
      strstart(filename, "file:", &filename);
-@@ -3030,6 +3037,18 @@ raw_co_create_opts(BlockDriver *drv, const char *filename,
+@@ -3028,6 +3035,18 @@ raw_co_create_opts(BlockDriver *drv, const char *filename,
          return -EINVAL;
      }
  
@@ -109,7 +109,7 @@ index 26aa0172b4..504c6ed316 100644
      options = (BlockdevCreateOptions) {
          .driver     = BLOCKDEV_DRIVER_FILE,
          .u.file     = {
-@@ -3041,6 +3060,8 @@ raw_co_create_opts(BlockDriver *drv, const char *filename,
+@@ -3039,6 +3058,8 @@ raw_co_create_opts(BlockDriver *drv, const char *filename,
              .nocow              = nocow,
              .has_extent_size_hint = has_extent_size_hint,
              .extent_size_hint   = extent_size_hint,
diff --git a/debian/patches/series b/debian/patches/series
index c27c245..71f7e01 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -5,6 +5,10 @@ extra/0004-migration-block-dirty-bitmap-fix-loading-bitmap-when.patch
 extra/0005-hw-ide-reset-cancel-async-DMA-operation-before-reset.patch
 extra/0006-Revert-Revert-graph-lock-Disable-locking-for-now.patch
 extra/0007-migration-states-workaround-snapshot-performance-reg.patch
+extra/0008-file-posix-Clear-bs-bl.zoned-on-error.patch
+extra/0009-file-posix-Check-bs-bl.zoned-for-zone-info.patch
+extra/0010-file-posix-Fix-zone-update-in-I-O-error-path.patch
+extra/0011-file-posix-Simplify-raw_co_prw-s-out-zone-code.patch
 bitmap-mirror/0001-drive-mirror-add-support-for-sync-bitmap-mode-never.patch
 bitmap-mirror/0002-drive-mirror-add-support-for-conditional-and-always-.patch
 bitmap-mirror/0003-mirror-add-check-for-bitmap-mode-without-bitmap.patch
-- 
2.39.2