[pve-devel] [PATCH qemu] zeroinit: fix regression with filename parsing

[pve-devel] [PATCH qemu] zeroinit: fix regression with filename parsing

[Fiona Ebner]

As reported in the community forum [0], cloning or importing images
to RBD storages (without the krbd setting) was broken. This is a
result of no filename parsing happening anymore in bdrv_open_child()
after commit b242e7f ("backport fix for CVE-2024-4467"), which the
zeroinit relied on for passing along the RBD filename+key-value pairs.

There is a dedicated function for opening the file child which still
does filename parsing. Use that for opening the file child. Role and
flags should still be the same as with the manual bdrv_open_child(),
because the zeroinit driver is a filter, and the assignment bs->file
is also done by bdrv_open_file_child().

Fixes: b242e7f ("backport fix for CVE-2024-4467")
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 ...add-the-zeroinit-block-driver-filter.patch | 24 +++++++------------
 1 file changed, 9 insertions(+), 15 deletions(-)

diff --git a/debian/patches/pve/0019-PVE-block-add-the-zeroinit-block-driver-filter.patch b/debian/patches/pve/0019-PVE-block-add-the-zeroinit-block-driver-filter.patch
index 34a7efe..7464ca5 100644
--- a/debian/patches/pve/0019-PVE-block-add-the-zeroinit-block-driver-filter.patch
+++ b/debian/patches/pve/0019-PVE-block-add-the-zeroinit-block-driver-filter.patch
@@ -5,12 +5,13 @@ Subject: [PATCH] PVE: block: add the zeroinit block driver filter
 
 Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
 [FE: adapt to changed function signatures
-     adhere to block graph lock requirements]
+     adhere to block graph lock requirements
+     use dedicated function to open file child]
 Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
 ---
  block/meson.build |   1 +
- block/zeroinit.c  | 214 ++++++++++++++++++++++++++++++++++++++++++++++
- 2 files changed, 215 insertions(+)
+ block/zeroinit.c  | 207 ++++++++++++++++++++++++++++++++++++++++++++++
+ 2 files changed, 208 insertions(+)
  create mode 100644 block/zeroinit.c
 
 diff --git a/block/meson.build b/block/meson.build
@@ -27,10 +28,10 @@ index e1f03fd773..b530e117b5 100644
  system_ss.add(when: 'CONFIG_TCG', if_true: files('blkreplay.c'))
 diff --git a/block/zeroinit.c b/block/zeroinit.c
 new file mode 100644
-index 0000000000..696558d8d6
+index 0000000000..7998c9332d
 --- /dev/null
 +++ b/block/zeroinit.c
-@@ -0,0 +1,214 @@
+@@ -0,0 +1,207 @@
 +/*
 + * Filter to fake a zero-initialized block device.
 + *
@@ -96,7 +97,6 @@ index 0000000000..696558d8d6
 +                          Error **errp)
 +{
 +    BDRVZeroinitState *s = bs->opaque;
-+    BdrvChild *file = NULL;
 +    QemuOpts *opts;
 +    Error *local_err = NULL;
 +    int ret;
@@ -112,15 +112,9 @@ index 0000000000..696558d8d6
 +    }
 +
 +    /* Open the raw file */
-+    file = bdrv_open_child(qemu_opt_get(opts, "x-next"), options, "next", bs,
-+                           &child_of_bds,
-+                           BDRV_CHILD_FILTERED | BDRV_CHILD_PRIMARY, false,
-+                           &local_err);
-+    bdrv_graph_wrlock();
-+    bs->file = file;
-+    bdrv_graph_wrunlock();
-+    if (local_err) {
-+        ret = -EINVAL;
++    ret = bdrv_open_file_child(qemu_opt_get(opts, "x-next"), options, "next",
++                               bs, &local_err);
++    if (ret < 0) {
 +        error_propagate(errp, local_err);
 +        goto fail;
 +    }
-- 
2.39.2



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [PATCH qemu] zeroinit: fix regression with filename parsing

[Fiona Ebner]

Am 08.07.24 um 12:09 schrieb Fiona Ebner:
> As reported in the community forum [0], cloning or importing images
> to RBD storages (without the krbd setting) was broken. This is a
> result of no filename parsing happening anymore in bdrv_open_child()
> after commit b242e7f ("backport fix for CVE-2024-4467"), which the
> zeroinit relied on for passing along the RBD filename+key-value pairs.
> 
> There is a dedicated function for opening the file child which still
> does filename parsing. Use that for opening the file child. Role and
> flags should still be the same as with the manual bdrv_open_child(),
> because the zeroinit driver is a filter, and the assignment bs->file
> is also done by bdrv_open_file_child().
> 

Sorry, missed the link for community forum...

[0]:
https://forum.proxmox.com/threads/qemu-9-0-available-on-pve-no-subscription-as-of-now.149772/post-681620

> Fixes: b242e7f ("backport fix for CVE-2024-4467")
> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [PATCH qemu] zeroinit: fix regression with filename parsing

[Fiona Ebner]

Am 08.07.24 um 12:09 schrieb Fiona Ebner:
> As reported in the community forum [0], cloning or importing images
> to RBD storages (without the krbd setting) was broken. This is a
> result of no filename parsing happening anymore in bdrv_open_child()
> after commit b242e7f ("backport fix for CVE-2024-4467"), which the
> zeroinit relied on for passing along the RBD filename+key-value pairs.
> 
> There is a dedicated function for opening the file child which still
> does filename parsing. Use that for opening the file child. Role and
> flags should still be the same as with the manual bdrv_open_child(),
> because the zeroinit driver is a filter, and the assignment bs->file
> is also done by bdrv_open_file_child().
> 

Also forgot to mention for completeness that the PBS driver is not
affected, because it doesn't use bdrv_open_child() and the alloc-track
block driver is not affected, because it passes options to
bdrv_open_child() rather than a filename that potentially still needs to
be parsed.

> Fixes: b242e7f ("backport fix for CVE-2024-4467")
> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] applied: [PATCH qemu] zeroinit: fix regression with filename parsing

[Fabian Grünbichler]

with the missing link added to the commit message.

On July 8, 2024 12:09 pm, Fiona Ebner wrote:
> As reported in the community forum [0], cloning or importing images
> to RBD storages (without the krbd setting) was broken. This is a
> result of no filename parsing happening anymore in bdrv_open_child()
> after commit b242e7f ("backport fix for CVE-2024-4467"), which the
> zeroinit relied on for passing along the RBD filename+key-value pairs.
> 
> There is a dedicated function for opening the file child which still
> does filename parsing. Use that for opening the file child. Role and
> flags should still be the same as with the manual bdrv_open_child(),
> because the zeroinit driver is a filter, and the assignment bs->file
> is also done by bdrv_open_file_child().
> 
> Fixes: b242e7f ("backport fix for CVE-2024-4467")
> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
> ---
>  ...add-the-zeroinit-block-driver-filter.patch | 24 +++++++------------
>  1 file changed, 9 insertions(+), 15 deletions(-)
> 
> diff --git a/debian/patches/pve/0019-PVE-block-add-the-zeroinit-block-driver-filter.patch b/debian/patches/pve/0019-PVE-block-add-the-zeroinit-block-driver-filter.patch
> index 34a7efe..7464ca5 100644
> --- a/debian/patches/pve/0019-PVE-block-add-the-zeroinit-block-driver-filter.patch
> +++ b/debian/patches/pve/0019-PVE-block-add-the-zeroinit-block-driver-filter.patch
> @@ -5,12 +5,13 @@ Subject: [PATCH] PVE: block: add the zeroinit block driver filter
>  
>  Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
>  [FE: adapt to changed function signatures
> -     adhere to block graph lock requirements]
> +     adhere to block graph lock requirements
> +     use dedicated function to open file child]
>  Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
>  ---
>   block/meson.build |   1 +
> - block/zeroinit.c  | 214 ++++++++++++++++++++++++++++++++++++++++++++++
> - 2 files changed, 215 insertions(+)
> + block/zeroinit.c  | 207 ++++++++++++++++++++++++++++++++++++++++++++++
> + 2 files changed, 208 insertions(+)
>   create mode 100644 block/zeroinit.c
>  
>  diff --git a/block/meson.build b/block/meson.build
> @@ -27,10 +28,10 @@ index e1f03fd773..b530e117b5 100644
>   system_ss.add(when: 'CONFIG_TCG', if_true: files('blkreplay.c'))
>  diff --git a/block/zeroinit.c b/block/zeroinit.c
>  new file mode 100644
> -index 0000000000..696558d8d6
> +index 0000000000..7998c9332d
>  --- /dev/null
>  +++ b/block/zeroinit.c
> -@@ -0,0 +1,214 @@
> +@@ -0,0 +1,207 @@
>  +/*
>  + * Filter to fake a zero-initialized block device.
>  + *
> @@ -96,7 +97,6 @@ index 0000000000..696558d8d6
>  +                          Error **errp)
>  +{
>  +    BDRVZeroinitState *s = bs->opaque;
> -+    BdrvChild *file = NULL;
>  +    QemuOpts *opts;
>  +    Error *local_err = NULL;
>  +    int ret;
> @@ -112,15 +112,9 @@ index 0000000000..696558d8d6
>  +    }
>  +
>  +    /* Open the raw file */
> -+    file = bdrv_open_child(qemu_opt_get(opts, "x-next"), options, "next", bs,
> -+                           &child_of_bds,
> -+                           BDRV_CHILD_FILTERED | BDRV_CHILD_PRIMARY, false,
> -+                           &local_err);
> -+    bdrv_graph_wrlock();
> -+    bs->file = file;
> -+    bdrv_graph_wrunlock();
> -+    if (local_err) {
> -+        ret = -EINVAL;
> ++    ret = bdrv_open_file_child(qemu_opt_get(opts, "x-next"), options, "next",
> ++                               bs, &local_err);
> ++    if (ret < 0) {
>  +        error_propagate(errp, local_err);
>  +        goto fail;
>  +    }
> -- 
> 2.39.2
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 
> 


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel