[pve-devel] [PATCH v2 qemu 8/9] cherry-pick stable fixes to avoid crash in IO error scenarios
Fiona Ebner
f.ebner at proxmox.com
Fri Oct 6 13:01:47 CEST 2023
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 at proxmox.com>
---
No changes in v2.
...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 at 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 at redhat.com>
+Message-Id: <20230824155345.109765-2-hreitz at redhat.com>
+Reviewed-by: Sam Li <faithilikerun at gmail.com>
+(cherry-picked from commit 56d1a022a77ea2125564913665eeadf3e303a671)
+Signed-off-by: Fiona Ebner <f.ebner at 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 at 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 at redhat.com>
+Message-Id: <20230824155345.109765-3-hreitz at redhat.com>
+Reviewed-by: Sam Li <faithilikerun at gmail.com>
+(cherry-picked from commit 4b5d80f3d02096a9bb1f651f6b3401ba40877159)
+Signed-off-by: Fiona Ebner <f.ebner at 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 at 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 at redhat.com>
+Message-Id: <20230824155345.109765-4-hreitz at redhat.com>
+Reviewed-by: Sam Li <faithilikerun at gmail.com>
+(cherry-picked from commit deab5c9a4ed74f76a713008a42527762b30a7e84)
+Signed-off-by: Fiona Ebner <f.ebner at 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 at 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 at redhat.com>
+Message-Id: <20230824155345.109765-5-hreitz at redhat.com>
+Reviewed-by: Sam Li <faithilikerun at gmail.com>
+(cherry-picked from commit d31b50a15dd25a560749b25fc40b6484fd1a57b7)
+Signed-off-by: Fiona Ebner <f.ebner at 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 at 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 at 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
More information about the pve-devel
mailing list