From c11c5e180a6e00e0093f51b050962ee1e2e30f7a Mon Sep 17 00:00:00 2001
From: Kir Kolyshkin <kolyshkin@gmail.com>
Date: Mon, 30 Mar 2020 19:59:47 -0700
Subject: pkg/spec/initFSMounts: fix
> $ ./bin/podman run -v /tmp:/tmp alpine true; echo $?
> 0
> $ ./bin/podman run -v /tmp:/tmp:ro alpine true; echo $?
> 0
> $ ./bin/podman run -v /tmp:/w0w:ro alpine true; echo $?
> Error: container_linux.go:349: starting container process caused "process_linux.go:449: container init caused \"rootfs_linux.go:58: mounting \\\"/tmp\\\" to rootfs \\\"/home/kir/.local/share/containers/storage/overlay/7636ef3650fc91ee4996ccc026532bb3cff7182c0430db662fffb933e0bcadc9/merged\\\" at \\\"/home/kir/.local/share/containers/storage/overlay/7636ef3650fc91ee4996ccc026532bb3cff7182c0430db662fffb933e0bcadc9/merged/w0w\\\" caused \\\"operation not permitted\\\"\"": OCI runtime permission denied error
> 126
The last command is not working because in-container mount point
is used to search for a parent mount in /proc/self/mountinfo.
And yet the following
> $ ./bin/podman run -v /tmp:/run/test:ro alpine true; echo $?
> 0
still works fine! Here's why:
> $ mount | grep -E '/run |/tmp '
> tmpfs on /run type tmpfs (rw,nosuid,nodev,seclabel,mode=755)
> tmpfs on /tmp type tmpfs (rw,nosuid,nodev,seclabel)
This is the reason why previous commit modified in-container mount
point.
Fixes: 0f5ae3c5af
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
---
pkg/spec/storage.go | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
(limited to 'pkg/spec')
diff --git a/pkg/spec/storage.go b/pkg/spec/storage.go
index b0687b4c2..404d94432 100644
--- a/pkg/spec/storage.go
+++ b/pkg/spec/storage.go
@@ -867,9 +867,9 @@ func InitFSMounts(inputMounts []spec.Mount) ([]spec.Mount, error) {
var mounts []spec.Mount
for _, m := range inputMounts {
if m.Type == TypeBind {
- baseMnt, err := findMount(m.Destination, systemMounts)
+ baseMnt, err := findMount(m.Source, systemMounts)
if err != nil {
- return nil, errors.Wrapf(err, "error looking up mountpoint for mount %s", m.Destination)
+ return nil, errors.Wrapf(err, "error looking up mountpoint for mount %s", m.Source)
}
var noexec, nosuid, nodev bool
for _, baseOpt := range strings.Split(baseMnt.Opts, ",") {
--
cgit v1.2.3-54-g00ecf
From f2c42a3958d12b45375aeb2384a3a8a103203c1c Mon Sep 17 00:00:00 2001
From: Kir Kolyshkin <kolyshkin@gmail.com>
Date: Tue, 31 Mar 2020 10:20:05 -0700
Subject: pkg/spec.InitFSMounts: fix mount opts in place
... rather than create a new slice and then make the caller
replace the original with the new one.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
---
pkg/spec/spec.go | 4 +---
pkg/spec/storage.go | 22 +++++++++-------------
pkg/specgen/oci.go | 4 +---
3 files changed, 11 insertions(+), 19 deletions(-)
(limited to 'pkg/spec')
diff --git a/pkg/spec/spec.go b/pkg/spec/spec.go
index d4fd5976f..194d2fcb3 100644
--- a/pkg/spec/spec.go
+++ b/pkg/spec/spec.go
@@ -371,11 +371,9 @@ func (config *CreateConfig) createConfigToOCISpec(runtime *libpod.Runtime, userM
// BIND MOUNTS
configSpec.Mounts = SupercedeUserMounts(userMounts, configSpec.Mounts)
// Process mounts to ensure correct options
- finalMounts, err := InitFSMounts(configSpec.Mounts)
- if err != nil {
+ if err := InitFSMounts(configSpec.Mounts); err != nil {
return nil, err
}
- configSpec.Mounts = finalMounts
// BLOCK IO
blkio, err := config.CreateBlockIO()
diff --git a/pkg/spec/storage.go b/pkg/spec/storage.go
index 404d94432..335907d12 100644
--- a/pkg/spec/storage.go
+++ b/pkg/spec/storage.go
@@ -855,21 +855,19 @@ func SupercedeUserMounts(mounts []spec.Mount, configMount []spec.Mount) []spec.M
}
// Ensure mount options on all mounts are correct
-func InitFSMounts(inputMounts []spec.Mount) ([]spec.Mount, error) {
+func InitFSMounts(mounts []spec.Mount) error {
// We need to look up mounts so we can figure out the proper mount flags
// to apply.
systemMounts, err := pmount.GetMounts()
if err != nil {
- return nil, errors.Wrapf(err, "error retrieving system mounts to look up mount options")
+ return errors.Wrapf(err, "error retrieving system mounts to look up mount options")
}
- // TODO: We probably don't need to re-build the mounts array
- var mounts []spec.Mount
- for _, m := range inputMounts {
+ for i, m := range mounts {
if m.Type == TypeBind {
baseMnt, err := findMount(m.Source, systemMounts)
if err != nil {
- return nil, errors.Wrapf(err, "error looking up mountpoint for mount %s", m.Source)
+ return errors.Wrapf(err, "error looking up mountpoint for mount %s", m.Source)
}
var noexec, nosuid, nodev bool
for _, baseOpt := range strings.Split(baseMnt.Opts, ",") {
@@ -890,21 +888,19 @@ func InitFSMounts(inputMounts []spec.Mount) ([]spec.Mount, error) {
opts, err := util.ProcessOptions(m.Options, false, defaultMountOpts)
if err != nil {
- return nil, err
+ return err
}
- m.Options = opts
+ mounts[i].Options = opts
}
if m.Type == TypeTmpfs && filepath.Clean(m.Destination) != "/dev" {
opts, err := util.ProcessOptions(m.Options, true, nil)
if err != nil {
- return nil, err
+ return err
}
- m.Options = opts
+ mounts[i].Options = opts
}
-
- mounts = append(mounts, m)
}
- return mounts, nil
+ return nil
}
// TODO: We could make this a bit faster by building a tree of the mountpoints
diff --git a/pkg/specgen/oci.go b/pkg/specgen/oci.go
index 2523f21b3..db60dc25e 100644
--- a/pkg/specgen/oci.go
+++ b/pkg/specgen/oci.go
@@ -215,11 +215,9 @@ func (s *SpecGenerator) toOCISpec(rt *libpod.Runtime, newImage *image.Image) (*s
// BIND MOUNTS
configSpec.Mounts = createconfig.SupercedeUserMounts(s.Mounts, configSpec.Mounts)
// Process mounts to ensure correct options
- finalMounts, err := createconfig.InitFSMounts(configSpec.Mounts)
- if err != nil {
+ if err := createconfig.InitFSMounts(configSpec.Mounts); err != nil {
return nil, err
}
- configSpec.Mounts = finalMounts
// Add annotations
if configSpec.Annotations == nil {
--
cgit v1.2.3-54-g00ecf
From e0614367ca27c2e47649e08291e0f1d19d42232e Mon Sep 17 00:00:00 2001
From: Kir Kolyshkin <kolyshkin@gmail.com>
Date: Tue, 31 Mar 2020 01:59:44 -0700
Subject: pkg/spec.InitFSMounts: optimize
Instead of getting mount options from /proc/self/mountinfo, which is
very costly to read/parse (and can even be unreliable), let's use
statfs(2) to figure out the flags we need.
[v2: move getting default options to pkg/util, make it linux-specific]
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
---
libpod/options.go | 2 +-
pkg/spec/storage.go | 60 ++++-----------------------------------------
pkg/util/mountOpts.go | 24 +++++++++---------
pkg/util/mountOpts_linux.go | 23 +++++++++++++++++
pkg/util/mountOpts_other.go | 7 ++++++
5 files changed, 49 insertions(+), 67 deletions(-)
create mode 100644 pkg/util/mountOpts_linux.go
create mode 100644 pkg/util/mountOpts_other.go
(limited to 'pkg/spec')
diff --git a/libpod/options.go b/libpod/options.go
index dfbec364a..65a089131 100644
--- a/libpod/options.go
+++ b/libpod/options.go
@@ -1334,7 +1334,7 @@ func WithNamedVolumes(volumes []*ContainerNamedVolume) CtrCreateOption {
}
destinations[vol.Dest] = true
- mountOpts, err := util.ProcessOptions(vol.Options, false, nil)
+ mountOpts, err := util.ProcessOptions(vol.Options, false, "")
if err != nil {
return errors.Wrapf(err, "error processing options for named volume %q mounted at %q", vol.Name, vol.Dest)
}
diff --git a/pkg/spec/storage.go b/pkg/spec/storage.go
index 335907d12..68a84d638 100644
--- a/pkg/spec/storage.go
+++ b/pkg/spec/storage.go
@@ -10,7 +10,6 @@ import (
"github.com/containers/buildah/pkg/parse"
"github.com/containers/libpod/libpod"
"github.com/containers/libpod/pkg/util"
- pmount "github.com/containers/storage/pkg/mount"
spec "github.com/opencontainers/runtime-spec/specs-go"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
@@ -856,44 +855,16 @@ func SupercedeUserMounts(mounts []spec.Mount, configMount []spec.Mount) []spec.M
// Ensure mount options on all mounts are correct
func InitFSMounts(mounts []spec.Mount) error {
- // We need to look up mounts so we can figure out the proper mount flags
- // to apply.
- systemMounts, err := pmount.GetMounts()
- if err != nil {
- return errors.Wrapf(err, "error retrieving system mounts to look up mount options")
- }
-
for i, m := range mounts {
- if m.Type == TypeBind {
- baseMnt, err := findMount(m.Source, systemMounts)
- if err != nil {
- return errors.Wrapf(err, "error looking up mountpoint for mount %s", m.Source)
- }
- var noexec, nosuid, nodev bool
- for _, baseOpt := range strings.Split(baseMnt.Opts, ",") {
- switch baseOpt {
- case "noexec":
- noexec = true
- case "nosuid":
- nosuid = true
- case "nodev":
- nodev = true
- }
- }
-
- defaultMountOpts := new(util.DefaultMountOptions)
- defaultMountOpts.Noexec = noexec
- defaultMountOpts.Nosuid = nosuid
- defaultMountOpts.Nodev = nodev
-
- opts, err := util.ProcessOptions(m.Options, false, defaultMountOpts)
+ switch {
+ case m.Type == TypeBind:
+ opts, err := util.ProcessOptions(m.Options, false, m.Source)
if err != nil {
return err
}
mounts[i].Options = opts
- }
- if m.Type == TypeTmpfs && filepath.Clean(m.Destination) != "/dev" {
- opts, err := util.ProcessOptions(m.Options, true, nil)
+ case m.Type == TypeTmpfs && filepath.Clean(m.Destination) != "/dev":
+ opts, err := util.ProcessOptions(m.Options, true, "")
if err != nil {
return err
}
@@ -902,24 +873,3 @@ func InitFSMounts(mounts []spec.Mount) error {
}
return nil
}
-
-// TODO: We could make this a bit faster by building a tree of the mountpoints
-// and traversing it to identify the correct mount.
-func findMount(target string, mounts []*pmount.Info) (*pmount.Info, error) {
- var err error
- target, err = filepath.Abs(target)
- if err != nil {
- return nil, errors.Wrapf(err, "cannot resolve %s", target)
- }
- var bestSoFar *pmount.Info
- for _, i := range mounts {
- if bestSoFar != nil && len(bestSoFar.Mountpoint) > len(i.Mountpoint) {
- // Won't be better than what we have already found
- continue
- }
- if strings.HasPrefix(target, i.Mountpoint) {
- bestSoFar = i
- }
- }
- return bestSoFar, nil
-}
diff --git a/pkg/util/mountOpts.go b/pkg/util/mountOpts.go
index d21800bc3..329a7c913 100644
--- a/pkg/util/mountOpts.go
+++ b/pkg/util/mountOpts.go
@@ -13,19 +13,17 @@ var (
ErrDupeMntOption = errors.Errorf("duplicate mount option passed")
)
-// DefaultMountOptions sets default mount options for ProcessOptions.
-type DefaultMountOptions struct {
- Noexec bool
- Nosuid bool
- Nodev bool
+type defaultMountOptions struct {
+ noexec bool
+ nosuid bool
+ nodev bool
}
// ProcessOptions parses the options for a bind or tmpfs mount and ensures that
// they are sensible and follow convention. The isTmpfs variable controls
// whether extra, tmpfs-specific options will be allowed.
-// The defaults variable controls default mount options that will be set. If it
-// is not included, they will be set unconditionally.
-func ProcessOptions(options []string, isTmpfs bool, defaults *DefaultMountOptions) ([]string, error) {
+// The sourcePath variable, if not empty, contains a bind mount source.
+func ProcessOptions(options []string, isTmpfs bool, sourcePath string) ([]string, error) {
var (
foundWrite, foundSize, foundProp, foundMode, foundExec, foundSuid, foundDev, foundCopyUp, foundBind, foundZ bool
)
@@ -122,13 +120,17 @@ func ProcessOptions(options []string, isTmpfs bool, defaults *DefaultMountOption
if !foundProp {
newOptions = append(newOptions, "rprivate")
}
- if !foundExec && (defaults == nil || defaults.Noexec) {
+ defaults, err := getDefaultMountOptions(sourcePath)
+ if err != nil {
+ return nil, err
+ }
+ if !foundExec && defaults.noexec {
newOptions = append(newOptions, "noexec")
}
- if !foundSuid && (defaults == nil || defaults.Nosuid) {
+ if !foundSuid && defaults.nosuid {
newOptions = append(newOptions, "nosuid")
}
- if !foundDev && (defaults == nil || defaults.Nodev) {
+ if !foundDev && defaults.nodev {
newOptions = append(newOptions, "nodev")
}
if isTmpfs && !foundCopyUp {
diff --git a/pkg/util/mountOpts_linux.go b/pkg/util/mountOpts_linux.go
new file mode 100644
index 000000000..3eac4dd25
--- /dev/null
+++ b/pkg/util/mountOpts_linux.go
@@ -0,0 +1,23 @@
+package util
+
+import (
+ "os"
+
+ "golang.org/x/sys/unix"
+)
+
+func getDefaultMountOptions(path string) (defaultMountOptions, error) {
+ opts := defaultMountOptions{true, true, true}
+ if path == "" {
+ return opts, nil
+ }
+ var statfs unix.Statfs_t
+ if e := unix.Statfs(path, &statfs); e != nil {
+ return opts, &os.PathError{Op: "statfs", Path: path, Err: e}
+ }
+ opts.nodev = (statfs.Flags&unix.MS_NODEV == unix.MS_NODEV)
+ opts.noexec = (statfs.Flags&unix.MS_NOEXEC == unix.MS_NOEXEC)
+ opts.nosuid = (statfs.Flags&unix.MS_NOSUID == unix.MS_NOSUID)
+
+ return opts, nil
+}
diff --git a/pkg/util/mountOpts_other.go b/pkg/util/mountOpts_other.go
new file mode 100644
index 000000000..6a34942e5
--- /dev/null
+++ b/pkg/util/mountOpts_other.go
@@ -0,0 +1,7 @@
+// +build !linux
+
+package util
+
+func getDefaultMountOptions(path string) (opts defaultMountOptions, err error) {
+ return
+}
--
cgit v1.2.3-54-g00ecf