From bc1bdb89d98aed2b82d21b95e02eade506513d78 Mon Sep 17 00:00:00 2001 From: Akihiro Suda Date: Sat, 20 May 2023 05:22:44 +0900 Subject: [PATCH] Prohibit using a differential disk as a base disk Co-authored-by: Jan Dubois Signed-off-by: Akihiro Suda --- pkg/qemu/imgutil/imgutil.go | 51 +++++++++++++++++++++++++++---------- pkg/qemu/qemu.go | 19 ++++++++++---- pkg/vz/disk.go | 16 ++++++++---- pkg/vz/vm_darwin.go | 18 ++++++------- 4 files changed, 71 insertions(+), 33 deletions(-) diff --git a/pkg/qemu/imgutil/imgutil.go b/pkg/qemu/imgutil/imgutil.go index a3bfb92350f..9261ed175bc 100644 --- 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 index e653c0e1f41..f8fe70acdb7 100644 --- 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 index 557361273bb..334827a98a5 100644 --- 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 index f2ae41cf340..d33718250d2 100644 --- 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)