From 0572e59725d7d22e8b2915b23073615d55f1a0b1 Mon Sep 17 00:00:00 2001 From: Mike Perry Date: Wed, 21 Sep 2022 21:41:44 -0400 Subject: [PATCH] Fixes: 15858 (podman system reset --force destroy machine) Safe guards calls to os.RemoveAll in order to prevent calls from accidently deleting the root file system in very strange edge cases. Did this by creating GuardedRemoveAll and migrated machine os.RemoveAll calls to it. Signed-off-by: Mike Perry --- pkg/machine/config.go | 10 ++++++++++ pkg/machine/e2e/init_test.go | 2 +- pkg/machine/e2e/machine_test.go | 2 +- pkg/machine/qemu/machine.go | 4 ++-- pkg/machine/wsl/machine.go | 6 +++--- 5 files changed, 17 insertions(+), 7 deletions(-) diff --git a/pkg/machine/config.go b/pkg/machine/config.go index 8c22ae6a385e..ee22be3dea3e 100644 --- a/pkg/machine/config.go +++ b/pkg/machine/config.go @@ -5,6 +5,7 @@ package machine import ( "errors" + "fmt" "net" "net/url" "os" @@ -237,6 +238,15 @@ func ConfDirPrefix() (string, error) { return confDir, nil } +// GuardedRemoveAll functions much like os.RemoveAll but +// will not delete certain catastrophic paths. +func GuardedRemoveAll(path string) error { + if path == "" || path == "/" { + return fmt.Errorf("refusing to recusively delete `%s`", path) + } + return os.RemoveAll(path) +} + // ResourceConfig describes physical attributes of the machine type ResourceConfig struct { // CPUs to be assigned to the VM diff --git a/pkg/machine/e2e/init_test.go b/pkg/machine/e2e/init_test.go index ebf59dcd7c5b..8fc6fa5a68a3 100644 --- a/pkg/machine/e2e/init_test.go +++ b/pkg/machine/e2e/init_test.go @@ -142,7 +142,7 @@ var _ = Describe("podman machine init", func() { _, err = os.CreateTemp(tmpDir, "example") Expect(err).To(BeNil()) mount := tmpDir + ":/testmountdir" - defer os.RemoveAll(tmpDir) + defer func() { _ = machine.GuardedRemoveAll(tmpDir) }() name := randomString() i := new(initMachine) diff --git a/pkg/machine/e2e/machine_test.go b/pkg/machine/e2e/machine_test.go index 5cd89c7abc08..6c460621d6f9 100644 --- a/pkg/machine/e2e/machine_test.go +++ b/pkg/machine/e2e/machine_test.go @@ -128,7 +128,7 @@ func teardown(origHomeDir string, testDir string, mb *machineTestBuilder) { fmt.Printf("error occurred rm'ing machine: %q\n", err) } } - if err := os.RemoveAll(testDir); err != nil { + if err := machine.GuardedRemoveAll(testDir); err != nil { Fail(fmt.Sprintf("failed to remove test dir: %q", err)) } // this needs to be last in teardown diff --git a/pkg/machine/qemu/machine.go b/pkg/machine/qemu/machine.go index a6907c0dfba6..8fc2ad0e0f5b 100644 --- a/pkg/machine/qemu/machine.go +++ b/pkg/machine/qemu/machine.go @@ -1690,7 +1690,7 @@ func (p *Provider) RemoveAndCleanMachines() error { } prevErr = err } else { - err := os.RemoveAll(dataDir) + err := machine.GuardedRemoveAll(dataDir) if err != nil { if prevErr != nil { logrus.Error(prevErr) @@ -1707,7 +1707,7 @@ func (p *Provider) RemoveAndCleanMachines() error { } prevErr = err } else { - err := os.RemoveAll(confDir) + err := machine.GuardedRemoveAll(confDir) if err != nil { if prevErr != nil { logrus.Error(prevErr) diff --git a/pkg/machine/wsl/machine.go b/pkg/machine/wsl/machine.go index 81980736dc36..79e97293389a 100644 --- a/pkg/machine/wsl/machine.go +++ b/pkg/machine/wsl/machine.go @@ -1339,7 +1339,7 @@ func (v *MachineVM) Remove(name string, opts machine.RemoveOptions) (string, fun logrus.Error(err) } for _, f := range files { - if err := os.RemoveAll(f); err != nil { + if err := machine.GuardedRemoveAll(f); err != nil { logrus.Error(err) } } @@ -1644,7 +1644,7 @@ func (p *Provider) RemoveAndCleanMachines() error { } prevErr = err } else { - err := os.RemoveAll(dataDir) + err := machine.GuardedRemoveAll(dataDir) if err != nil { if prevErr != nil { logrus.Error(prevErr) @@ -1661,7 +1661,7 @@ func (p *Provider) RemoveAndCleanMachines() error { } prevErr = err } else { - err := os.RemoveAll(confDir) + err := machine.GuardedRemoveAll(confDir) if err != nil { if prevErr != nil { logrus.Error(prevErr)