Merge pull request kubernetes#126599 from yaroslavborbat/fix-mount-bind

fix: preserve options after remount for bind mounting

Author: k8s-ci-robot

staging/src/k8s.io/mount-utils/go.mod

@@ -8,7 +8,6 @@ godebug default=go1.23
 
 require (
 	github.com/moby/sys/mountinfo v0.7.1
-	github.com/opencontainers/runc v1.1.14
 	github.com/stretchr/testify v1.9.0
 	golang.org/x/exp v0.0.0-20240719175910-8a7402abbf56
 	golang.org/x/sys v0.23.0
@@ -17,15 +16,11 @@ require (
 )
 
 require (
-	github.com/coreos/go-systemd/v22 v22.5.0 // indirect
 	github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect
 	github.com/go-logr/logr v1.4.2 // indirect
-	github.com/godbus/dbus/v5 v5.1.0 // indirect
 	github.com/kr/pretty v0.3.1 // indirect
-	github.com/opencontainers/runtime-spec v1.2.0 // indirect
 	github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect
 	github.com/rogpeppe/go-internal v1.12.0 // indirect
-	github.com/sirupsen/logrus v1.9.3 // indirect
 	gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c // indirect
 	gopkg.in/yaml.v3 v3.0.1 // indirect
 )

staging/src/k8s.io/mount-utils/go.sum

@@ -35,7 +35,6 @@ import (
 	"github.com/moby/sys/mountinfo"
 	"golang.org/x/sys/unix"
 
-	libcontaineruserns "github.com/opencontainers/runc/libcontainer/userns"
 	"k8s.io/klog/v2"
 	utilexec "k8s.io/utils/exec"
 )
@@ -114,7 +113,7 @@ func (mounter *Mounter) hasSystemd() bool {
 
 // Map unix.Statfs mount flags ro, nodev, noexec, nosuid, noatime, relatime,
 // nodiratime to mount option flag strings.
-func getUserNSBindMountOptions(path string, statfs func(path string, buf *unix.Statfs_t) (err error)) ([]string, error) {
+func getBindMountOptions(path string, statfs func(path string, buf *unix.Statfs_t) (err error)) ([]string, error) {
 	var s unix.Statfs_t
 	var mountOpts []string
 	if err := statfs(path, &s); err != nil {
@@ -137,32 +136,23 @@ func getUserNSBindMountOptions(path string, statfs func(path string, buf *unix.S
 	return mountOpts, nil
 }
 
-// Do a bind mount including the needed remount for applying the bind opts.
-// If the remount fails and we are running in a user namespace
-// figure out if the source filesystem has the ro, nodev, noexec, nosuid,
-// noatime, relatime or nodiratime flag set and try another remount with the found flags.
+// Performs a bind mount with the specified options, and then remounts
+// the mount point with the same `nodev`, `nosuid`, `noexec`, `nosuid`, `noatime`,
+// `relatime`, `nodiratime` options as the original mount point.
 func (mounter *Mounter) bindMountSensitive(mounterPath string, mountCmd string, source string, target string, fstype string, bindOpts []string, bindRemountOpts []string, bindRemountOptsSensitive []string, mountFlags []string, systemdMountRequired bool) error {
-	err := mounter.doMount(mounterPath, defaultMountCommand, source, target, fstype, bindOpts, bindRemountOptsSensitive, mountFlags, systemdMountRequired)
+	err := mounter.doMount(mounterPath, mountCmd, source, target, fstype, bindOpts, bindRemountOptsSensitive, mountFlags, systemdMountRequired)
 	if err != nil {
 		return err
 	}
-	err = mounter.doMount(mounterPath, defaultMountCommand, source, target, fstype, bindRemountOpts, bindRemountOptsSensitive, mountFlags, systemdMountRequired)
-	if libcontaineruserns.RunningInUserNS() {
-		if err == nil {
-			return nil
-		}
-		// Check if the source has ro, nodev, noexec, nosuid, noatime, relatime,
-		// nodiratime flag...
-		fixMountOpts, err := getUserNSBindMountOptions(source, unix.Statfs)
-		if err != nil {
-			return &os.PathError{Op: "statfs", Path: source, Err: err}
-		}
-		// ... and retry the mount with flags found above.
-		bindRemountOpts = append(bindRemountOpts, fixMountOpts...)
-		return mounter.doMount(mounterPath, defaultMountCommand, source, target, fstype, bindRemountOpts, bindRemountOptsSensitive, mountFlags, systemdMountRequired)
-	} else {
-		return err
+	// Check if the source has ro, nodev, noexec, nosuid, noatime, relatime,
+	// nodiratime flag...
+	fixMountOpts, err := getBindMountOptions(source, unix.Statfs)
+	if err != nil {
+		return &os.PathError{Op: "statfs", Path: source, Err: err}
 	}
+	// ... and retry the mount with flags found above.
+	bindRemountOpts = append(bindRemountOpts, fixMountOpts...)
+	return mounter.doMount(mounterPath, mountCmd, source, target, fstype, bindRemountOpts, bindRemountOptsSensitive, mountFlags, systemdMountRequired)
 }
 
 // Mount mounts source to target as fstype with given options. 'source' and 'fstype' must

staging/src/k8s.io/mount-utils/mount_linux.go

@@ -821,7 +821,7 @@ func mkStatfsFlags[T1 constraints.Integer, T2 constraints.Integer](orig T1, add
 	return orig | T1(add)
 }
 
-func TestGetUserNSBindMountOptions(t *testing.T) {
+func TestGetBindMountOptions(t *testing.T) {
 	var testCases = map[string]struct {
 		flags        int32 // smallest size used by any platform we care about
 		mountoptions string
@@ -843,9 +843,9 @@ func TestGetUserNSBindMountOptions(t *testing.T) {
 		return nil
 	}
 
-	testGetUserNSBindMountOptionsSingleCase := func(t *testing.T) {
+	testGetBindMountOptionsSingleCase := func(t *testing.T) {
 		path := strings.Split(t.Name(), "/")[1]
-		options, _ := getUserNSBindMountOptions(path, statfsMock)
+		options, _ := getBindMountOptions(path, statfsMock)
 		sort.Strings(options)
 		optionString := strings.Join(options, ",")
 		mountOptions := testCases[path].mountoptions
@@ -855,7 +855,7 @@ func TestGetUserNSBindMountOptions(t *testing.T) {
 	}
 
 	for k := range testCases {
-		t.Run(k, testGetUserNSBindMountOptionsSingleCase)
+		t.Run(k, testGetBindMountOptionsSingleCase)
 	}
 }
 

staging/src/k8s.io/mount-utils/mount_linux_test.go

@@ -43,6 +43,7 @@ import (
 	"time"
 
 	"github.com/onsi/ginkgo/v2"
+
 	v1 "k8s.io/api/core/v1"
 	rbacv1 "k8s.io/api/rbac/v1"
 	storagev1 "k8s.io/api/storage/v1"
@@ -1223,7 +1224,12 @@ func (l *localDriver) PrepareTest(ctx context.Context, f *framework.Framework) *
 	framework.ExpectNoError(err)
 
 	l.hostExec = utils.NewHostExec(f)
-	l.ltrMgr = utils.NewLocalResourceManager("local-driver", l.hostExec, "/tmp")
+	// It is recommended to mount /tmp with options noexec, nodev, nosuid.
+	// tmpfs on /tmp type tmpfs (rw,nosuid,nodev,noexec,relatime,seclabel,inode64)
+	// This prevents scripts and binaries from being executed from the /tmp directory.
+	// This can cause errors like "Permission denied" when executing files from `/tmp`.
+	// To pass the test that verifies the execution of files on a volume, we use `/var/tmp` instead of `/tmp`.
+	l.ltrMgr = utils.NewLocalResourceManager("local-driver", l.hostExec, "/var/tmp")
 
 	// This can't be done in SkipUnsupportedTest because the test framework is not initialized yet
 	if l.volumeType == utils.LocalVolumeGCELocalSSD {

test/e2e/storage/drivers/in_tree.go

@@ -27,6 +27,7 @@ import (
 	"strings"
 
 	"github.com/onsi/ginkgo/v2"
+
 	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/util/uuid"
 	"k8s.io/kubernetes/test/e2e/framework"
@@ -215,7 +216,7 @@ func (l *ltrMgr) cleanupLocalVolumeDirectory(ctx context.Context, ltr *LocalTest
 func (l *ltrMgr) setupLocalVolumeDirectoryLink(ctx context.Context, node *v1.Node, parameters map[string]string) *LocalTestResource {
 	hostDir := l.getTestDir()
 	hostDirBackend := hostDir + "-backend"
-	cmd := fmt.Sprintf("mkdir %s && ln -s %s %s", hostDirBackend, hostDirBackend, hostDir)
+	cmd := fmt.Sprintf("mkdir -p %s && ln -s %s %s", hostDirBackend, hostDirBackend, hostDir)
 	err := l.hostExec.IssueCommand(ctx, cmd, node)
 	framework.ExpectNoError(err)
 	return &LocalTestResource{
@@ -235,7 +236,7 @@ func (l *ltrMgr) cleanupLocalVolumeDirectoryLink(ctx context.Context, ltr *Local
 
 func (l *ltrMgr) setupLocalVolumeDirectoryBindMounted(ctx context.Context, node *v1.Node, parameters map[string]string) *LocalTestResource {
 	hostDir := l.getTestDir()
-	cmd := fmt.Sprintf("mkdir %s && mount --bind %s %s", hostDir, hostDir, hostDir)
+	cmd := fmt.Sprintf("mkdir -p %s && mount --bind %s %s", hostDir, hostDir, hostDir)
 	err := l.hostExec.IssueCommand(ctx, cmd, node)
 	framework.ExpectNoError(err)
 	return &LocalTestResource{
@@ -255,7 +256,7 @@ func (l *ltrMgr) cleanupLocalVolumeDirectoryBindMounted(ctx context.Context, ltr
 func (l *ltrMgr) setupLocalVolumeDirectoryLinkBindMounted(ctx context.Context, node *v1.Node, parameters map[string]string) *LocalTestResource {
 	hostDir := l.getTestDir()
 	hostDirBackend := hostDir + "-backend"
-	cmd := fmt.Sprintf("mkdir %s && mount --bind %s %s && ln -s %s %s", hostDirBackend, hostDirBackend, hostDirBackend, hostDirBackend, hostDir)
+	cmd := fmt.Sprintf("mkdir -p %s && mount --bind %s %s && ln -s %s %s", hostDirBackend, hostDirBackend, hostDirBackend, hostDirBackend, hostDir)
 	err := l.hostExec.IssueCommand(ctx, cmd, node)
 	framework.ExpectNoError(err)
 	return &LocalTestResource{