Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DNM carry/fix #3967] Remove mount fallback flag #3975

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions contrib/completions/bash/runc
Original file line number Diff line number Diff line change
Expand Up @@ -461,7 +461,6 @@ _runc_run() {
--no-subreaper
--no-pivot
--no-new-keyring
--no-mount-fallback
"

local options_with_args="
Expand Down Expand Up @@ -568,7 +567,6 @@ _runc_create() {
--help
--no-pivot
--no-new-keyring
--no-mount-fallback
"

local options_with_args="
Expand Down Expand Up @@ -629,7 +627,6 @@ _runc_restore() {
--no-pivot
--auto-dedup
--lazy-pages
--no-mount-fallback
"

local options_with_args="
Expand Down
4 changes: 0 additions & 4 deletions create.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,6 @@ command(s) that get executed on start, edit the args parameter of the spec. See
Name: "preserve-fds",
Usage: "Pass N additional file descriptors to the container (stdio + $LISTEN_FDS + N in total)",
},
cli.BoolFlag{
Name: "no-mount-fallback",
Usage: "Do not fallback when the specific configuration is not applicable (e.g., do not try to remount a bind mount again after the first attempt failed on source filesystems that have nodev, noexec, nosuid, noatime, relatime, strictatime, nodiratime set)",
},
},
Action: func(context *cli.Context) error {
if err := checkArgs(context, 1, exactArgs); err != nil {
Expand Down
4 changes: 0 additions & 4 deletions libcontainer/configs/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -212,10 +212,6 @@ type Config struct {
// RootlessCgroups is set when unlikely to have the full access to cgroups.
// When RootlessCgroups is set, cgroups errors are ignored.
RootlessCgroups bool `json:"rootless_cgroups,omitempty"`

// Do not try to remount a bind mount again after the first attempt failed on source
// filesystems that have nodev, noexec, nosuid, noatime, relatime, strictatime, nodiratime set
NoMountFallback bool `json:"no_mount_fallback,omitempty"`
}

type (
Expand Down
4 changes: 4 additions & 0 deletions libcontainer/configs/mount_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ type Mount struct {
// Mount flags.
Flags int `json:"flags"`

// Mount flags that were explicitly cleared in the configuration (meaning
// the user explicitly requested that these flags *not* be set).
ClearedFlags int `json:"cleared_flags"`

// Propagation Flags
PropagationFlags []int `json:"propagation_flags"`

Expand Down
31 changes: 16 additions & 15 deletions libcontainer/rootfs_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ type mountConfig struct {
cgroup2Path string
rootlessCgroups bool
cgroupns bool
noMountFallback bool
}

// mountEntry contains mount data specific to a mount point.
Expand Down Expand Up @@ -84,7 +83,6 @@ func prepareRootfs(pipe io.ReadWriter, iConfig *initConfig, mountFds mountFds) (
cgroup2Path: iConfig.Cgroup2Path,
rootlessCgroups: iConfig.RootlessCgroups,
cgroupns: config.Namespaces.Contains(configs.NEWCGROUP),
noMountFallback: config.NoMountFallback,
}
for i, m := range config.Mounts {
entry := mountEntry{Mount: m}
Expand Down Expand Up @@ -514,7 +512,7 @@ func mountToRootfs(c *mountConfig, m mountEntry) error {
// first check that we have non-default options required before attempting a remount
if m.Flags&^(unix.MS_REC|unix.MS_REMOUNT|unix.MS_BIND) != 0 {
// only remount if unique mount options are set
if err := remount(m, rootfs, c.noMountFallback); err != nil {
if err := remount(m, rootfs); err != nil {
return err
}
}
Expand Down Expand Up @@ -1103,33 +1101,36 @@ func writeSystemProperty(key, value string) error {
return os.WriteFile(path.Join("/proc/sys", keyPath), []byte(value), 0o644)
}

func remount(m mountEntry, rootfs string, noMountFallback bool) error {
func remount(m mountEntry, rootfs string) error {
const mntLockedFlags = unix.MS_RDONLY | unix.MS_NODEV | unix.MS_NOEXEC |
unix.MS_NOSUID | unix.MS_NOATIME | unix.MS_RELATIME |
unix.MS_STRICTATIME | unix.MS_NODIRATIME

return utils.WithProcfd(rootfs, m.Destination, func(dstFD string) error {
flags := uintptr(m.Flags | unix.MS_REMOUNT)
err := mountViaFDs(m.Source, m.srcFD, m.Destination, dstFD, m.Device, flags, "")
if err == nil {
return nil
}
// Check if the source has flags set according to noMountFallback
src := m.src()
var s unix.Statfs_t
if err := unix.Statfs(src, &s); err != nil {
return &os.PathError{Op: "statfs", Path: src, Err: err}
}
var checkflags int
if noMountFallback {
// Check for ro only
checkflags = unix.MS_RDONLY
} else {
// Check for ro, nodev, noexec, nosuid, noatime, relatime, strictatime,
// nodiratime
checkflags = unix.MS_RDONLY | unix.MS_NODEV | unix.MS_NOEXEC | unix.MS_NOSUID | unix.MS_NOATIME | unix.MS_RELATIME | unix.MS_STRICTATIME | unix.MS_NODIRATIME
// Check if the source has any MNT_LOCKED flags set. If so, we need to
// pass the same set of flags when running in a user namespace in order
// for the remount to succeed.
if int(s.Flags)&mntLockedFlags == 0 {
return err
}
if int(s.Flags)&checkflags == 0 {
// However, if the user explicitly request the flags *not* be set, we
// need to return an error to avoid producing mounts that don't match
// their requirements.
if int(s.Flags)&m.ClearedFlags&mntLockedFlags != 0 {
return err
}
// ... and retry the mount with flags found above.
flags |= uintptr(int(s.Flags) & checkflags)
flags |= uintptr(int(s.Flags) & mntLockedFlags)
return mountViaFDs(m.Source, m.srcFD, m.Destination, dstFD, m.Device, flags, "")
})
}
Expand Down
4 changes: 2 additions & 2 deletions libcontainer/specconv/spec_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,6 @@ type CreateOpts struct {
Spec *specs.Spec
RootlessEUID bool
RootlessCgroups bool
NoMountFallback bool
}

// getwd is a wrapper similar to os.Getwd, except it always gets
Expand Down Expand Up @@ -359,7 +358,6 @@ func CreateLibcontainerConfig(opts *CreateOpts) (*configs.Config, error) {
NoNewKeyring: opts.NoNewKeyring,
RootlessEUID: opts.RootlessEUID,
RootlessCgroups: opts.RootlessCgroups,
NoMountFallback: opts.NoMountFallback,
}

for _, m := range spec.Mounts {
Expand Down Expand Up @@ -981,8 +979,10 @@ func parseMountOptions(options []string) *configs.Mount {
if f, exists := mountFlags[o]; exists && f.flag != 0 {
if f.clear {
m.Flags &= ^f.flag
m.ClearedFlags |= f.flag
} else {
m.Flags |= f.flag
m.ClearedFlags &= ^f.flag
}
} else if f, exists := mountPropagationMapping[o]; exists && f != 0 {
m.PropagationFlags = append(m.PropagationFlags, f)
Expand Down
4 changes: 0 additions & 4 deletions restore.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,10 +98,6 @@ using the runc checkpoint command.`,
Value: "",
Usage: "Specify an LSM mount context to be used during restore.",
},
cli.BoolFlag{
Name: "no-mount-fallback",
Usage: "Do not fallback when the specific configuration is not applicable (e.g., do not try to remount a bind mount again after the first attempt failed on source filesystems that have nodev, noexec, nosuid, noatime, relatime, strictatime, nodiratime set)",
},
},
Action: func(context *cli.Context) error {
if err := checkArgs(context, 1, exactArgs); err != nil {
Expand Down
4 changes: 0 additions & 4 deletions run.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,6 @@ command(s) that get executed on start, edit the args parameter of the spec. See
Name: "preserve-fds",
Usage: "Pass N additional file descriptors to the container (stdio + $LISTEN_FDS + N in total)",
},
cli.BoolFlag{
Name: "no-mount-fallback",
Usage: "Do not fallback when the specific configuration is not applicable (e.g., do not try to remount a bind mount again after the first attempt failed on source filesystems that have nodev, noexec, nosuid, noatime, relatime, strictatime, nodiratime set)",
},
},
Action: func(context *cli.Context) error {
if err := checkArgs(context, 1, exactArgs); err != nil {
Expand Down
59 changes: 32 additions & 27 deletions tests/integration/mounts_sshfs.bats
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,12 @@ function setup() {
}

function teardown() {
# Some distros do not have fusermount installed
# as a dependency of fuse-sshfs, and good ol' umount works.
fusermount -u "$DIR" || umount "$DIR"
if [ -v DIR ]; then
# Some distros do not have fusermount installed
# as a dependency of fuse-sshfs, and good ol' umount works.
fusermount -u "$DIR" || umount "$DIR"
unset DIR
fi

teardown_bundle
}
Expand All @@ -30,33 +33,35 @@ function setup_sshfs() {
fi
}

@test "runc run [rw bind mount of a ro fuse sshfs mount]" {
@test "runc run [implied-rw bind mount of a ro fuse sshfs mount]" {
setup_sshfs "ro"
# All of the extra mount flags are needed to force a remount with new flags.
update_config ' .mounts += [{
type: "bind",
source: "'"$DIR"'",
destination: "/mnt",
options: ["rw", "rprivate", "nosuid", "nodev", "rbind"]
options: ["rprivate", "nosuid", "nodev", "rbind", "sync"]
}]'

runc run --no-mount-fallback test_busybox
runc run test_busybox
[ "$status" -eq 0 ]
}

@test "runc run [dev,exec,suid,atime bind mount of a nodev,nosuid,noexec,noatime fuse sshfs mount]" {
setup_sshfs "nodev,nosuid,noexec,noatime"
# The "sync" option is used to trigger a remount with the below options.
# It serves no further purpose. Otherwise only a bind mount without
# applying the below options will be done.
@test "runc run [explicit-rw bind mount of a ro fuse sshfs mount]" {
setup_sshfs "ro"
update_config ' .mounts += [{
type: "bind",
source: "'"$DIR"'",
destination: "/mnt",
options: ["dev", "suid", "exec", "atime", "rprivate", "rbind", "sync"]
options: ["rw", "rprivate", "nosuid", "nodev", "rbind"]
}]'

runc run test_busybox
[ "$status" -eq 0 ]
# The above will fail because we explicitly requested a mount with a
# MNT_LOCKED mount option cleared (when the source mount has those mounts
# enabled), namely MS_RDONLY.
[ "$status" -ne 0 ]
[[ "$output" == *"runc run failed: unable to start container process: error during container init: error mounting"*"operation not permitted"* ]]
}

@test "runc run [ro bind mount of a nodev,nosuid,noexec,noatime fuse sshfs mount]" {
Expand All @@ -72,7 +77,9 @@ function setup_sshfs() {
[ "$status" -eq 0 ]
}

@test "runc run [dev,exec,suid,atime bind mount of a nodev,nosuid,noexec,noatime fuse sshfs mount without fallback]" {
@test "runc run [dev,exec,suid,atime bind mount of a nodev,nosuid,noexec,noatime fuse sshfs mount]" {
requires rootless

setup_sshfs "nodev,nosuid,noexec,noatime"
# The "sync" option is used to trigger a remount with the below options.
# It serves no further purpose. Otherwise only a bind mount without
Expand All @@ -84,27 +91,25 @@ function setup_sshfs() {
options: ["dev", "suid", "exec", "atime", "rprivate", "rbind", "sync"]
}]'

runc run --no-mount-fallback test_busybox
# The above will fail as we added --no-mount-fallback which causes us not to
# try to remount a bind mount again after the first attempt failed on source
# filesystems that have nodev, noexec, nosuid, noatime set.
runc run test_busybox
# The above will fail because we explicitly requested a mount with several
# MNT_LOCKED mount options cleared (when the source mount has those mounts
# enabled).
[ "$status" -ne 0 ]
[[ "$output" == *"runc run failed: unable to start container process: error during container init: error mounting"*"operation not permitted"* ]]
}

@test "runc run [ro bind mount of a nodev,nosuid,noexec,noatime fuse sshfs mount without fallback]" {
setup_sshfs "nodev,nosuid,noexec,noatime"
@test "runc run [ro,noexec bind mount of a nosuid,noatime fuse sshfs mount]" {
requires rootless

setup_sshfs "nosuid,noatime"
update_config ' .mounts += [{
type: "bind",
source: "'"$DIR"'",
destination: "/mnt",
options: ["rbind", "ro"]
options: ["rbind", "ro", "exec"]
}]'

runc run --no-mount-fallback test_busybox
# The above will fail as we added --no-mount-fallback which causes us not to
# try to remount a bind mount again after the first attempt failed on source
# filesystems that have nodev, noexec, nosuid, noatime set.
[ "$status" -ne 0 ]
[[ "$output" == *"runc run failed: unable to start container process: error during container init: error mounting"*"operation not permitted"* ]]
runc run test_busybox
[ "$status" -eq 0 ]
}
1 change: 0 additions & 1 deletion utils_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,6 @@ func createContainer(context *cli.Context, id string, spec *specs.Spec) (*libcon
Spec: spec,
RootlessEUID: os.Geteuid() != 0,
RootlessCgroups: rootlessCg,
NoMountFallback: context.Bool("no-mount-fallback"),
})
if err != nil {
return nil, err
Expand Down