diff options
422 files changed, 13444 insertions, 3329 deletions
diff --git a/HACKING.md b/HACKING.md index c069633482..03e83a8955 100644 --- a/HACKING.md +++ b/HACKING.md @@ -126,6 +126,8 @@ To run the various tests that we have to ensure a high quality source just run: This will check if the source format is consistent, that it builds, all tests work as expected and that "go vet" has nothing to complain. +The source format follows the `gofmt -s` formating. Please run this on your sources files if `run-checks` complains about the format. + You can run individual test for a sub-package by changing into that directory and: go test -check.f $testname diff --git a/advisor/backend.go b/advisor/backend.go index 8856b5a7a3..ab6baee0e5 100644 --- a/advisor/backend.go +++ b/advisor/backend.go @@ -28,18 +28,22 @@ import ( "github.com/snapcore/snapd/dirs" ) -var cmdBucketKey = []byte("Commands") +var ( + cmdBucketKey = []byte("Commands") + pkgBucketKey = []byte("Snaps") +) type writer struct { - db *bolt.DB - tx *bolt.Tx - bucket *bolt.Bucket + db *bolt.DB + tx *bolt.Tx + cmdBucket *bolt.Bucket + pkgBucket *bolt.Bucket } type CommandDB interface { // AddSnap adds the entries for commands pointing to the given // snap name to the commands database. - AddSnap(snapName string, commands []string) error + AddSnap(snapName, summary string, commands []string) error // Commit persist the changes, and closes the database. If the // database has already been committed/rollbacked, does nothing. Commit() error @@ -64,12 +68,23 @@ func Create() (CommandDB, error) { t.tx, err = t.db.Begin(true) if err == nil { - err = t.tx.DeleteBucket(cmdBucketKey) + err := t.tx.DeleteBucket(cmdBucketKey) if err == nil || err == bolt.ErrBucketNotFound { - t.bucket, err = t.tx.CreateBucket(cmdBucketKey) + t.cmdBucket, err = t.tx.CreateBucket(cmdBucketKey) } if err != nil { t.tx.Rollback() + + } + + if err == nil { + err := t.tx.DeleteBucket(pkgBucketKey) + if err == nil || err == bolt.ErrBucketNotFound { + t.pkgBucket, err = t.tx.CreateBucket(pkgBucketKey) + } + if err != nil { + t.tx.Rollback() + } } } @@ -81,22 +96,26 @@ func Create() (CommandDB, error) { return t, nil } -func (t *writer) AddSnap(snapName string, commands []string) error { +func (t *writer) AddSnap(snapName, summary string, commands []string) error { bname := []byte(snapName) for _, cmd := range commands { bcmd := []byte(cmd) - row := t.bucket.Get(bcmd) + row := t.cmdBucket.Get(bcmd) if row == nil { row = bname } else { row = append(append(row, ','), bname...) } - if err := t.bucket.Put(bcmd, row); err != nil { + if err := t.cmdBucket.Put(bcmd, row); err != nil { return err } } + if err := t.pkgBucket.Put([]byte(snapName), []byte(summary)); err != nil { + return err + } + return nil } @@ -111,7 +130,8 @@ func (t *writer) Rollback() error { func (t *writer) done(commit bool) error { var e1, e2 error - t.bucket = nil + t.cmdBucket = nil + t.pkgBucket = nil if t.tx != nil { if commit { e1 = t.tx.Commit() @@ -130,9 +150,10 @@ func (t *writer) done(commit bool) error { return e1 } -// Dump returns the whole database as a map. For use in testing and debugging. -func Dump() (map[string][]string, error) { - db, err := bolt.Open(dirs.SnapCommandsDB, 0600, &bolt.Options{ +// DumpCommands returns the whole database as a map. For use in +// testing and debugging. +func DumpCommands() (map[string][]string, error) { + db, err := bolt.Open(dirs.SnapCommandsDB, 0644, &bolt.Options{ ReadOnly: true, Timeout: 1 * time.Second, }) @@ -167,7 +188,7 @@ type boltFinder struct { // Open the database for reading. func Open() (Finder, error) { - db, err := bolt.Open(dirs.SnapCommandsDB, 0600, &bolt.Options{ + db, err := bolt.Open(dirs.SnapCommandsDB, 0644, &bolt.Options{ ReadOnly: true, Timeout: 1 * time.Second, }) @@ -178,7 +199,7 @@ func Open() (Finder, error) { return &boltFinder{*db}, nil } -func (f *boltFinder) Find(command string) ([]Command, error) { +func (f *boltFinder) FindCommand(command string) ([]Command, error) { tx, err := f.Begin(false) if err != nil { return nil, err @@ -206,3 +227,23 @@ func (f *boltFinder) Find(command string) ([]Command, error) { return cmds, nil } + +func (f *boltFinder) FindPackage(pkgName string) (*Package, error) { + tx, err := f.Begin(false) + if err != nil { + return nil, err + } + defer tx.Rollback() + + b := tx.Bucket(pkgBucketKey) + if b == nil { + return nil, nil + } + + bsummary := b.Get([]byte(pkgName)) + if bsummary == nil { + return nil, nil + } + + return &Package{Snap: pkgName, Summary: string(bsummary)}, nil +} diff --git a/advisor/cmdfinder.go b/advisor/cmdfinder.go index 0960dfda08..15d50eb388 100644 --- a/advisor/cmdfinder.go +++ b/advisor/cmdfinder.go @@ -24,8 +24,6 @@ type Command struct { Command string } -var newFinder = Open - func FindCommand(command string) ([]Command, error) { finder, err := newFinder() if err != nil { @@ -33,7 +31,7 @@ func FindCommand(command string) ([]Command, error) { } defer finder.Close() - return finder.Find(command) + return finder.FindCommand(command) } const ( @@ -88,7 +86,7 @@ func FindMisspelledCommand(command string) ([]Command, error) { alternatives := make([]Command, 0, 32) for _, w := range similarWords(command) { - res, err := finder.Find(w) + res, err := finder.FindCommand(w) if err != nil { return nil, err } @@ -99,16 +97,3 @@ func FindMisspelledCommand(command string) ([]Command, error) { return alternatives, nil } - -type Finder interface { - Find(command string) ([]Command, error) - Close() error -} - -func ReplaceCommandsFinder(constructor func() (Finder, error)) (restore func()) { - old := newFinder - newFinder = constructor - return func() { - newFinder = old - } -} diff --git a/advisor/cmdfinder_test.go b/advisor/cmdfinder_test.go index c84c918615..da8c1028c1 100644 --- a/advisor/cmdfinder_test.go +++ b/advisor/cmdfinder_test.go @@ -44,8 +44,8 @@ func (s *cmdfinderSuite) SetUpTest(c *C) { db, err := advisor.Create() c.Assert(err, IsNil) - c.Assert(db.AddSnap("foo", []string{"foo", "meh"}), IsNil) - c.Assert(db.AddSnap("bar", []string{"bar", "meh"}), IsNil) + c.Assert(db.AddSnap("foo", "foo summary", []string{"foo", "meh"}), IsNil) + c.Assert(db.AddSnap("bar", "bar summary", []string{"bar", "meh"}), IsNil) c.Assert(db.Commit(), IsNil) } @@ -125,8 +125,8 @@ func (s *cmdfinderSuite) TestFindMisspelledCommandMiss(c *C) { c.Check(cmds, HasLen, 0) } -func (s *cmdfinderSuite) TestDump(c *C) { - cmds, err := advisor.Dump() +func (s *cmdfinderSuite) TestDumpCommands(c *C) { + cmds, err := advisor.DumpCommands() c.Assert(err, IsNil) c.Check(cmds, DeepEquals, map[string][]string{ "foo": {"foo"}, diff --git a/advisor/finder.go b/advisor/finder.go new file mode 100644 index 0000000000..ca3b9982fe --- /dev/null +++ b/advisor/finder.go @@ -0,0 +1,36 @@ +// -*- Mode: Go; indent-tabs-mode: t -*- + +/* + * Copyright (C) 2018 Canonical Ltd + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 3 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + * + */ + +package advisor + +var newFinder = Open + +type Finder interface { + FindCommand(command string) ([]Command, error) + FindPackage(pkgName string) (*Package, error) + Close() error +} + +func ReplaceCommandsFinder(constructor func() (Finder, error)) (restore func()) { + old := newFinder + newFinder = constructor + return func() { + newFinder = old + } +} diff --git a/advisor/pkgfinder.go b/advisor/pkgfinder.go new file mode 100644 index 0000000000..43dd143203 --- /dev/null +++ b/advisor/pkgfinder.go @@ -0,0 +1,35 @@ +// -*- Mode: Go; indent-tabs-mode: t -*- + +/* + * Copyright (C) 2018 Canonical Ltd + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 3 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + * + */ + +package advisor + +type Package struct { + Snap string + Summary string +} + +func FindPackage(pkgName string) (*Package, error) { + finder, err := newFinder() + if err != nil { + return nil, err + } + defer finder.Close() + + return finder.FindPackage(pkgName) +} diff --git a/boot/kernel_os_test.go b/boot/kernel_os_test.go index d37ca865c5..7e37cd5211 100644 --- a/boot/kernel_os_test.go +++ b/boot/kernel_os_test.go @@ -20,7 +20,6 @@ package boot_test import ( - "io/ioutil" "path/filepath" "testing" @@ -34,6 +33,7 @@ import ( "github.com/snapcore/snapd/release" "github.com/snapcore/snapd/snap" "github.com/snapcore/snapd/snap/snaptest" + "github.com/snapcore/snapd/testutil" ) func TestBoot(t *testing.T) { TestingT(t) } @@ -97,9 +97,7 @@ func (s *kernelOSSuite) TestExtractKernelAssetsAndRemove(c *C) { } fullFn := filepath.Join(kernelAssetsDir, def[0]) - content, err := ioutil.ReadFile(fullFn) - c.Assert(err, IsNil) - c.Assert(string(content), Equals, def[1]) + c.Check(fullFn, testutil.FileEquals, def[1]) } // remove @@ -152,7 +150,7 @@ func (s *kernelOSSuite) TestSetNextBootOnClassic(c *C) { defer restore() // Create a fake OS snap that we try to update - snapInfo := snaptest.MockSnap(c, "name: os\ntype: os", "SNAP", &snap.SideInfo{Revision: snap.R(42)}) + snapInfo := snaptest.MockSnap(c, "name: os\ntype: os", &snap.SideInfo{Revision: snap.R(42)}) err := boot.SetNextBoot(snapInfo) c.Assert(err, IsNil) diff --git a/client/client.go b/client/client.go index fff5d9cdee..8708874b4e 100644 --- a/client/client.go +++ b/client/client.go @@ -395,8 +395,12 @@ type OSRelease struct { VersionID string `json:"version-id,omitempty"` } +// RefreshInfo contains information about refreshes. type RefreshInfo struct { - Schedule string `json:"schedule"` + // Timer contains the refresh.timer setting. + Timer string `json:"timer,omitempty"` + // Schedule contains the legacy refresh.schedule setting. + Schedule string `json:"schedule,omitempty"` Last string `json:"last,omitempty"` Next string `json:"next,omitempty"` } diff --git a/client/login_test.go b/client/login_test.go index 309224ec59..dafbe51809 100644 --- a/client/login_test.go +++ b/client/login_test.go @@ -29,6 +29,7 @@ import ( "github.com/snapcore/snapd/client" "github.com/snapcore/snapd/osutil" + "github.com/snapcore/snapd/testutil" ) func (cs *clientSuite) TestClientLogin(c *check.C) { @@ -53,9 +54,7 @@ func (cs *clientSuite) TestClientLogin(c *check.C) { c.Assert(cs.cli.LoggedInUser(), check.Not(check.IsNil)) c.Check(osutil.FileExists(outfile), check.Equals, true) - content, err := ioutil.ReadFile(outfile) - c.Check(err, check.IsNil) - c.Check(string(content), check.Equals, `{"username":"the-user-name","macaroon":"the-root-macaroon","discharges":["discharge-macaroon"]}`) + c.Check(outfile, testutil.FileEquals, `{"username":"the-user-name","macaroon":"the-root-macaroon","discharges":["discharge-macaroon"]}`) } func (cs *clientSuite) TestClientLoginWhenLoggedIn(c *check.C) { @@ -90,9 +89,7 @@ func (cs *clientSuite) TestClientLoginWhenLoggedIn(c *check.C) { c.Assert(cs.cli.LoggedInUser(), check.DeepEquals, expected) c.Check(osutil.FileExists(outfile), check.Equals, true) - content, err := ioutil.ReadFile(outfile) - c.Check(err, check.IsNil) - c.Check(string(content), check.Equals, `{"username":"the-user-name","email":"zed@bar.com","macaroon":"the-root-macaroon","discharges":["discharge-macaroon"]}`) + c.Check(outfile, testutil.FileEquals, `{"username":"the-user-name","email":"zed@bar.com","macaroon":"the-root-macaroon","discharges":["discharge-macaroon"]}`) } func (cs *clientSuite) TestClientLoginError(c *check.C) { @@ -146,9 +143,7 @@ func (cs *clientSuite) TestWriteAuthData(c *check.C) { c.Assert(err, check.IsNil) c.Check(osutil.FileExists(outfile), check.Equals, true) - content, err := ioutil.ReadFile(outfile) - c.Check(err, check.IsNil) - c.Check(string(content), check.Equals, `{"macaroon":"macaroon","discharges":["discharge"]}`) + c.Check(outfile, testutil.FileEquals, `{"macaroon":"macaroon","discharges":["discharge"]}`) } func (cs *clientSuite) TestReadAuthData(c *check.C) { diff --git a/client/snap_op.go b/client/snap_op.go index 75d5aeead0..ad1637d338 100644 --- a/client/snap_op.go +++ b/client/snap_op.go @@ -30,6 +30,7 @@ import ( ) type SnapOptions struct { + Amend bool `json:"amend,omitempty"` Channel string `json:"channel,omitempty"` Revision string `json:"revision,omitempty"` DevMode bool `json:"devmode,omitempty"` diff --git a/cmd/Makefile.am b/cmd/Makefile.am index 505b75aa55..c6a47b62df 100644 --- a/cmd/Makefile.am +++ b/cmd/Makefile.am @@ -15,7 +15,7 @@ if WITH_UNIT_TESTS CHECK_CFLAGS += -Werror endif -subdirs = snap-confine snap-discard-ns system-shutdown libsnap-confine-private +subdirs = snap-confine snap-discard-ns system-shutdown libsnap-confine-private snap-gdb-shim # Run check-syntax when checking # TODO: conver those to autotools-style tests later @@ -205,7 +205,7 @@ snap_confine_snap_confine_SOURCES = \ snap-confine/user-support.c \ snap-confine/user-support.h -snap_confine_snap_confine_CFLAGS = $(CHECK_CFLAGS) $(AM_CFLAGS) -DLIBEXECDIR=\"$(libexecdir)\" +snap_confine_snap_confine_CFLAGS = $(CHECK_CFLAGS) $(AM_CFLAGS) -DLIBEXECDIR=\"$(libexecdir)\" -DNATIVE_LIBDIR=\"$(libdir)\" snap_confine_snap_confine_LDFLAGS = $(AM_LDFLAGS) snap_confine_snap_confine_LDADD = libsnap-confine-private.a snap_confine_snap_confine_CFLAGS += $(LIBUDEV_CFLAGS) @@ -342,7 +342,7 @@ endif libexec_PROGRAMS += snap-mgmt/snap-mgmt -snap-mgmt/snap-mgmt: snap-mgmt/snap-mgmt.sh.in Makefile +snap-mgmt/snap-mgmt: snap-mgmt/snap-mgmt.sh.in Makefile snap-mgmt/$(am__dirstamp) sed -e 's,[@]SNAP_MOUNT_DIR[@],$(SNAP_MOUNT_DIR),' <$< >$@ @@ -355,19 +355,19 @@ install-exec-hook:: ln -sf $(libexecdir)/snap-confine $(DESTDIR)$(bindir)/ubuntu-core-launcher ## -## snappy-app-dev +## snap-device-helper ## EXTRA_DIST += \ - snap-confine/snappy-app-dev + snap-confine/snap-device-helper # NOTE: This makes distcheck fail but it is required for udev, so go figure. # http://www.gnu.org/software/automake/manual/automake.html#Hard_002dCoded-Install-Paths # # Install support script for udev rules install-exec-local:: - install -d -m 755 $(DESTDIR)$(shell pkg-config udev --variable=udevdir) - install -m 755 $(srcdir)/snap-confine/snappy-app-dev $(DESTDIR)$(shell pkg-config udev --variable=udevdir) + install -d -m 755 $(DESTDIR)$(libexecdir) + install -m 755 $(srcdir)/snap-confine/snap-device-helper $(DESTDIR)$(libexecdir) ## ## snap-discard-ns @@ -443,3 +443,14 @@ system_shutdown_unit_tests_LDADD = libsnap-confine-private.a system_shutdown_unit_tests_CFLAGS = $(GLIB_CFLAGS) system_shutdown_unit_tests_LDADD += $(GLIB_LIBS) endif + +## +## snap-gdb-shim +## + +libexec_PROGRAMS += snap-gdb-shim/snap-gdb-shim + +snap_gdb_shim_snap_gdb_shim_SOURCES = \ + snap-gdb-shim/snap-gdb-shim.c + +snap_gdb_shim_snap_gdb_shim_LDADD = libsnap-confine-private.a diff --git a/cmd/configure.ac b/cmd/configure.ac index f2e6ce6efb..74f7474379 100644 --- a/cmd/configure.ac +++ b/cmd/configure.ac @@ -209,5 +209,12 @@ AC_ARG_ENABLE([static-libseccomp], esac], [enable_static_libseccomp=no]) AM_CONDITIONAL([STATIC_LIBSECCOMP], [test "x$enable_static_libseccomp" = "xyes"]) +LIB32_DIR="${prefix}/lib32" +AC_ARG_WITH([32bit-libdir], + AS_HELP_STRING([--with-32bit-libdir=DIR], [Use an alternate lib32 directory]), + [LIB32_DIR="$withval"]) +AC_SUBST(LIB32_DIR) +AC_DEFINE_UNQUOTED([LIB32_DIR], "${LIB32_DIR}", [Location of the lib32 directory]) + AC_CONFIG_FILES([Makefile]) AC_OUTPUT diff --git a/cmd/libsnap-confine-private/cgroup-freezer-support.c b/cmd/libsnap-confine-private/cgroup-freezer-support.c index 39fac02a3b..a5cfa047bf 100644 --- a/cmd/libsnap-confine-private/cgroup-freezer-support.c +++ b/cmd/libsnap-confine-private/cgroup-freezer-support.c @@ -19,7 +19,7 @@ static const char *freezer_cgroup_dir = "/sys/fs/cgroup/freezer"; void sc_cgroup_freezer_join(const char *snap_name, pid_t pid) { - // Format the name of the cgroup hierarchy. + // Format the name of the cgroup hierarchy. char buf[PATH_MAX] = { 0 }; sc_must_snprintf(buf, sizeof buf, "snap.%s", snap_name); @@ -65,3 +65,85 @@ void sc_cgroup_freezer_join(const char *snap_name, pid_t pid) debug("moved process %ld to freezer cgroup hierarchy for snap %s", (long)pid, snap_name); } + +bool sc_cgroup_freezer_occupied(const char *snap_name) +{ + // Format the name of the cgroup hierarchy. + char buf[PATH_MAX] = { 0 }; + sc_must_snprintf(buf, sizeof buf, "snap.%s", snap_name); + + // Open the freezer cgroup directory. + int cgroup_fd SC_CLEANUP(sc_cleanup_close) = -1; + cgroup_fd = open(freezer_cgroup_dir, + O_PATH | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC); + if (cgroup_fd < 0) { + die("cannot open freezer cgroup (%s)", freezer_cgroup_dir); + } + // Open the proc directory. + int proc_fd SC_CLEANUP(sc_cleanup_close) = -1; + proc_fd = open("/proc", O_PATH | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC); + if (proc_fd < 0) { + die("cannot open /proc"); + } + // Open the hierarchy directory for the given snap. + int hierarchy_fd SC_CLEANUP(sc_cleanup_close) = -1; + hierarchy_fd = openat(cgroup_fd, buf, + O_PATH | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC); + if (hierarchy_fd < 0) { + if (errno == ENOENT) { + return false; + } + die("cannot open freezer cgroup hierarchy for snap %s", + snap_name); + } + // Open the "cgroup.procs" file. Alternatively we could open the "tasks" + // file and see per-thread data but we don't need that. + int cgroup_procs_fd SC_CLEANUP(sc_cleanup_close) = -1; + cgroup_procs_fd = openat(hierarchy_fd, "cgroup.procs", + O_RDONLY | O_NOFOLLOW | O_CLOEXEC); + if (cgroup_procs_fd < 0) { + die("cannot open cgroup.procs file for freezer cgroup hierarchy for snap %s", snap_name); + } + + FILE *cgroup_procs SC_CLEANUP(sc_cleanup_file) = NULL; + cgroup_procs = fdopen(cgroup_procs_fd, "r"); + if (cgroup_procs == NULL) { + die("cannot convert tasks file descriptor to FILE"); + } + cgroup_procs_fd = -1; // cgroup_procs_fd will now be closed by fclose. + + char *line_buf SC_CLEANUP(sc_cleanup_string) = NULL; + size_t line_buf_size = 0; + ssize_t num_read; + struct stat statbuf; + do { + num_read = getline(&line_buf, &line_buf_size, cgroup_procs); + if (num_read < 0 && errno != 0) { + die("cannot read next PID belonging to snap %s", + snap_name); + } + if (num_read <= 0) { + break; + } else { + if (line_buf[num_read - 1] == '\n') { + line_buf[num_read - 1] = '\0'; + } else { + die("could not find newline in cgroup.procs"); + } + } + debug("found process id: %s\n", line_buf); + + if (fstatat(proc_fd, line_buf, &statbuf, AT_SYMLINK_NOFOLLOW) < + 0) { + // The process may have died already. + if (errno != ENOENT) { + die("cannot stat /proc/%s", line_buf); + } + } + debug("found process %s belonging to user %d", + line_buf, statbuf.st_uid); + return true; + } while (num_read > 0); + + return false; +} diff --git a/cmd/libsnap-confine-private/cgroup-freezer-support.h b/cmd/libsnap-confine-private/cgroup-freezer-support.h index 06f9be7a11..259dca6d8f 100644 --- a/cmd/libsnap-confine-private/cgroup-freezer-support.h +++ b/cmd/libsnap-confine-private/cgroup-freezer-support.h @@ -23,4 +23,13 @@ **/ void sc_cgroup_freezer_join(const char *snap_name, pid_t pid); +/** + * Check if a freezer cgroup for given snap has any processes belonging to a given user. + * + * This function examines the freezer cgroup called "snap.$snap_name" and looks + * at each of its processes. If any process exists then the function returns true. +**/ +// TODO: Support per user filtering for eventual per-user mount namespaces +bool sc_cgroup_freezer_occupied(const char *snap_name); + #endif diff --git a/cmd/snap-confine/mount-support-nvidia.c b/cmd/snap-confine/mount-support-nvidia.c index 42f56c3dce..fdd588dfd7 100644 --- a/cmd/snap-confine/mount-support-nvidia.c +++ b/cmd/snap-confine/mount-support-nvidia.c @@ -26,6 +26,7 @@ #include <sys/mount.h> #include <sys/stat.h> #include <sys/types.h> +#include <stdint.h> #include <unistd.h> #include "../libsnap-confine-private/classic.h" @@ -38,13 +39,16 @@ // note: if the parent dir changes to something other than // the current /var/lib/snapd/lib then sc_mkdir_and_mount_and_bind // and sc_mkdir_and_mount_and_bind need updating. -#define SC_LIBGL_DIR "/var/lib/snapd/lib/gl" -#define SC_LIBGL32_DIR "/var/lib/snapd/lib/gl32" -#define SC_VULKAN_DIR "/var/lib/snapd/lib/vulkan" +#define SC_LIB "/var/lib/snapd/lib" +#define SC_LIBGL_DIR SC_LIB "/gl" +#define SC_LIBGL32_DIR SC_LIB "/gl32" +#define SC_VULKAN_DIR SC_LIB "/vulkan" + +#define SC_VULKAN_SOURCE_DIR "/usr/share/vulkan" // Location for NVIDIA vulkan files (including _wayland) static const char *vulkan_globs[] = { - "/usr/share/vulkan/icd.d/*nvidia*.json", + "icd.d/*nvidia*.json", }; static const size_t vulkan_globs_len = @@ -65,79 +69,42 @@ static const size_t vulkan_globs_len = // FIXME: this doesn't yet work with libGLX and libglvnd redirector // FIXME: this still doesn't work with the 361 driver static const char *nvidia_globs[] = { - "/usr/lib/libEGL.so*", - "/usr/lib/libEGL_nvidia.so*", - "/usr/lib/libGL.so*", - "/usr/lib/libOpenGL.so*", - "/usr/lib/libGLESv1_CM.so*", - "/usr/lib/libGLESv1_CM_nvidia.so*", - "/usr/lib/libGLESv2.so*", - "/usr/lib/libGLESv2_nvidia.so*", - "/usr/lib/libGLX_indirect.so*", - "/usr/lib/libGLX_nvidia.so*", - "/usr/lib/libGLX.so*", - "/usr/lib/libGLdispatch.so*", - "/usr/lib/libGLU.so*", - "/usr/lib/libXvMCNVIDIA.so*", - "/usr/lib/libXvMCNVIDIA_dynamic.so*", - "/usr/lib/libcuda.so*", - "/usr/lib/libnvcuvid.so*", - "/usr/lib/libnvidia-cfg.so*", - "/usr/lib/libnvidia-compiler.so*", - "/usr/lib/libnvidia-eglcore.so*", - "/usr/lib/libnvidia-egl-wayland*", - "/usr/lib/libnvidia-encode.so*", - "/usr/lib/libnvidia-fatbinaryloader.so*", - "/usr/lib/libnvidia-fbc.so*", - "/usr/lib/libnvidia-glcore.so*", - "/usr/lib/libnvidia-glsi.so*", - "/usr/lib/libnvidia-ifr.so*", - "/usr/lib/libnvidia-ml.so*", - "/usr/lib/libnvidia-ptxjitcompiler.so*", - "/usr/lib/libnvidia-tls.so*", - "/usr/lib/vdpau/libvdpau_nvidia.so*", + "libEGL.so*", + "libEGL_nvidia.so*", + "libGL.so*", + "libOpenGL.so*", + "libGLESv1_CM.so*", + "libGLESv1_CM_nvidia.so*", + "libGLESv2.so*", + "libGLESv2_nvidia.so*", + "libGLX_indirect.so*", + "libGLX_nvidia.so*", + "libGLX.so*", + "libGLdispatch.so*", + "libGLU.so*", + "libXvMCNVIDIA.so*", + "libXvMCNVIDIA_dynamic.so*", + "libcuda.so*", + "libnvcuvid.so*", + "libnvidia-cfg.so*", + "libnvidia-compiler.so*", + "libnvidia-eglcore.so*", + "libnvidia-egl-wayland*", + "libnvidia-encode.so*", + "libnvidia-fatbinaryloader.so*", + "libnvidia-fbc.so*", + "libnvidia-glcore.so*", + "libnvidia-glsi.so*", + "libnvidia-ifr.so*", + "libnvidia-ml.so*", + "libnvidia-ptxjitcompiler.so*", + "libnvidia-tls.so*", + "vdpau/libvdpau_nvidia.so*", }; static const size_t nvidia_globs_len = sizeof nvidia_globs / sizeof *nvidia_globs; -// 32-bit variants of the NVIDIA driver libraries -static const char *nvidia_globs32[] = { - "/usr/lib32/libEGL.so*", - "/usr/lib32/libEGL_nvidia.so*", - "/usr/lib32/libGL.so*", - "/usr/lib32/libOpenGL.so*", - "/usr/lib32/libGLESv1_CM.so*", - "/usr/lib32/libGLESv1_CM_nvidia.so*", - "/usr/lib32/libGLESv2.so*", - "/usr/lib32/libGLESv2_nvidia.so*", - "/usr/lib32/libGLX_indirect.so*", - "/usr/lib32/libGLX_nvidia.so*", - "/usr/lib32/libGLX.so*", - "/usr/lib32/libGLdispatch.so*", - "/usr/lib32/libGLU.so*", - "/usr/lib32/libXvMCNVIDIA.so*", - "/usr/lib32/libXvMCNVIDIA_dynamic.so*", - "/usr/lib32/libcuda.so*", - "/usr/lib32/libnvcuvid.so*", - "/usr/lib32/libnvidia-cfg.so*", - "/usr/lib32/libnvidia-compiler.so*", - "/usr/lib32/libnvidia-eglcore.so*", - "/usr/lib32/libnvidia-encode.so*", - "/usr/lib32/libnvidia-fatbinaryloader.so*", - "/usr/lib32/libnvidia-fbc.so*", - "/usr/lib32/libnvidia-glcore.so*", - "/usr/lib32/libnvidia-glsi.so*", - "/usr/lib32/libnvidia-ifr.so*", - "/usr/lib32/libnvidia-ml.so*", - "/usr/lib32/libnvidia-ptxjitcompiler.so*", - "/usr/lib32/libnvidia-tls.so*", - "/usr/lib32/vdpau/libvdpau_nvidia.so*", -}; - -static const size_t nvidia_globs32_len = - sizeof nvidia_globs32 / sizeof *nvidia_globs32; - #endif // ifdef NVIDIA_BIARCH // Populate libgl_dir with a symlink farm to files matching glob_list. @@ -147,7 +114,11 @@ static const size_t nvidia_globs32_len = // If the library is a symbolic link then relative links are kept as-is but // absolute links are translated to have "/path/to/hostfs" up front so that // they work after the pivot_root elsewhere. +// +// The glob list passed to us is produced with paths relative to source dir, +// to simplify the various tie-in points with this function. static void sc_populate_libgl_with_hostfs_symlinks(const char *libgl_dir, + const char *source_dir, const char *glob_list[], size_t glob_list_len) { @@ -156,13 +127,17 @@ static void sc_populate_libgl_with_hostfs_symlinks(const char *libgl_dir, // Find all the entries matching the list of globs for (size_t i = 0; i < glob_list_len; ++i) { const char *glob_pattern = glob_list[i]; - int err = - glob(glob_pattern, i ? GLOB_APPEND : 0, NULL, &glob_res); + char glob_pattern_full[512] = { 0 }; + sc_must_snprintf(glob_pattern_full, sizeof glob_pattern_full, + "%s/%s", source_dir, glob_pattern); + + int err = glob(glob_pattern_full, i ? GLOB_APPEND : 0, NULL, + &glob_res); // Not all of the files have to be there (they differ depending on the // driver version used). Ignore all errors that are not GLOB_NOMATCH. if (err != 0 && err != GLOB_NOMATCH) { die("cannot search using glob pattern %s: %d", - glob_pattern, err); + glob_pattern_full, err); } } // Symlink each file found @@ -216,6 +191,15 @@ static void sc_populate_libgl_with_hostfs_symlinks(const char *libgl_dir, "%s/%s", libgl_dir, filename); debug("creating symbolic link %s -> %s", symlink_name, symlink_target); + + // Make sure we don't have some link already (merged GLVND systems) + if (lstat(symlink_name, &stat_buf) == 0) { + if (unlink(symlink_name) != 0) { + die("cannot remove symbolic link target %s", + symlink_name); + } + } + if (symlink(symlink_target, symlink_name) != 0) { die("cannot create symbolic link %s -> %s", symlink_name, symlink_target); @@ -224,6 +208,8 @@ static void sc_populate_libgl_with_hostfs_symlinks(const char *libgl_dir, } static void sc_mkdir_and_mount_and_glob_files(const char *rootfs_dir, + const char *source_dir[], + size_t source_dir_len, const char *tgt_dir, const char *glob_list[], size_t glob_list_len) @@ -237,14 +223,22 @@ static void sc_mkdir_and_mount_and_glob_files(const char *rootfs_dir, if (res != 0 && errno != EEXIST) { die("cannot create tmpfs target %s", libgl_dir); } + if (res == 0 && (chown(libgl_dir, 0, 0) < 0)) { + // Adjust the ownership only if we created the directory. + die("cannot change ownership of %s", libgl_dir); + } debug("mounting tmpfs at %s", libgl_dir); if (mount("none", libgl_dir, "tmpfs", MS_NODEV | MS_NOEXEC, NULL) != 0) { die("cannot mount tmpfs at %s", libgl_dir); }; - // Populate libgl_dir with symlinks to libraries from hostfs - sc_populate_libgl_with_hostfs_symlinks(libgl_dir, glob_list, - glob_list_len); + + for (size_t i = 0; i < source_dir_len; i++) { + // Populate libgl_dir with symlinks to libraries from hostfs + sc_populate_libgl_with_hostfs_symlinks(libgl_dir, source_dir[i], + glob_list, + glob_list_len); + } // Remount $tgt_dir (i.e. .../lib/gl) read only debug("remounting tmpfs as read-only %s", libgl_dir); if (mount(NULL, buf, NULL, MS_REMOUNT | MS_RDONLY, NULL) != 0) { @@ -254,12 +248,55 @@ static void sc_mkdir_and_mount_and_glob_files(const char *rootfs_dir, #ifdef NVIDIA_BIARCH +// Expose host NVIDIA drivers to the snap on biarch systems. +// +// Order is absolutely imperative here. We'll attempt to find the +// primary files for the architecture in the main directory, and end +// up copying any files across. However it is possible we're using a +// GLVND enabled host, in which case we copied libGL* to the farm. +// The next step in the list is to look within the private nvidia +// directory, exposed using ld.so.conf tricks within the host OS. +// In some distros (i.e. Solus) only the private libGL/libEGL files +// may be found here, and they'll clobber the existing GLVND files from +// the previous run. +// In other distros (like Fedora) all NVIDIA libraries are contained +// within the private directory, so we clobber the GLVND files and we +// also grab all the private NVIDIA libraries. +// +// In non GLVND cases we just copy across the exposed libGLs and NVIDIA +// libraries from wherever we find, and clobbering is also harmless. static void sc_mount_nvidia_driver_biarch(const char *rootfs_dir) { - sc_mkdir_and_mount_and_glob_files(rootfs_dir, SC_LIBGL_DIR, + + const char *native_sources[] = { + NATIVE_LIBDIR, + NATIVE_LIBDIR "/nvidia*", + }; + const size_t native_sources_len = + sizeof native_sources / sizeof *native_sources; + +#if UINTPTR_MAX == 0xffffffffffffffff + // Alternative 32-bit support + const char *lib32_sources[] = { + LIB32_DIR, + LIB32_DIR "/nvidia*", + }; + const size_t lib32_sources_len = + sizeof lib32_sources / sizeof *lib32_sources; +#endif + + // Primary arch + sc_mkdir_and_mount_and_glob_files(rootfs_dir, + native_sources, native_sources_len, + SC_LIBGL_DIR, nvidia_globs, + nvidia_globs_len); + +#if UINTPTR_MAX == 0xffffffffffffffff + // Alternative 32-bit support + sc_mkdir_and_mount_and_glob_files(rootfs_dir, lib32_sources, + lib32_sources_len, SC_LIBGL32_DIR, nvidia_globs, nvidia_globs_len); - sc_mkdir_and_mount_and_glob_files(rootfs_dir, SC_LIBGL32_DIR, - nvidia_globs32, nvidia_globs32_len); +#endif } #endif // ifdef NVIDIA_BIARCH @@ -329,6 +366,10 @@ static void sc_mkdir_and_mount_and_bind(const char *rootfs_dir, if (res != 0 && errno != EEXIST) { die("cannot create directory %s", dst); } + if (res == 0 && (chown(dst, 0, 0) < 0)) { + // Adjust the ownership only if we created the directory. + die("cannot change ownership of %s", dst); + } // Bind mount the binary nvidia driver into $tgt_dir (i.e. /var/lib/snapd/lib/gl). debug("bind mounting nvidia driver %s -> %s", src, dst); if (mount(src, dst, NULL, MS_BIND, NULL) != 0) { @@ -347,12 +388,34 @@ static void sc_mount_nvidia_driver_multiarch(const char *rootfs_dir) #endif // ifdef NVIDIA_MULTIARCH +static void sc_mount_vulkan(const char *rootfs_dir) +{ + const char *vulkan_sources[] = { + SC_VULKAN_SOURCE_DIR, + }; + const size_t vulkan_sources_len = + sizeof vulkan_sources / sizeof *vulkan_sources; + + sc_mkdir_and_mount_and_glob_files(rootfs_dir, vulkan_sources, + vulkan_sources_len, SC_VULKAN_DIR, + vulkan_globs, vulkan_globs_len); +} + void sc_mount_nvidia_driver(const char *rootfs_dir) { /* If NVIDIA module isn't loaded, don't attempt to mount the drivers */ if (access(SC_NVIDIA_DRIVER_VERSION_FILE, F_OK) != 0) { return; } + + int res = mkdir(SC_LIB, 0755); + if (res != 0 && errno != EEXIST) { + die("cannot create " SC_LIB); + } + if (res == 0 && (chown(SC_LIB, 0, 0) < 0)) { + // Adjust the ownership only if we created the directory. + die("cannot change ownership of " SC_LIB); + } #ifdef NVIDIA_MULTIARCH sc_mount_nvidia_driver_multiarch(rootfs_dir); #endif // ifdef NVIDIA_MULTIARCH @@ -361,6 +424,5 @@ void sc_mount_nvidia_driver(const char *rootfs_dir) #endif // ifdef NVIDIA_BIARCH // Common for both driver mechanisms - sc_mkdir_and_mount_and_glob_files(rootfs_dir, SC_VULKAN_DIR, - vulkan_globs, vulkan_globs_len); + sc_mount_vulkan(rootfs_dir); } diff --git a/cmd/snap-confine/mount-support.c b/cmd/snap-confine/mount-support.c index e78b9a058e..b9affb2c71 100644 --- a/cmd/snap-confine/mount-support.c +++ b/cmd/snap-confine/mount-support.c @@ -535,7 +535,7 @@ static bool __attribute__ ((used)) return false; } -static int sc_open_snap_update_ns(void) +int sc_open_snap_update_ns(void) { // +1 is for the case where the link is exactly PATH_MAX long but we also // want to store the terminating '\0'. The readlink system call doesn't add @@ -563,7 +563,8 @@ static int sc_open_snap_update_ns(void) return fd; } -void sc_populate_mount_ns(const char *base_snap_name, const char *snap_name) +void sc_populate_mount_ns(int snap_update_ns_fd, const char *base_snap_name, + const char *snap_name) { // Get the current working directory before we start fiddling with // mounts and possibly pivot_root. At the end of the whole process, we @@ -573,10 +574,6 @@ void sc_populate_mount_ns(const char *base_snap_name, const char *snap_name) if (vanilla_cwd == NULL) { die("cannot get the current working directory"); } - // Find and open snap-update-ns from the same path as where we - // (snap-confine) were called. - int snap_update_ns_fd SC_CLEANUP(sc_cleanup_close) = -1; - snap_update_ns_fd = sc_open_snap_update_ns(); bool on_classic_distro = is_running_on_classic_distribution(); // on classic or with alternative base snaps we need to setup @@ -707,6 +704,11 @@ void sc_ensure_shared_snap_mount(void) { if (!is_mounted_with_shared_option("/") && !is_mounted_with_shared_option(SNAP_MOUNT_DIR)) { + // TODO: We could be more aggressive and refuse to function but since + // we have no data on actual environments that happen to limp along in + // this configuration let's not do that yet. This code should be + // removed once we have a measurement and feedback mechanism that lets + // us decide based on measurable data. sc_do_mount(SNAP_MOUNT_DIR, SNAP_MOUNT_DIR, "none", MS_BIND | MS_REC, 0); sc_do_mount("none", SNAP_MOUNT_DIR, NULL, MS_SHARED | MS_REC, diff --git a/cmd/snap-confine/mount-support.h b/cmd/snap-confine/mount-support.h index ebeee838b5..bed0462032 100644 --- a/cmd/snap-confine/mount-support.h +++ b/cmd/snap-confine/mount-support.h @@ -19,6 +19,15 @@ #define SNAP_MOUNT_SUPPORT_H /** + * Return a file descriptor referencing the snap-update-ns utility + * + * By calling this prior to changing the mount namespace, it is + * possible to execute the utility even if a different version is now + * mounted at the expected location. + **/ +int sc_open_snap_update_ns(void); + +/** * Assuming a new mountspace, populate it accordingly. * * This function performs many internal tasks: @@ -31,7 +40,8 @@ * The function will also try to preserve the current working directory but if * this is impossible it will chdir to SC_VOID_DIR. **/ -void sc_populate_mount_ns(const char *base_snap_name, const char *snap_name); +void sc_populate_mount_ns(int snap_update_ns_fd, const char *base_snap_name, + const char *snap_name); /** * Ensure that / or /snap is mounted with the SHARED option. diff --git a/cmd/snap-confine/ns-support.c b/cmd/snap-confine/ns-support.c index e6d0587a72..be6b195984 100644 --- a/cmd/snap-confine/ns-support.c +++ b/cmd/snap-confine/ns-support.c @@ -38,11 +38,13 @@ #include <sys/wait.h> #include <unistd.h> +#include "../libsnap-confine-private/cgroup-freezer-support.h" +#include "../libsnap-confine-private/classic.h" #include "../libsnap-confine-private/cleanup-funcs.h" +#include "../libsnap-confine-private/locking.h" #include "../libsnap-confine-private/mountinfo.h" #include "../libsnap-confine-private/string-utils.h" #include "../libsnap-confine-private/utils.h" -#include "../libsnap-confine-private/locking.h" #include "user-support.h" /*! @@ -306,10 +308,167 @@ static bool should_discard_current_ns(dev_t base_snap_dev) die("cannot find mount entry of the root filesystem inside snap namespace"); } -void sc_create_or_join_ns_group(struct sc_ns_group *group, - struct sc_apparmor *apparmor, - const char *base_snap_name, - const char *snap_name) +enum sc_discard_vote { + SC_DISCARD_NO = 1, + SC_DISCARD_YES = 2, +}; + +// The namespace may be stale. To check this we must actually switch into it +// but then we use up our setns call (the kernel misbehaves if we setns twice). +// To work around this we'll fork a child and use it to probe. The child will +// inspect the namespace and send information back via eventfd and then exit +// unconditionally. +static int sc_inspect_and_maybe_discard_stale_ns(int mnt_fd, + const char *snap_name, + const char *base_snap_name) +{ + char base_snap_rev[PATH_MAX] = { 0 }; + char fname[PATH_MAX] = { 0 }; + char mnt_fname[PATH_MAX] = { 0 }; + dev_t base_snap_dev; + int event_fd SC_CLEANUP(sc_cleanup_close) = -1; + + // Read the revision of the base snap by looking at the current symlink. + sc_must_snprintf(fname, sizeof fname, "%s/%s/current", + SNAP_MOUNT_DIR, base_snap_name); + if (readlink(fname, base_snap_rev, sizeof base_snap_rev) < 0) { + die("cannot read revision of base snap %s", fname); + } + if (base_snap_rev[sizeof base_snap_rev - 1] != '\0') { + die("cannot use symbolic link %s - value is too long", fname); + } + // Find the device that is backing the current revision of the base snap. + base_snap_dev = find_base_snap_device(base_snap_name, base_snap_rev); + + // Check if we are running on classic. Do it here because we will always + // (seemingly) run on a core system once we are inside a mount namespace. + bool is_classic = is_running_on_classic_distribution(); + + // Store the PID of this process. This is done instead of calls to + // getppid() below because then we can reliably track the PID of the + // parent even if the child process is re-parented. + pid_t parent = getpid(); + + // Create an eventfd for the communication with the child. + event_fd = eventfd(0, EFD_CLOEXEC); + if (event_fd < 0) { + die("cannot create eventfd for communication with inspection process"); + } + // Fork a child, it will do the inspection for us. + pid_t child = fork(); + if (child < 0) { + die("cannot fork support process for namespace inspection"); + } + + if (child == 0) { + // This is the child process which will inspect the mount namespace. + // + // Configure the child to die as soon as the parent dies. In an odd + // case where the parent is killed then we don't want to complete our + // task or wait for anything. + if (prctl(PR_SET_PDEATHSIG, SIGINT, 0, 0, 0) < 0) { + die("cannot set parent process death notification signal to SIGINT"); + } + // Check that parent process is still alive. If this is the case then + // we can *almost* reliably rely on the PR_SET_PDEATHSIG signal to wake + // us up from eventfd_read() below. In the rare case that the PID + // numbers overflow and the now-dead parent PID is recycled we will + // still hang forever on the read from eventfd below. + debug("ensuring that parent process is still alive"); + if (kill(parent, 0) < 0) { + switch (errno) { + case ESRCH: + debug("parent process has already terminated"); + abort(); + default: + die("cannot ensure that parent process is still alive"); + break; + } + } + + debug("joining the namespace that we are about to probe"); + // Move to the mount namespace of the snap we're trying to inspect. + if (setns(mnt_fd, CLONE_NEWNS) < 0) { + die("cannot join the mount namespace in order to inspect it"); + } + // Check if the namespace needs to be discarded. + // + // TODO: enable this for core distributions. This is complex because on + // core the rootfs is mounted in initrd and is _not_ changed (no + // pivot_root) and the base snap is again mounted (2nd time) by + // systemd. This makes us end up in a situation where the outer base + // snap will never match the rootfs inside the mount namespace. + bool should_discard = + is_classic ? should_discard_current_ns(base_snap_dev) : + false; + + // Send this back to the parent: 2 - discard, 1 - keep. + // Note that we cannot just use 0 and 1 because of the semantics of eventfd(2). + debug + ("sending information about the state of the mount namespace (%s)", + should_discard ? "discard" : "keep"); + if (eventfd_write + (event_fd, + should_discard ? SC_DISCARD_YES : SC_DISCARD_NO) < 0) { + die("cannot send information about the state of the mount namespace"); + } + // Exit, we're done. + debug + ("support process for mount namespace inspection is about to finish"); + exit(0); + } + // This is back in the parent process. + // + // Enable a sanity timeout in case the read blocks for unbound amount of + // time. This will ensure we will not hang around while holding the lock. + // Next, read the value written by the child process. + sc_enable_sanity_timeout(); + eventfd_t value = 0; + debug("receiving information about the state of the mount namespace"); + if (eventfd_read(event_fd, &value) < 0) { + die("cannot receive information about the state of the mount namespace"); + } + sc_disable_sanity_timeout(); + + // Wait for the child process to exit and collect its exit status. + errno = 0; + int status = 0; + if (waitpid(child, &status, 0) < 0) { + die("cannot wait for the support process for mount namespace inspection"); + } + if (!WIFEXITED(status) || WEXITSTATUS(status) != 0) { + die("support process for mount namespace inspection exited abnormally"); + } + // If the namespace is up-to-date then we are done. + if (value == SC_DISCARD_NO) { + debug("the mount namespace is up-to-date and can be reused"); + return 0; + } + // The namespace is stale, let's check if we can discard it. + debug("the mount namespace is stale and should be discarded"); + if (sc_cgroup_freezer_occupied(snap_name)) { + // Some processes are still using the namespace so we cannot discard it + // as that would fracture the view that the set of processes inside + // have on what is mounted. + return 0; + } + // The namespace is both stale and empty. We can discard it now. + debug("discarding stale and empty mount namespace"); + sc_must_snprintf(mnt_fname, sizeof mnt_fname, + "%s/%s%s", sc_ns_dir, snap_name, SC_NS_MNT_FILE); + + // Use MNT_DETACH as otherwise we get EBUSY. + if (umount2(mnt_fname, MNT_DETACH | UMOUNT_NOFOLLOW) < 0) { + die("cannot umount stale mount namespace %s", mnt_fname); + } + debug("stale mount namespace discarded"); + return EAGAIN; +} + +int sc_create_or_join_ns_group(struct sc_ns_group *group, + struct sc_apparmor *apparmor, + const char *base_snap_name, + const char *snap_name) { // Open the mount namespace file. char mnt_fname[PATH_MAX] = { 0 }; @@ -351,23 +510,12 @@ void sc_create_or_join_ns_group(struct sc_ns_group *group, #endif if (ns_statfs_buf.f_type == NSFS_MAGIC || ns_statfs_buf.f_type == PROC_SUPER_MAGIC) { - char fname[PATH_MAX] = { 0 }; - char base_snap_rev[PATH_MAX] = { 0 }; - - // Read the revision of the base snap. - sc_must_snprintf(fname, sizeof fname, "%s/%s/current", - SNAP_MOUNT_DIR, base_snap_name); - if (readlink(fname, base_snap_rev, sizeof base_snap_rev) < 0) { - die("cannot read symlink %s", fname); - } - if (base_snap_rev[sizeof base_snap_rev - 1] != '\0') { - die("cannot use symbolic link %s - value is too long", - fname); - } - - dev_t base_snap_dev = - find_base_snap_device(base_snap_name, base_snap_rev); + // Inspect and perhaps discard the preserved mount namespace. + if (sc_inspect_and_maybe_discard_stale_ns + (mnt_fd, snap_name, base_snap_name) == EAGAIN) { + return EAGAIN; + } // Remember the vanilla working directory so that we may attempt to restore it later. char *vanilla_cwd SC_CLEANUP(sc_cleanup_string) = NULL; vanilla_cwd = get_current_dir_name(); @@ -385,13 +533,6 @@ void sc_create_or_join_ns_group(struct sc_ns_group *group, ("successfully re-associated the mount namespace with the namespace group %s", group->name); - bool should_discard_ns = - should_discard_current_ns(base_snap_dev); - - if (should_discard_ns) { - debug("discarding obsolete base filesystem namespace"); - debug("(not yet implemented)"); - } // Try to re-locate back to vanilla working directory. This can fail // because that directory is no longer present. if (chdir(vanilla_cwd) != 0) { @@ -404,7 +545,7 @@ void sc_create_or_join_ns_group(struct sc_ns_group *group, } debug("successfully moved to %s", SC_VOID_DIR); } - return; + return 0; } debug("initializing new namespace group %s", group->name); // Create a new namespace and ask the caller to populate it. @@ -501,9 +642,10 @@ void sc_create_or_join_ns_group(struct sc_ns_group *group, } group->should_populate = true; } + return 0; } -bool sc_should_populate_ns_group(struct sc_ns_group *group) +bool sc_should_populate_ns_group(struct sc_ns_group * group) { return group->should_populate; } diff --git a/cmd/snap-confine/ns-support.h b/cmd/snap-confine/ns-support.h index bf2c8dcf7c..492399a13b 100644 --- a/cmd/snap-confine/ns-support.h +++ b/cmd/snap-confine/ns-support.h @@ -105,12 +105,13 @@ void sc_close_ns_group(struct sc_ns_group *group); * the parent process unshares the mount namespace and sets a flag so that * sc_should_populate_ns_group() returns true. * - * @returns true if the mount namespace needs to be populated + * @returns 0 on success and EAGAIN if the namespace was stale and needs + * to be re-made. **/ -void sc_create_or_join_ns_group(struct sc_ns_group *group, - struct sc_apparmor *apparmor, - const char *base_snap_name, - const char *snap_name); +int sc_create_or_join_ns_group(struct sc_ns_group *group, + struct sc_apparmor *apparmor, + const char *base_snap_name, + const char *snap_name); /** * Check if the namespace needs to be populated. diff --git a/cmd/snap-confine/snap-confine.apparmor.in b/cmd/snap-confine/snap-confine.apparmor.in index 7f7e83a881..eb099a655d 100644 --- a/cmd/snap-confine/snap-confine.apparmor.in +++ b/cmd/snap-confine/snap-confine.apparmor.in @@ -55,15 +55,17 @@ # cgroup: freezer # Allow creating per-snap cgroup freezers and adding snap command (task) # invocations to the freezer. This allows for reliably enumerating all - # running tasks for the snap. + # running tasks for the snap. In addition, allow enumerating processes in + # the cgroup to determine if it is occupied. /sys/fs/cgroup/freezer/ r, /sys/fs/cgroup/freezer/snap.*/ w, /sys/fs/cgroup/freezer/snap.*/tasks w, + /sys/fs/cgroup/freezer/snap.*/cgroup.procs r, # querying udev /etc/udev/udev.conf r, /sys/**/uevent r, - /lib/udev/snappy-app-dev ixr, # drop + /usr/lib/snapd/snap-device-helper ixr, # drop /run/udev/** rw, /{,usr/}bin/tr ixr, /usr/lib/locale/** r, @@ -303,6 +305,12 @@ # Allow snap-confine to read snap contexts /var/lib/snapd/context/snap.* r, + # Allow snap-confine to unmount stale mount namespaces. + umount /run/snapd/ns/*.mnt, + # Required to correctly unmount bound mount namespace. + # See LP: #1735459 for details. + umount /, + # Support for the quirk system /var/ r, /var/lib/ r, @@ -351,7 +359,7 @@ capability sys_admin, signal (send, receive) set=(abrt) peer=@LIBEXECDIR@/snap-confine, signal (send) set=(int) peer=@LIBEXECDIR@/snap-confine//mount-namespace-capture-helper, - signal (send, receive) set=(alrm, exists) peer=@LIBEXECDIR@/snap-confine, + signal (send, receive) set=(int, alrm, exists) peer=@LIBEXECDIR@/snap-confine, signal (receive) set=(exists) peer=@LIBEXECDIR@/snap-confine//mount-namespace-capture-helper, # workaround for linux 4.13/upstream, see @@ -504,6 +512,8 @@ # Needed to perform mount/unmounts. capability sys_admin, + # Needed for mimic construction. + capability chown, # Allow freezing and thawing the per-snap cgroup freezers /sys/fs/cgroup/freezer/snap.*/freezer.state rw, @@ -542,22 +552,36 @@ /tmp/.snap/{,**} rw, # Allow mounting/unmounting any part of $SNAP over to a temporary place # in /tmp/.snap/ during the preparation of a writable mimic. - mount options=(bind, rw) /snap/*/** -> /tmp/.snap/**, - # Allow mounting tmpfs over the original $SNAP/** directory. - mount fstype=tmpfs options=(rw) tmpfs -> /snap/*/**, + # FIXME: update this with per-snap snap-update-ns profiles + mount options=(bind, rw) /** -> /tmp/.snap/**, + # Allow mounting tmpfs over the original read-only directory. + # FIXME: update this with per-snap snap-update-ns profiles + mount fstype=tmpfs options=(rw) tmpfs -> /**, # Allow bind mounting anything from the temporary place in /tmp/.snap/ # back to $SNAP/** (to re-construct the data that was there before). - mount options=(bind, rw) /tmp/.snap/** -> /snap/*/**, + # FIXME: update this with per-snap snap-update-ns profiles + mount options=(bind, rw) /tmp/.snap/** -> /**, # Allow unmounting the temporary directory in /tmp once it is no longer # necessary. umount /tmp/.snap/**, - # Allow creating missing directories under $SNAP once the writable - # mimic is ready. + # Allow creating missing directories anywhere under the root directory + # (but not in the root directory itself) where they need to be created + # as a mount point for layouts or for content sharing. This is a + # superset of other cases so they are removed + # FIXME: update this with per-snap snap-update-ns profiles / r, - /snap/ r, - /snap/{,**} r, - /snap/*/** w, + /** r, + /*/** w, + + # Allow layouts to bind mount *from* $SNAP, $SNAP_DATA and $SNAP_COMMON + # *to* anywhere under the root directory. This is safe because the + # mounts happen inside an isolated mount namespace (but see below). + mount options=(bind) /snap/*/** -> /*/**, + mount options=(bind) /var/snap/*/** -> /*/**, + # As an exception, don't allow bind mounts to /media which has special + # sharing and propagates mount events outside of the snap namespace. + audit deny mount -> /media, # Allow the content interface to bind fonts from the host filesystem mount options=(ro bind) /var/lib/snapd/hostfs/usr/share/fonts/ -> /snap/*/*/**, @@ -567,11 +591,7 @@ mount options=(ro bind) /var/lib/snapd/hostfs/var/cache/fontconfig/ -> /var/cache/fontconfig/, # Allow unmounts matching possible mounts listed above. - umount /snap/*/*/**, - umount /var/snap/*/**, - umount /usr/share/fonts, - umount /usr/local/share/fonts, - umount /var/cache/fontconfig, + umount /*/**, # But we don't want anyone to touch /snap/bin audit deny mount /snap/bin/** -> /**, diff --git a/cmd/snap-confine/snap-confine.c b/cmd/snap-confine/snap-confine.c index 7b8693a77b..a15349ad12 100644 --- a/cmd/snap-confine/snap-confine.c +++ b/cmd/snap-confine/snap-confine.c @@ -18,12 +18,14 @@ #include "config.h" #endif +#include <errno.h> #include <glob.h> +#include <signal.h> #include <stdio.h> #include <stdlib.h> #include <string.h> -#include <sys/types.h> #include <sys/stat.h> +#include <sys/types.h> #include <unistd.h> #include "../libsnap-confine-private/cgroup-freezer-support.h" @@ -217,15 +219,33 @@ int main(int argc, char **argv) sc_initialize_ns_groups(); sc_unlock_global(global_lock_fd); + // Find and open snap-update-ns from the same + // path as where we (snap-confine) were + // called. + int snap_update_ns_fd SC_CLEANUP(sc_cleanup_close) = -1; + snap_update_ns_fd = sc_open_snap_update_ns(); + // Do per-snap initialization. int snap_lock_fd = sc_lock(snap_name); debug("initializing mount namespace: %s", snap_name); struct sc_ns_group *group = NULL; group = sc_open_ns_group(snap_name, 0); - sc_create_or_join_ns_group(group, &apparmor, - base_snap_name, snap_name); + if (sc_create_or_join_ns_group(group, &apparmor, + base_snap_name, + snap_name) == EAGAIN) { + // If the namespace was stale and was discarded we just need to + // try again. Since this is done with the per-snap lock held + // there are no races here. + if (sc_create_or_join_ns_group(group, &apparmor, + base_snap_name, + snap_name) == + EAGAIN) { + die("unexpectedly the namespace needs to be discarded again"); + } + } if (sc_should_populate_ns_group(group)) { - sc_populate_mount_ns(base_snap_name, snap_name); + sc_populate_mount_ns(snap_update_ns_fd, + base_snap_name, snap_name); sc_preserve_populated_ns_group(group); } sc_close_ns_group(group); diff --git a/cmd/snap-confine/snappy-app-dev b/cmd/snap-confine/snap-device-helper index 6d75549fb4..bc779179f7 100755 --- a/cmd/snap-confine/snappy-app-dev +++ b/cmd/snap-confine/snap-device-helper @@ -1,8 +1,8 @@ #!/bin/sh -# udev callout (should live in /lib/udev) to allow a snap to access a device node +# udev callout to allow a snap to access a device node set -e # debugging -#exec >>/tmp/snappy-app-dev.log +#exec >>/tmp/snap-device-helper.log #exec 2>&1 #set -x # end debugging diff --git a/cmd/snap-confine/spread-tests/regression/lp-1599608/task.yaml b/cmd/snap-confine/spread-tests/regression/lp-1599608/task.yaml index c83a15feef..564589275e 100644 --- a/cmd/snap-confine/spread-tests/regression/lp-1599608/task.yaml +++ b/cmd/snap-confine/spread-tests/regression/lp-1599608/task.yaml @@ -3,10 +3,10 @@ summary: Check that execle doesn't regress systems: [-debian-8] details: | The setup for this test is unorthodox because by the time the cgroup code is - executed, the mounts are in place and /lib/udev/snappy-app-dev from the core + executed, the mounts are in place and /lib/udev/snap-device-helper from the core snap is used. Unfortunately, simple bind mounts over /snap/ubuntu-core/current/lib/udev don't work and the core snap must be - unpacked, lib/udev/snappy-app-dev modified to be tested, repacked and mounted. + unpacked, lib/udev/snap-device-helper modified to be tested, repacked and mounted. We unmount the core snap and move it aside to avoid both the original and the updated core snap from being mounted on the same mount point, which confuses the kernel. @@ -23,10 +23,10 @@ prepare: | echo "Unmount original core snap" umount $(ls -1d /snap/ubuntu-core/* | grep -v current | tail -1) mv $(ls -1 /var/lib/snapd/snaps/ubuntu-core_*.snap | tail -1) $(ls -1 /var/lib/snapd/snaps/ubuntu-core_*.snap | tail -1).orig - echo "Create modified core snap for snappy-app-dev" + echo "Create modified core snap for snap-device-helper" unsquashfs $(ls -1 /var/lib/snapd/snaps/ubuntu-core_*.snap.orig | tail -1) - echo 'echo PATH=$PATH > /run/udev/spread-test.out' >> ./squashfs-root/lib/udev/snappy-app-dev - echo 'echo TESTVAR=$TESTVAR >> /run/udev/spread-test.out' >> ./squashfs-root/lib/udev/snappy-app-dev + echo 'echo PATH=$PATH > /run/udev/spread-test.out' >> ./squashfs-root/lib/udev/snap-device-helper + echo 'echo TESTVAR=$TESTVAR >> /run/udev/spread-test.out' >> ./squashfs-root/lib/udev/snap-device-helper mksquashfs ./squashfs-root $(ls -1 /var/lib/snapd/snaps/ubuntu-core_*.snap.orig | tail -1 | sed 's/.orig//') -comp xz -no-fragments if [ ! -e $(ls -1 /var/lib/snapd/snaps/ubuntu-core_*.snap | tail -1) ]; then exit 1; fi echo "Mount modified core snap" diff --git a/cmd/snap-confine/udev-support.c b/cmd/snap-confine/udev-support.c index e352b19d38..632f6b6c2d 100644 --- a/cmd/snap-confine/udev-support.c +++ b/cmd/snap-confine/udev-support.c @@ -61,9 +61,9 @@ _run_snappy_app_dev_add_majmin(struct snappy_udev *udev_s, sc_must_snprintf(buf, sizeof(buf), "%u:%u", major, minor); } - debug("running snappy-app-dev add %s %s %s", udev_s->tagname, + debug("running snap-device-helper add %s %s %s", udev_s->tagname, path, buf); - execle("/lib/udev/snappy-app-dev", "/lib/udev/snappy-app-dev", + execle("/usr/lib/snapd/snap-device-helper", "/usr/lib/snapd/snap-device-helper", "add", udev_s->tagname, path, buf, NULL, env); die("execl failed"); } @@ -217,7 +217,7 @@ void setup_devices_cgroup(const char *security_tag, struct snappy_udev *udev_s) // https://github.com/torvalds/linux/blob/master/Documentation/admin-guide/devices.txt for (unsigned pty_major = 136; pty_major <= 143; pty_major++) { // '/dev/pts/slaves' is only used for debugging and by - // /lib/udev/snappy-app-dev to determine if it is a block + // /usr/lib/snapd/snap-device-helper to determine if it is a block // device, so just use something to indicate what the // addition is for _run_snappy_app_dev_add_majmin(udev_s, "/dev/pts/slaves", diff --git a/cmd/snap-exec/main.go b/cmd/snap-exec/main.go
|