Prohibit using a differential disk as a base disk

Co-authored-by: Jan Dubois <jan@jandubois.com> Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
lmaitresym · May 29, 2023 · bc1bdb8 · bc1bdb8
1 parent bfa5bab
commit bc1bdb8
Show file tree

Hide file tree

Showing 4 changed files with 71 additions and 33 deletions.
diff --git a/pkg/qemu/imgutil/imgutil.go b/pkg/qemu/imgutil/imgutil.go
@@ -5,8 +5,8 @@ import (
 	"encoding/json"
 	"fmt"
 	"os/exec"
-	"path/filepath"
-	"strings"
+
+	"github.com/sirupsen/logrus"
 )
 
 type InfoChild struct {
@@ -111,19 +111,42 @@ func GetInfo(f string) (*Info, error) {
 	return ParseInfo(stdout.Bytes())
 }
 
-func DetectFormat(f string) (string, error) {
-	switch ext := strings.ToLower(filepath.Ext(f)); ext {
-	case ".qcow2":
-		return "qcow2", nil
-	case ".raw":
-		return "raw", nil
+func AcceptableAsBasedisk(info *Info) error {
+	switch info.Format {
+	case "qcow2", "raw":
+		// NOP
+	default:
+		logrus.WithField("filename", info.Filename).
+			Warnf("Unsupported image format %q. The image may not boot, or may have an extra privilege to access the host filesystem. Use with caution.", info.Format)
+	}
+	if info.BackingFilename != "" {
+		return fmt.Errorf("base disk (%q) must not have a backing file (%q)", info.Filename, info.BackingFilename)
 	}
-	imgInfo, err := GetInfo(f)
-	if err != nil {
-		return "", err
+	if info.FullBackingFilename != "" {
+		return fmt.Errorf("base disk (%q) must not have a backing file (%q)", info.Filename, info.FullBackingFilename)
 	}
-	if imgInfo.Format == "" {
-		return "", fmt.Errorf("failed to detect format of %q", f)
+	if info.FormatSpecific != nil {
+		if vmdk := info.FormatSpecific.Vmdk(); vmdk != nil {
+			for _, e := range vmdk.Extents {
+				if e.Filename != info.Filename {
+					return fmt.Errorf("base disk (%q) must not have an extent file (%q)", info.Filename, e.Filename)
+				}
+			}
+		}
 	}
-	return imgInfo.Format, nil
+	// info.Children is set since QEMU 8.0
+	switch len(info.Children) {
+	case 0:
+	// NOP
+	case 1:
+		if info.Filename != info.Children[0].Info.Filename {
+			return fmt.Errorf("base disk (%q) child must not have a different filename (%q)", info.Filename, info.Children[0].Info.Filename)
+		}
+		if len(info.Children[0].Info.Children) > 0 {
+			return fmt.Errorf("base disk (%q) child must not have children of its own", info.Filename)
+		}
+	default:
+		return fmt.Errorf("base disk (%q) must not have multiple children: %+v", info.Filename, info.Children)
+	}
+	return nil
 }
diff --git a/pkg/qemu/qemu.go b/pkg/qemu/qemu.go
@@ -96,13 +96,19 @@ func EnsureDisk(cfg Config) error {
 	if err != nil {
 		return err
 	}
+	baseDiskInfo, err := imgutil.GetInfo(baseDisk)
+	if err != nil {
+		return fmt.Errorf("failed to get the information of base disk %q: %w", baseDisk, err)
+	}
+	if err = imgutil.AcceptableAsBasedisk(baseDiskInfo); err != nil {
+		return fmt.Errorf("file %q is not acceptable as the base disk: %w", baseDisk, err)
+	}
+	if baseDiskInfo.Format == "" {
+		return fmt.Errorf("failed to inspect the format of %q", baseDisk)
+	}
 	args := []string{"create", "-f", "qcow2"}
 	if !isBaseDiskISO {
-		baseDiskFormat, err := imgutil.DetectFormat(baseDisk)
-		if err != nil {
-			return err
-		}
-		args = append(args, "-F", baseDiskFormat, "-b", baseDisk)
+		args = append(args, "-F", baseDiskInfo.Format, "-b", baseDisk)
 	}
 	args = append(args, diffDisk, strconv.Itoa(int(diskSize)))
 	cmd := exec.Command("qemu-img", args...)
@@ -581,6 +587,9 @@ func Cmdline(cfg Config) (string, []string, error) {
 		if err != nil {
 			return "", nil, fmt.Errorf("failed to get the information of %q: %w", baseDisk, err)
 		}
+		if err = imgutil.AcceptableAsBasedisk(baseDiskInfo); err != nil {
+			return "", nil, fmt.Errorf("file %q is not acceptable as the base disk: %w", baseDisk, err)
+		}
 		if baseDiskInfo.Format == "" {
 			return "", nil, fmt.Errorf("failed to inspect the format of %q", baseDisk)
 		}

diff --git a/pkg/vz/disk.go b/pkg/vz/disk.go
@@ -49,13 +49,19 @@ func EnsureDisk(driver *driver.BaseDriver) error {
 	if err != nil {
 		return err
 	}
+	baseDiskInfo, err := imgutil.GetInfo(baseDisk)
+	if err != nil {
+		return fmt.Errorf("failed to get the information of base disk %q: %w", baseDisk, err)
+	}
+	if err = imgutil.AcceptableAsBasedisk(baseDiskInfo); err != nil {
+		return fmt.Errorf("file %q is not acceptable as the base disk: %w", baseDisk, err)
+	}
+	if baseDiskInfo.Format == "" {
+		return fmt.Errorf("failed to inspect the format of %q", baseDisk)
+	}
 	args := []string{"create", "-f", "qcow2"}
 	if !isBaseDiskISO {
-		baseDiskFormat, err := imgutil.DetectFormat(baseDisk)
-		if err != nil {
-			return err
-		}
-		args = append(args, "-F", baseDiskFormat, "-b", baseDisk)
+		args = append(args, "-F", baseDiskInfo.Format, "-b", baseDisk)
 	}
 	args = append(args, diffDisk, strconv.Itoa(int(diskSize)))
 	cmd := exec.Command("qemu-img", args...)

diff --git a/pkg/vz/vm_darwin.go b/pkg/vz/vm_darwin.go
@@ -373,12 +373,12 @@ func attachNetwork(driver *driver.BaseDriver, vmConfig *vz.VirtualMachineConfigu
 }
 
 func validateDiskFormat(diskPath string) error {
-	format, err := imgutil.DetectFormat(diskPath)
+	info, err := imgutil.GetInfo(diskPath)
 	if err != nil {
 		return fmt.Errorf("failed to detect the format of %q: %w", diskPath, err)
 	}
-	if format != "raw" {
-		return fmt.Errorf("expected the format of %q to be \"raw\", got %q", diskPath, format)
+	if info.Format != "raw" {
+		return fmt.Errorf("expected the format of %q to be \"raw\", got %q", diskPath, info.Format)
 	}
 	// TODO: ensure that the disk is formatted with GPT or ISO9660
 	return nil
@@ -437,23 +437,23 @@ func attachDisks(driver *driver.BaseDriver, vmConfig *vz.VirtualMachineConfigura
 		}
 		extraDiskPath := filepath.Join(d.Dir, filenames.DataDisk)
 
-		extraDiskFormat, err := imgutil.DetectFormat(extraDiskPath)
+		extraDiskInfo, err := imgutil.GetInfo(extraDiskPath)
 		if err != nil {
 			return fmt.Errorf("failed to run detect disk format %q: %q", diskName, err)
 		}
-		if extraDiskFormat != "raw" {
-			if strings.Contains(extraDiskFormat, string(os.PathSeparator)) {
+		if extraDiskInfo.Format != "raw" {
+			if strings.Contains(extraDiskInfo.Format, string(os.PathSeparator)) {
 				return fmt.Errorf(
 					"failed to convert disk %q to raw for vz driver because extraDiskFormat %q contains a path separator %q",
 					diskName,
-					extraDiskFormat,
+					extraDiskInfo.Format,
 					os.PathSeparator,
 				)
 			}
 			rawPath := fmt.Sprintf("%s.raw", extraDiskPath)
-			oldFormatPath := fmt.Sprintf("%s.%s", extraDiskPath, extraDiskFormat)
+			oldFormatPath := fmt.Sprintf("%s.%s", extraDiskPath, extraDiskInfo.Format)
 			if err = imgutil.ConvertToRaw(extraDiskPath, rawPath); err != nil {
-				return fmt.Errorf("failed to convert %s disk %q to raw for vz driver: %w", extraDiskFormat, diskName, err)
+				return fmt.Errorf("failed to convert %s disk %q to raw for vz driver: %w", extraDiskInfo.Format, diskName, err)
 			}
 			if err = os.Rename(extraDiskPath, oldFormatPath); err != nil {
 				return fmt.Errorf("failed to rename additional disk for vz driver: %w", err)