[PATCH] qemuDomainChangeNet: forbid changing portgroup

Adam Julis posted 1 patch 2 months, 1 week ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/43502fd93d323c395bd649baf20e3b510d232ce4.1719580019.git.ajulis@redhat.com
There is a newer version of this series
src/qemu/qemu_hotplug.c | 7 +++++++
1 file changed, 7 insertions(+)
[PATCH] qemuDomainChangeNet: forbid changing portgroup
Posted by Adam Julis 2 months, 1 week ago
While changing the portgroup attribute causes incorrect
behavior, this option is disabled for hot-plug.

Resolves: https://issues.redhat.com/browse/RHEL-7299
Signed-off-by: Adam Julis <ajulis@redhat.com>
---
 src/qemu/qemu_hotplug.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 4a3f4f657e..355f742535 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -3884,6 +3884,13 @@ qemuDomainChangeNet(virQEMUDriver *driver,
         goto cleanup;
     }
 
+    if (newdev->type == VIR_DOMAIN_NET_TYPE_NETWORK &&
+        STRNEQ_NULLABLE(olddev->data.network.portgroup, newdev->data.network.portgroup)) {
+        virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+                       _("cannot modify network device portgroup attribute"));
+        goto cleanup;
+    }
+
     /* allocate new actual device to compare to old - we will need to
      * free it if we fail for any reason
      */
-- 
2.45.2
Re: [PATCH] qemuDomainChangeNet: forbid changing portgroup
Posted by Peter Krempa 2 months, 1 week ago
On Fri, Jun 28, 2024 at 15:07:26 +0200, Adam Julis wrote:
> While changing the portgroup attribute causes incorrect
> behavior, this option is disabled for hot-plug.
> 
> Resolves: https://issues.redhat.com/browse/RHEL-7299
> Signed-off-by: Adam Julis <ajulis@redhat.com>
> ---
>  src/qemu/qemu_hotplug.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 4a3f4f657e..355f742535 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -3884,6 +3884,13 @@ qemuDomainChangeNet(virQEMUDriver *driver,
>          goto cleanup;
>      }
>  
> +    if (newdev->type == VIR_DOMAIN_NET_TYPE_NETWORK &&

Note that at this point in the function it's not yet guaranteed that
'newdev->type' is equal to 'olddev->type'

> +        STRNEQ_NULLABLE(olddev->data.network.portgroup, newdev->data.network.portgroup)) {

so accessing the union members of olddev at this point is not safe as
they can be of a different type.

There's a code path inside the function that does this if both are
equal. Also note that the code allows backend change ('needReconnect'),
so there's a possibility that the config option can be in fact changed
(I didn't verify though).


> +        virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
> +                       _("cannot modify network device portgroup attribute"));
> +        goto cleanup;
> +    }
> +
>      /* allocate new actual device to compare to old - we will need to
>       * free it if we fail for any reason
>       */
> -- 
> 2.45.2
>