From 5bdd97f77fc3100c6338928d4d54af32273d36fb Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Fri, 23 Aug 2019 13:24:06 -0400 Subject: Set base mount options for bind mounts from base system If I mount, say, /usr/bin into my container - I expect to be able to run the executables in that mount. Unconditionally applying noexec would be a bad idea. Before my patches to change mount options and allow exec/dev/suid being set explicitly, we inferred the mount options from where on the base system the mount originated, and the options it had there. Implement the same functionality for the new option handling. There's a lot of performance left on the table here, but I don't know that this is ever going to take enough time to make it worth optimizing. Signed-off-by: Matthew Heon --- pkg/spec/spec.go | 21 -------------------- pkg/spec/storage.go | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 53 insertions(+), 23 deletions(-) (limited to 'pkg/spec') diff --git a/pkg/spec/spec.go b/pkg/spec/spec.go index 779fb5290..44bbda885 100644 --- a/pkg/spec/spec.go +++ b/pkg/spec/spec.go @@ -2,13 +2,11 @@ package createconfig import ( "os" - "path/filepath" "strings" "github.com/containers/libpod/libpod" "github.com/containers/libpod/pkg/cgroups" "github.com/containers/libpod/pkg/rootless" - pmount "github.com/containers/storage/pkg/mount" "github.com/docker/docker/oci/caps" "github.com/docker/go-units" "github.com/opencontainers/runc/libcontainer/user" @@ -457,25 +455,6 @@ func (config *CreateConfig) createConfigToOCISpec(runtime *libpod.Runtime, userM return configSpec, nil } -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 -} - func blockAccessToKernelFilesystems(config *CreateConfig, g *generate.Generator) { if !config.Privileged { for _, mp := range []string{ diff --git a/pkg/spec/storage.go b/pkg/spec/storage.go index 9f4248924..7ed21a055 100644 --- a/pkg/spec/storage.go +++ b/pkg/spec/storage.go @@ -10,6 +10,7 @@ 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" "github.com/containers/storage/pkg/stringid" spec "github.com/opencontainers/runtime-spec/specs-go" "github.com/pkg/errors" @@ -816,17 +817,46 @@ 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) { + // 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") + } + + // TODO: We probably don't need to re-build the mounts array var mounts []spec.Mount for _, m := range inputMounts { if m.Type == TypeBind { - opts, err := util.ProcessOptions(m.Options, false) + baseMnt, err := findMount(m.Destination, systemMounts) + if err != nil { + return nil, errors.Wrapf(err, "error looking up mountpoint for mount %s", m.Destination) + } + 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) if err != nil { return nil, err } m.Options = opts } if m.Type == TypeTmpfs && filepath.Clean(m.Destination) != "/dev" { - opts, err := util.ProcessOptions(m.Options, true) + opts, err := util.ProcessOptions(m.Options, true, nil) if err != nil { return nil, err } @@ -837,3 +867,24 @@ func initFSMounts(inputMounts []spec.Mount) ([]spec.Mount, error) { } return mounts, 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 +} -- cgit v1.2.3-54-g00ecf