core/mount: Prevent accidental removal of rootfs files

Using os.RemoveAll() is quite risky, as if the unmount failed and we
can delete files from the container rootfs. In fact, we were doing just
that.

Let's use os.Remove() to make sure we only deleted empty dirs.

Big kudos to @mbaynton for reporting this issue with lot of details,
nailing it down to containerd lines of code and showing all the log
lines to understand the big picture.

Fixes: #10704

Signed-off-by: Rodrigo Campos <rodrigoca@microsoft.com>
This commit is contained in:
Rodrigo Campos 2024-09-23 16:15:10 +02:00
parent 004f3951d5
commit f8d84ecf92

View File

@ -256,9 +256,16 @@ func doPrepareIDMappedOverlay(lowerDirs []string, usernsFd int) (tmpLowerDirs []
if err := unix.Unmount(lowerDir, unix.MNT_DETACH); err != nil { if err := unix.Unmount(lowerDir, unix.MNT_DETACH); err != nil {
log.L.WithError(err).Warnf("failed to unmount temp lowerdir %s", lowerDir) log.L.WithError(err).Warnf("failed to unmount temp lowerdir %s", lowerDir)
} }
// Using os.Remove() so if it's not empty, we don't delete files in the
// rootfs.
if err := os.Remove(lowerDir); err != nil {
log.L.WithError(err).Warnf("failed to remove temporary overlay lowerdir's")
}
} }
if terr := os.RemoveAll(filepath.Clean(filepath.Join(tmpLowerDirs[0], ".."))); terr != nil {
log.L.WithError(terr).Warnf("failed to remove temporary overlay lowerdir's") // This dir should be empty now. Otherwise, we don't do anything.
if err := os.Remove(filepath.Join(tmpLowerDirs[0], "..")); err != nil {
log.L.WithError(err).Infof("failed to remove temporary overlay dir")
} }
} }
for i, lowerDir := range lowerDirs { for i, lowerDir := range lowerDirs {