Skip to content

Commit

Permalink
Fixes: 15858 (podman system reset --force destroy machine)
Browse files Browse the repository at this point in the history
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 <mike@bitbistro.org>
  • Loading branch information
BitBistro committed Oct 23, 2022
1 parent a77ac5b commit 0572e59
Show file tree
Hide file tree
Showing 5 changed files with 17 additions and 7 deletions.
10 changes: 10 additions & 0 deletions pkg/machine/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package machine

import (
"errors"
"fmt"
"net"
"net/url"
"os"
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion pkg/machine/e2e/init_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion pkg/machine/e2e/machine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions pkg/machine/qemu/machine.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand Down
6 changes: 3 additions & 3 deletions pkg/machine/wsl/machine.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down

0 comments on commit 0572e59

Please sign in to comment.