From daa60c8252d0e955acbfe9eaca52dbb272230d80 Mon Sep 17 00:00:00 2001
From: Wido den Hollander <wido@widodh.nl>
Date: Fri, 16 Oct 2026 12:06:03 +0000
Subject: [PATCH] kvm: only emit known video and CPU model names in the domain
 XML

VideoDef now maps the video.hardware detail through an enum of the model types libvirt accepts, and CpuModeDef only accepts plain QEMU model names for guest.cpu.model. Unknown values are ignored with a warning and the agent default is used instead, like the agent already does for disk controllers, NIC models and the TPM.
---
 .../resource/LibvirtComputingResource.java    | 31 ++++++---
 .../hypervisor/kvm/resource/LibvirtVMDef.java | 63 ++++++++++++++++++-
 .../LibvirtComputingResourceTest.java         | 43 ++++++++++++-
 .../kvm/resource/LibvirtVMDefTest.java        | 31 +++++++++
 4 files changed, 156 insertions(+), 12 deletions(-)

diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java
index 7c3f826be171..4c3fc56b6843 100644
--- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java
+++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java
@@ -332,7 +332,7 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv
     private static final String KVMCLOCK = "kvmclock";
     private static final String HYPERVCLOCK = "hypervclock";
     private static final String WINDOWS = "Windows";
-    private static final String X86_DEFAULT_VIDEO_MODEL = "vga";
+    private static final VideoDef.VideoModel X86_DEFAULT_VIDEO_MODEL = VideoDef.VideoModel.VGA;
     private static final int X86_DEFAULT_VIDEO_RAM_KIB = 32768;
     private static final String Q35 = "q35";
     private static final String PTY = "pty";
@@ -3352,26 +3352,35 @@ protected ConsoleDef createConsoleDef() {
 
     protected VideoDef createVideoDef(VirtualMachineTO vmTO) {
         Map<String, String> details = vmTO.getDetails();
-        String videoHw = this.videoHw;
+        VideoDef.VideoModel videoModel = VideoDef.VideoModel.fromString(this.videoHw);
+        if (videoModel == null && StringUtils.isNotBlank(this.videoHw)) {
+            LOGGER.warn("Ignoring unsupported video model '{}' from {}", this.videoHw, AgentProperties.VM_VIDEO_HARDWARE.getName());
+        }
         int videoRam = this.videoRam;
         if (details != null) {
             if (details.containsKey(VmDetailConstants.VIDEO_HARDWARE)) {
-                videoHw = details.get(VmDetailConstants.VIDEO_HARDWARE);
+                String requested = details.get(VmDetailConstants.VIDEO_HARDWARE);
+                VideoDef.VideoModel requestedModel = VideoDef.VideoModel.fromString(requested);
+                if (requestedModel != null) {
+                    videoModel = requestedModel;
+                } else {
+                    LOGGER.warn("Ignoring unsupported {} '{}' of VM {}", VmDetailConstants.VIDEO_HARDWARE, requested, vmTO.getName());
+                }
             }
             if (details.containsKey(VmDetailConstants.VIDEO_RAM)) {
                 String value = details.get(VmDetailConstants.VIDEO_RAM);
                 videoRam = NumbersUtil.parseInt(value, videoRam);
             }
         }
-        if (StringUtils.isBlank(videoHw) && isGuestX86(vmTO)) {
+        if (videoModel == null && isGuestX86(vmTO)) {
             // With no <video> element libvirt defaults x86 guests to cirrus, which is deprecated
             // in QEMU and renders a blank console on recent Windows guests (e.g. Windows Server 2025 Core)
-            videoHw = X86_DEFAULT_VIDEO_MODEL;
+            videoModel = X86_DEFAULT_VIDEO_MODEL;
             if (videoRam == 0) {
                 videoRam = X86_DEFAULT_VIDEO_RAM_KIB;
             }
         }
-        return new VideoDef(videoHw, videoRam);
+        return new VideoDef(videoModel, videoRam);
     }
 
     protected RngDef createRngDef() {
@@ -3472,7 +3481,15 @@ private CpuModeDef createCpuModeDef(VirtualMachineTO vmTO, int vcpus) {
         final CpuModeDef cmd = new CpuModeDef();
         Map<String, String> details = vmTO.getDetails();
         String cpuMode = MapUtils.isNotEmpty(details) && details.get(VmDetailConstants.GUEST_CPU_MODE) != null ? details.get(VmDetailConstants.GUEST_CPU_MODE) : guestCpuMode;
-        String cpuModel = MapUtils.isNotEmpty(details) && details.get(VmDetailConstants.GUEST_CPU_MODEL) != null ? details.get(VmDetailConstants.GUEST_CPU_MODEL) : guestCpuModel;
+        String cpuModel = guestCpuModel;
+        if (MapUtils.isNotEmpty(details) && details.get(VmDetailConstants.GUEST_CPU_MODEL) != null) {
+            String requested = details.get(VmDetailConstants.GUEST_CPU_MODEL);
+            if (CpuModeDef.isValidModelName(requested)) {
+                cpuModel = requested;
+            } else {
+                LOGGER.warn("Ignoring invalid {} '{}' of VM {}", VmDetailConstants.GUEST_CPU_MODEL, requested, vmTO.getName());
+            }
+        }
         cmd.setMode(cpuMode);
         cmd.setModel(cpuModel);
         cmd.setFeatures(cpuFeatures);
diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDef.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDef.java
index 439e4f663416..353b068faebb 100644
--- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDef.java
+++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDef.java
@@ -26,6 +26,7 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.regex.Pattern;
 
 import com.cloud.cpu.CPU;
 import org.apache.cloudstack.api.ApiConstants.IoDriverPolicy;
@@ -1994,6 +1995,12 @@ public String toString() {
     }
 
     public static class CpuModeDef {
+        /**
+         * CPU model names as QEMU and libvirt define them, such as Nehalem, EPYC-Rome-v2 or x86-64-v2.
+         * The model reaches the domain XML from a VM detail, so anything else is rejected.
+         */
+        private static final Pattern CPU_MODEL_NAME = Pattern.compile("^[A-Za-z0-9][A-Za-z0-9._+-]*$");
+
         private String _mode;
         private String _model;
         private List<String> _features;
@@ -2001,6 +2008,10 @@ public static class CpuModeDef {
         private int _threadsPerCore = -1;
         private int _sockets = -1;
 
+        public static boolean isValidModelName(String model) {
+            return model != null && CPU_MODEL_NAME.matcher(model).matches();
+        }
+
         public void setMode(String mode) {
             _mode = mode;
         }
@@ -2011,7 +2022,14 @@ public void setFeatures(List<String> features) {
             }
         }
 
+        /**
+         * Sets the CPU model for the custom mode. A name that is not a plain QEMU model name is ignored.
+         */
         public void setModel(String model) {
+            if (model != null && !isValidModelName(model)) {
+                LOGGER.warn("Ignoring CPU model '{}' because it is not a valid model name", model);
+                return;
+            }
             _model = model;
         }
 
@@ -2090,18 +2108,57 @@ public String toString() {
     }
 
     public static class VideoDef {
-        private String _videoModel;
+        /**
+         * Video model types accepted by libvirt. The model name reaches the domain XML
+         * from a VM detail, so only values from this list are ever emitted.
+         */
+        public enum VideoModel {
+            VGA("vga"), CIRRUS("cirrus"), VMVGA("vmvga"), XEN("xen"), VBOX("vbox"), QXL("qxl"), VIRTIO("virtio"), GOP("gop"), BOCHS("bochs"), RAMFB("ramfb"), NONE("none");
+
+            private final String model;
+
+            VideoModel(String model) {
+                this.model = model;
+            }
+
+            @Override
+            public String toString() {
+                return model;
+            }
+
+            /**
+             * Returns the matching model, ignoring case, or null when the value is blank or unknown.
+             */
+            public static VideoModel fromString(String model) {
+                if (StringUtils.isBlank(model)) {
+                    return null;
+                }
+                String trimmed = model.trim();
+                for (VideoModel videoModel : values()) {
+                    if (videoModel.model.equalsIgnoreCase(trimmed)) {
+                        return videoModel;
+                    }
+                }
+                return null;
+            }
+        }
+
+        private VideoModel _videoModel;
         private int _videoRam;
 
-        public VideoDef(String videoModel, int videoRam) {
+        public VideoDef(VideoModel videoModel, int videoRam) {
             _videoModel = videoModel;
             _videoRam = videoRam;
         }
 
+        public VideoModel getVideoModel() {
+            return _videoModel;
+        }
+
         @Override
         public String toString() {
             StringBuilder videoBuilder = new StringBuilder();
-            if (_videoModel != null && !_videoModel.isEmpty()){
+            if (_videoModel != null) {
                 videoBuilder.append("<video>\n");
                 if (_videoRam != 0) {
                     videoBuilder.append("<model type='" + _videoModel + "' vram='" + _videoRam + "'/>\n");
diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java
index 33fb377bc90e..ff57dfdb1b06 100644
--- a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java
+++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java
@@ -705,14 +705,53 @@ public void testCreateConsoleDef() {
     public void testCreateVideoDef() {
         VirtualMachineTO to = createDefaultVM(false);
         libvirtComputingResourceSpy.videoRam = 200;
-        libvirtComputingResourceSpy.videoHw = "vGPU";
+        libvirtComputingResourceSpy.videoHw = "VmVGA";
 
         VideoDef videoDef = libvirtComputingResourceSpy.createVideoDef(to);
         Document domainDoc = parse(videoDef.toString());
-        assertXpath(domainDoc, "/video/model/@type", "vGPU");
+        assertXpath(domainDoc, "/video/model/@type", "vmvga");
         assertXpath(domainDoc, "/video/model/@vram", "200");
     }
 
+    @Test
+    public void testCreateVideoDefIgnoresUnsupportedAgentProperty() {
+        VirtualMachineTO to = createDefaultVM(false);
+        libvirtComputingResourceSpy.videoHw = "vGPU";
+
+        VideoDef videoDef = libvirtComputingResourceSpy.createVideoDef(to);
+        Document domainDoc = parse(videoDef.toString());
+        assertXpath(domainDoc, "/video/model/@type", "vga");
+        assertXpath(domainDoc, "/video/model/@vram", "32768");
+    }
+
+    @Test
+    public void testCreateVideoDefIgnoresInjectedDetail() {
+        VirtualMachineTO to = createDefaultVM(false);
+        Map<String, String> details = new HashMap<>();
+        details.put(VmDetailConstants.VIDEO_HARDWARE, "virtio'/></video><disk type='file' device='disk'><driver name='qemu' type='raw'/>"
+                + "<source file='/etc/shadow'/><target dev='vdz' bus='virtio'/></disk><video><model type='virtio");
+        to.setDetails(details);
+
+        VideoDef videoDef = libvirtComputingResourceSpy.createVideoDef(to);
+        String xml = videoDef.toString();
+        assertFalse(xml.contains("<disk"));
+        Document domainDoc = parse(xml);
+        assertXpath(domainDoc, "/video/model/@type", "vga");
+    }
+
+    @Test
+    public void testCreateVideoDefUnknownDetailKeepsAgentProperty() {
+        VirtualMachineTO to = createDefaultVM(false);
+        libvirtComputingResourceSpy.videoHw = "qxl";
+        Map<String, String> details = new HashMap<>();
+        details.put(VmDetailConstants.VIDEO_HARDWARE, "bogus");
+        to.setDetails(details);
+
+        VideoDef videoDef = libvirtComputingResourceSpy.createVideoDef(to);
+        Document domainDoc = parse(videoDef.toString());
+        assertXpath(domainDoc, "/video/model/@type", "qxl");
+    }
+
     @Test
     public void testCreateVideoDefX86DefaultsToVga() {
         VirtualMachineTO to = createDefaultVM(false);
diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDefTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDefTest.java
index 56ad267eac7e..a6c3301e01f0 100644
--- a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDefTest.java
+++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDefTest.java
@@ -213,6 +213,37 @@ public void testCpuModeDef() {
 
     }
 
+    @Test
+    public void testCpuModeDefIgnoresInvalidModelName() {
+        LibvirtVMDef.CpuModeDef cpuModeDef = new LibvirtVMDef.CpuModeDef();
+        cpuModeDef.setMode("custom");
+        cpuModeDef.setModel("Nehalem</model></cpu><devices><disk type='file'/></devices><cpu><model>x");
+
+        assertEquals("<cpu></cpu>", cpuModeDef.toString());
+        assertTrue(LibvirtVMDef.CpuModeDef.isValidModelName("EPYC-Rome-v2"));
+        assertTrue(LibvirtVMDef.CpuModeDef.isValidModelName("x86-64-v2"));
+        assertTrue(LibvirtVMDef.CpuModeDef.isValidModelName("Skylake-Client-noTSX-IBRS"));
+        assertFalse(LibvirtVMDef.CpuModeDef.isValidModelName(""));
+        assertFalse(LibvirtVMDef.CpuModeDef.isValidModelName("-Nehalem"));
+        assertFalse(LibvirtVMDef.CpuModeDef.isValidModelName("Nehalem Broadwell"));
+        assertFalse(LibvirtVMDef.CpuModeDef.isValidModelName("Nehalem<"));
+    }
+
+    @Test
+    public void testVideoDefOnlyEmitsKnownModels() {
+        assertEquals(LibvirtVMDef.VideoDef.VideoModel.VIRTIO, LibvirtVMDef.VideoDef.VideoModel.fromString("VirtIO"));
+        assertEquals(LibvirtVMDef.VideoDef.VideoModel.QXL, LibvirtVMDef.VideoDef.VideoModel.fromString(" qxl "));
+        assertNull(LibvirtVMDef.VideoDef.VideoModel.fromString(null));
+        assertNull(LibvirtVMDef.VideoDef.VideoModel.fromString(""));
+        assertNull(LibvirtVMDef.VideoDef.VideoModel.fromString("virtio'/></video><disk type='file'/><video><model type='virtio"));
+
+        assertEquals("", new LibvirtVMDef.VideoDef(null, 65536).toString());
+        assertEquals("<video>\n<model type='virtio' vram='65536'/>\n</video>\n",
+                new LibvirtVMDef.VideoDef(LibvirtVMDef.VideoDef.VideoModel.VIRTIO, 65536).toString());
+        assertEquals("<video>\n<model type='vga'/>\n</video>\n",
+                new LibvirtVMDef.VideoDef(LibvirtVMDef.VideoDef.VideoModel.VGA, 0).toString());
+    }
+
     @Test
     public void testCpuModeDefCpuFeatures() {
         LibvirtVMDef.CpuModeDef cpuModeDef = new LibvirtVMDef.CpuModeDef();