From 3e8cf6959cbb829a3e54146c77b0e8c74c8c72af Mon Sep 17 00:00:00 2001
From: "James O. D. Hunt" <james.o.hunt@intel.com>
Date: Thu, 12 Oct 2023 12:13:53 +0100
Subject: [PATCH] runtime: Validate hypervisor section name in config file

Previously, if you accidentally modified the name of the hypervisor
section in the config file, the default golang runtime gives a cryptic
error message ("`VM memory cannot be zero`"). This can be demonstrated
using the `kata-runtime` utility program which uses the same golang
config package as the actual runtime (`containerd-shim-kata-v2`):

```bash
$ kata-runtime env >/dev/null; echo $?
0
$ sudo sed -i 's!^\[hypervisor\.qemu\]!\[hypervisor\.foo\]!g' /etc/kata-containers/configuration.toml
$ kata-runtime env >/dev/null; echo $?
VM memory cannot be zero
1
```

The hypervisor name is now validated so that the behaviour becomes:

```bash
$ kata-runtime env >/dev/null; echo $?
0
$ sudo sed -i 's!^\[hypervisor\.qemu\]!\[hypervisor\.foo\]!g' /etc/kata-containers/configuration.toml
$ ./kata-runtime env >/dev/null; echo $?
/etc/kata-containers/configuration.toml: configuration file contains invalid hypervisor section: "foo"
1
```

Fixes: #8212.

Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>
---
 src/runtime/pkg/katautils/config.go      |  4 ++
 src/runtime/pkg/katautils/config_test.go | 56 ++++++++++++++++++++++++
 2 files changed, 60 insertions(+)

diff --git a/src/runtime/pkg/katautils/config.go b/src/runtime/pkg/katautils/config.go
index 9dc0ff57c0..ed9ddbf184 100644
--- a/src/runtime/pkg/katautils/config.go
+++ b/src/runtime/pkg/katautils/config.go
@@ -55,6 +55,8 @@ const (
 
 	// the maximum amount of PCI bridges that can be cold plugged in a VM
 	maxPCIBridges uint32 = 5
+
+	errInvalidHypervisorPrefix = "configuration file contains invalid hypervisor section"
 )
 
 type tomlConfig struct {
@@ -1175,6 +1177,8 @@ func updateRuntimeConfigHypervisor(configPath string, tomlConf tomlConfig, confi
 		case dragonballHypervisorTableType:
 			config.HypervisorType = vc.DragonballHypervisor
 			hConfig, err = newDragonballHypervisorConfig(hypervisor)
+		default:
+			err = fmt.Errorf("%s: %+q", errInvalidHypervisorPrefix, k)
 		}
 
 		if err != nil {
diff --git a/src/runtime/pkg/katautils/config_test.go b/src/runtime/pkg/katautils/config_test.go
index 8b89234370..56fee45b23 100644
--- a/src/runtime/pkg/katautils/config_test.go
+++ b/src/runtime/pkg/katautils/config_test.go
@@ -1735,3 +1735,59 @@ vfio_mode="vfio"
 	assert.Equal(t, config.Runtime.InterNetworkModel, "macvtap")
 	assert.Equal(t, config.Runtime.VfioMode, "vfio")
 }
+
+func TestUpdateRuntimeConfigHypervisor(t *testing.T) {
+	assert := assert.New(t)
+
+	type tableTypeEntry struct {
+		name  string
+		valid bool
+	}
+
+	configFile := "/some/where/configuration.toml"
+
+	// Note: We cannot test acrnHypervisorTableType since
+	// newAcrnHypervisorConfig() expects ACRN binaries to be
+	// installed.
+	var entries = []tableTypeEntry{
+		{clhHypervisorTableType, true},
+		{dragonballHypervisorTableType, true},
+		{firecrackerHypervisorTableType, true},
+		{qemuHypervisorTableType, true},
+		{"foo", false},
+		{"bar", false},
+		{clhHypervisorTableType + "baz", false},
+	}
+
+	for i, h := range entries {
+		config := oci.RuntimeConfig{}
+
+		tomlConf := tomlConfig{
+			Hypervisor: map[string]hypervisor{
+				h.name: {
+					NumVCPUs:       int32(2),
+					MemorySize:     uint32(2048),
+					Path:           "/",
+					Kernel:         "/",
+					Image:          "/",
+					Firmware:       "/",
+					FirmwareVolume: "/",
+					SharedFS:       "virtio-fs",
+					VirtioFSDaemon: "/usr/libexec/kata-qemu/virtiofsd",
+				},
+			},
+		}
+
+		err := updateRuntimeConfigHypervisor(configFile, tomlConf, &config)
+
+		if h.valid {
+			assert.NoError(err, "test %d (%+v)", i, h)
+		} else {
+			assert.Error(err, "test %d (%+v)", i, h)
+
+			expectedErr := fmt.Errorf("%v: %v: %+q", configFile, errInvalidHypervisorPrefix, h.name)
+
+			assert.Equal(err, expectedErr, "test %d (%+v)", i, h)
+		}
+	}
+}