Merge remote-tracking branch 'upstream/master' into HEAD

fix conflicts

boot/boot.go

@@ -23,6 +23,7 @@ import (
 	"errors"
 	"fmt"
 
+	"github.com/snapcore/snapd/bootloader"
 	"github.com/snapcore/snapd/snap"
 )
 
@@ -96,12 +97,21 @@ func Participant(s snap.PlaceInfo, t snap.Type, dev Device) BootParticipant {
 	return trivial{}
 }
 
+// bootloaderOptionsForDeviceKernel returns a set of bootloader options that
+// enable correct kernel extraction and removal for given device
+func bootloaderOptionsForDeviceKernel(dev Device) *bootloader.Options {
+	return &bootloader.Options{
+		// unified extractable kernel if in uc20 mode
+		ExtractedRunKernelImage: dev.HasModeenv(),
+	}
+}
+
 // Kernel checks that the given arguments refer to a kernel snap
 // that participates in the boot process, and returns the associated
 // BootKernel, or a trivial implementation otherwise.
 func Kernel(s snap.PlaceInfo, t snap.Type, dev Device) BootKernel {
 	if t == snap.TypeKernel && applicable(s, t, dev) {
-		return &coreKernel{s: s}
+		return &coreKernel{s: s, bopts: bootloaderOptionsForDeviceKernel(dev)}
 	}
 	return trivial{}
 }

boot/boot_test.go

@@ -32,6 +32,7 @@ import (
 	"github.com/snapcore/snapd/bootloader/bootloadertest"
 	"github.com/snapcore/snapd/dirs"
 	"github.com/snapcore/snapd/snap"
+	"github.com/snapcore/snapd/snap/snaptest"
 	"github.com/snapcore/snapd/testutil"
 )
 
@@ -303,7 +304,6 @@ func (s *bootSetSuite) TestParticipantBaseWithModel(c *C) {
 func (s *bootSetSuite) TestKernelWithModel(c *C) {
 	info := &snap.Info{}
 	info.RealName = "kernel"
-	expected := boot.NewCoreKernel(info)
 
 	type tableT struct {
 		model string
@@ -319,7 +319,7 @@ func (s *bootSetSuite) TestKernelWithModel(c *C) {
 		}, {
 			model: "kernel",
 			nop:   false,
-			krn:   expected,
+			krn:   boot.NewCoreKernel(info, boottest.MockDevice("kernel")),
 		}, {
 			model: "",
 			nop:   true,
@@ -339,6 +339,30 @@ func (s *bootSetSuite) TestKernelWithModel(c *C) {
 	}
 }
 
+func (s *bootSetSuite) TestCoreKernel20(c *C) {
+	coreDev := boottest.MockUC20Device("pc-kernel")
+	c.Assert(coreDev.HasModeenv(), Equals, true)
+
+	kernel, err := snap.ParsePlaceInfoFromSnapFileName("pc-kernel_2.snap")
+	c.Assert(err, IsNil)
+
+	// get the boot kernel from our kernel snap
+	bootKern := boot.Kernel(kernel, snap.TypeKernel, coreDev)
+	// can't use FitsTypeOf with coreKernel here, cause that causes an import
+	// loop as boottest imports boot and coreKernel is unexported
+	c.Assert(bootKern.IsTrivial(), Equals, false)
+
+	// extract the kernel assets from the coreKernel
+	// the container here doesn't really matter since it's just being passed
+	// to the mock bootloader method anyways
+	kernelContainer := snaptest.MockContainer(c, nil)
+	err = bootKern.ExtractKernelAssets(kernelContainer)
+	c.Assert(err, IsNil)
+
+	// make sure that the bootloader was told to extract some assets
+	c.Assert(s.bootloader.ExtractKernelAssetsCalls, DeepEquals, []snap.PlaceInfo{kernel})
+}
+
 func (s *bootSetSuite) TestMarkBootSuccessfulAllSnap(c *C) {
 	coreDev := boottest.MockDevice("some-snap")
 

boot/export_test.go

@@ -31,8 +31,8 @@ func NewCoreBootParticipant(s snap.PlaceInfo, t snap.Type, dev Device) *coreBoot
 	return &coreBootParticipant{s: s, bs: bs}
 }
 
-func NewCoreKernel(s snap.PlaceInfo) *coreKernel {
-	return &coreKernel{s}
+func NewCoreKernel(s snap.PlaceInfo, d Device) *coreKernel {
+	return &coreKernel{s, bootloaderOptionsForDeviceKernel(d)}
 }
 
 type Trivial = trivial

boot/kernel_os.go

@@ -53,7 +53,8 @@ func (bp *coreBootParticipant) SetNextBoot() (rebootRequired bool, err error) {
 }
 
 type coreKernel struct {
-	s snap.PlaceInfo
+	s     snap.PlaceInfo
+	bopts *bootloader.Options
 }
 
 // ensure coreKernel is a Kernel
@@ -63,7 +64,7 @@ func (*coreKernel) IsTrivial() bool { return false }
 
 func (k *coreKernel) RemoveKernelAssets() error {
 	// XXX: shouldn't we check the snap type?
-	bootloader, err := bootloader.Find("", nil)
+	bootloader, err := bootloader.Find("", k.bopts)
 	if err != nil {
 		return fmt.Errorf("cannot remove kernel assets: %s", err)
 	}
@@ -73,11 +74,10 @@ func (k *coreKernel) RemoveKernelAssets() error {
 }
 
 func (k *coreKernel) ExtractKernelAssets(snapf snap.Container) error {
-	bootloader, err := bootloader.Find("", nil)
+	bootloader, err := bootloader.Find("", k.bopts)
 	if err != nil {
 		return fmt.Errorf("cannot extract kernel assets: %s", err)
 	}
-
 	// ask bootloader to extract the kernel assets if needed
 	return bootloader.ExtractKernelAssets(k.s, snapf)
 }

boot/kernel_os_test.go

@@ -63,13 +63,13 @@ func (s *coreBootSetSuite) SetUpTest(c *C) {
 
 func (s *coreBootSetSuite) TestExtractKernelAssetsError(c *C) {
 	bootloader.ForceError(errors.New("brkn"))
-	err := boot.NewCoreKernel(&snap.Info{}).ExtractKernelAssets(nil)
+	err := boot.NewCoreKernel(&snap.Info{}, boottest.MockDevice("")).ExtractKernelAssets(nil)
 	c.Check(err, ErrorMatches, `cannot extract kernel assets: brkn`)
 }
 
 func (s *coreBootSetSuite) TestRemoveKernelAssetsError(c *C) {
 	bootloader.ForceError(errors.New("brkn"))
-	err := boot.NewCoreKernel(&snap.Info{}).RemoveKernelAssets()
+	err := boot.NewCoreKernel(&snap.Info{}, boottest.MockDevice("")).RemoveKernelAssets()
 	c.Check(err, ErrorMatches, `cannot remove kernel assets: brkn`)
 }
 
@@ -265,7 +265,7 @@ func (s *ubootBootSetSuite) TestExtractKernelAssetsAndRemoveOnUboot(c *C) {
 	info, err := snap.ReadInfoFromSnapFile(snapf, si)
 	c.Assert(err, IsNil)
 
-	bp := boot.NewCoreKernel(info)
+	bp := boot.NewCoreKernel(info, boottest.MockDevice(""))
 	err = bp.ExtractKernelAssets(snapf)
 	c.Assert(err, IsNil)
 
@@ -347,7 +347,7 @@ func (s *grubBootSetSuite) TestExtractKernelAssetsNoUnpacksKernelForGrub(c *C) {
 	info, err := snap.ReadInfoFromSnapFile(snapf, si)
 	c.Assert(err, IsNil)
 
-	bp := boot.NewCoreKernel(info)
+	bp := boot.NewCoreKernel(info, boottest.MockDevice(""))
 	err = bp.ExtractKernelAssets(snapf)
 	c.Assert(err, IsNil)
 
@@ -378,7 +378,7 @@ func (s *grubBootSetSuite) TestExtractKernelForceWorks(c *C) {
 	info, err := snap.ReadInfoFromSnapFile(snapf, si)
 	c.Assert(err, IsNil)
 
-	bp := boot.NewCoreKernel(info)
+	bp := boot.NewCoreKernel(info, boottest.MockDevice(""))
 	err = bp.ExtractKernelAssets(snapf)
 	c.Assert(err, IsNil)
 

cmd/snap-confine/mount-support-nvidia.c

@@ -121,8 +121,6 @@ static const char *nvidia_globs[] = {
 static const size_t nvidia_globs_len =
     sizeof nvidia_globs / sizeof *nvidia_globs;
 
-#define LIBNVIDIA_GLCORE_SO_PATTERN "libnvidia-glcore.so.%d.%d"
-
 #endif				// defined(NVIDIA_BIARCH) || defined(NVIDIA_MULTIARCH)
 
 // Populate libgl_dir with a symlink farm to files matching glob_list.
@@ -345,55 +343,62 @@ static void sc_mount_nvidia_driver_biarch(const char *rootfs_dir)
 
 #ifdef NVIDIA_MULTIARCH
 
-struct sc_nvidia_driver {
-	int major_version;
-	int minor_version;
-};
+typedef struct {
+	int major;
+	// Driver version format is MAJOR.MINOR[.MICRO] but we only care about the
+	// major version and the full version string. The micro component has been
+	// seen with relevant leading zeros (e.g. "440.48.02").
+	char raw[128]; // The size was picked as "big enough" for version strings.
+} sc_nv_version;
 
-static void sc_probe_nvidia_driver(struct sc_nvidia_driver *driver)
+static void sc_probe_nvidia_driver(sc_nv_version * version)
 {
+	memset(version, 0, sizeof *version);
+
 	FILE *file SC_CLEANUP(sc_cleanup_file) = NULL;
 	debug("opening file describing nvidia driver version");
 	file = fopen(SC_NVIDIA_DRIVER_VERSION_FILE, "rt");
 	if (file == NULL) {
 		if (errno == ENOENT) {
 			debug("nvidia driver version file doesn't exist");
-			driver->major_version = 0;
-			driver->minor_version = 0;
 			return;
 		}
 		die("cannot open file describing nvidia driver version");
 	}
-	// Driver version format is MAJOR.MINOR where both MAJOR and MINOR are
-	// integers. We can use sscanf to parse this data.
-	if (fscanf
-	    (file, "%d.%d", &driver->major_version,
-	     &driver->minor_version) != 2) {
-		die("cannot parse nvidia driver version string");
+	int nread = fread(version->raw, 1, sizeof version->raw - 1, file);
+	if (nread < 0) {
+		die("cannot read nvidia driver version string");
+	}
+	if (nread == sizeof version->raw - 1 && !feof(file)) {
+		die("cannot fit entire nvidia driver version string");
+	}
+	version->raw[nread] = '\0';
+	if (nread > 0 && version->raw[nread - 1] == '\n') {
+		version->raw[nread - 1] = '\0';
+	}
+	if (sscanf(version->raw, "%d.", &version->major) != 1) {
+		die("cannot parse major version from nvidia driver version string");
 	}
-	debug("parsed nvidia driver version: %d.%d", driver->major_version,
-	      driver->minor_version);
 }
 
 static void sc_mkdir_and_mount_and_bind(const char *rootfs_dir,
 					const char *src_dir,
 					const char *tgt_dir)
 {
-	struct sc_nvidia_driver driver;
+	sc_nv_version version;
 
 	// Probe sysfs to get the version of the driver that is currently inserted.
-	sc_probe_nvidia_driver(&driver);
+	sc_probe_nvidia_driver(&version);
 
 	// If there's driver in the kernel then don't mount userspace.
-	if (driver.major_version == 0) {
+	if (version.major == 0) {
 		return;
 	}
 	// Construct the paths for the driver userspace libraries
 	// and for the gl directory.
 	char src[PATH_MAX] = { 0 };
 	char dst[PATH_MAX] = { 0 };
-	sc_must_snprintf(src, sizeof src, "%s-%d", src_dir,
-			 driver.major_version);
+	sc_must_snprintf(src, sizeof src, "%s-%d", src_dir, version.major);
 	sc_must_snprintf(dst, sizeof dst, "%s%s", rootfs_dir, tgt_dir);
 
 	// If there is no userspace driver available then don't try to mount it.
@@ -423,21 +428,24 @@ static int sc_mount_nvidia_is_driver_in_dir(const char *dir)
 {
 	char driver_path[512] = { 0 };
 
-	struct sc_nvidia_driver driver;
+	sc_nv_version version;
 
 	// Probe sysfs to get the version of the driver that is currently inserted.
-	sc_probe_nvidia_driver(&driver);
+	sc_probe_nvidia_driver(&version);
 
 	// If there's no driver then we should not bother ourselves with finding the
 	// matching library
-	if (driver.major_version == 0) {
+	if (version.major == 0) {
 		return 0;
 	}
-	// Probe if a well known library is found in directory dir
+
+	// Probe if a well known library is found in directory dir. We must use the
+	// raw version because it may contain more than just major.minor. In
+	// practice the micro version may have leading zeros that are relevant.
 	sc_must_snprintf(driver_path, sizeof driver_path,
-			 "%s/" LIBNVIDIA_GLCORE_SO_PATTERN, dir,
-			 driver.major_version, driver.minor_version);
+			 "%s/libnvidia-glcore.so.%s", dir, version.raw);
 
+	debug("looking for nvidia canary file %s", driver_path);
 	if (access(driver_path, F_OK) == 0) {
 		debug("nvidia library detected at path %s", driver_path);
 		return 1;

daemon/api_users.go

@@ -244,19 +244,32 @@ func postUsers(c *Command, r *http.Request, user *auth.UserState) Response {
 }
 
 func removeUser(c *Command, username string, opts postUserDeleteData) Response {
+	// TODO: allow to remove user entries by email as well
+
 	// catch silly errors
 	if username == "" {
 		return BadRequest("need a username to remove")
 	}
+	// check the user is known to snapd
+	st := c.d.overlord.State()
+	st.Lock()
+	_, err := auth.UserByUsername(st, username)
+	st.Unlock()
+	if err == auth.ErrInvalidUser {
+		return BadRequest("user %q is not known", username)
+	}
+	if err != nil {
+		return InternalError(err.Error())
+	}
+
 	// first remove the system user
 	if err := osutilDelUser(username, &osutil.DelUserOptions{ExtraUsers: !release.OnClassic}); err != nil {
 		return InternalError(err.Error())
 	}
 
 	// then the UserState
-	st := c.d.overlord.State()
 	st.Lock()
-	err := auth.RemoveUserByName(st, username)
+	err = auth.RemoveUserByUsername(st, username)
 	st.Unlock()
 	// ErrInvalidUser means "not found" in this case
 	if err != nil && err != auth.ErrInvalidUser {

daemon/api_users_test.go

@@ -247,6 +247,12 @@ func (s *userSuite) TestPostUserActionRemoveNoUsername(c *check.C) {
 }
 
 func (s *userSuite) TestPostUserActionRemoveDelUserErr(c *check.C) {
+	st := s.d.overlord.State()
+	st.Lock()
+	_, err := auth.NewUser(st, "some-user", "email@test.com", "macaroon", []string{"discharge"})
+	st.Unlock()
+	c.Check(err, check.IsNil)
+
 	called := 0
 	osutilDelUser = func(username string, opts *osutil.DelUserOptions) error {
 		called++
@@ -283,7 +289,7 @@ func (s *userSuite) TestPostUserActionRemoveStateErr(c *check.C) {
 	rsp := postUsers(usersCmd, req, nil).(*resp)
 	c.Check(rsp.Status, check.Equals, 500)
 	c.Check(rsp.Result.(*errorResult).Message, check.Matches, `internal error: could not unmarshal state entry "auth": .*`)
-	c.Check(called, check.Equals, 1)
+	c.Check(called, check.Equals, 0)
 }
 
 func (s *userSuite) TestPostUserActionRemoveNoUserInState(c *check.C) {
@@ -299,9 +305,8 @@ func (s *userSuite) TestPostUserActionRemoveNoUserInState(c *check.C) {
 	c.Assert(err, check.IsNil)
 
 	rsp := postUsers(usersCmd, req, nil).(*resp)
-	c.Check(rsp.Status, check.Equals, 200)
-	c.Check(rsp.Result, check.IsNil)
-	c.Check(called, check.Equals, 1)
+	c.Check(rsp, check.DeepEquals, BadRequest(`user "some-user" is not known`))
+	c.Check(called, check.Equals, 0)
 }
 
 func (s *userSuite) TestPostUserActionRemove(c *check.C) {
@@ -311,8 +316,20 @@ func (s *userSuite) TestPostUserActionRemove(c *check.C) {
 	st.Unlock()
 	c.Check(err, check.IsNil)
 
-	// everything that happens when the user is not in state still happens
-	s.TestPostUserActionRemoveNoUserInState(c)
+	called := 0
+	osutilDelUser = func(username string, opts *osutil.DelUserOptions) error {
+		called++
+		c.Check(username, check.Equals, "some-user")
+		return nil
+	}
+
+	buf := bytes.NewBufferString(`{"action":"remove","username":"some-user"}`)
+	req, err := http.NewRequest("POST", "/v2/users", buf)
+	c.Assert(err, check.IsNil)
+	rsp := postUsers(usersCmd, req, nil).(*resp)
+	c.Check(rsp.Status, check.Equals, 200)
+	c.Check(rsp.Result, check.IsNil)
+	c.Check(called, check.Equals, 1)
 
 	// and the user is removed from state
 	st.Lock()

dirs/dirs.go

@@ -331,7 +331,7 @@ func SetRootDir(rootdir string) {
 	}
 
 	XdgRuntimeDirBase = filepath.Join(rootdir, "/run/user")
-	XdgRuntimeDirGlob = filepath.Join(rootdir, XdgRuntimeDirBase, "*/")
+	XdgRuntimeDirGlob = filepath.Join(XdgRuntimeDirBase, "*/")
 
 	CompletionHelperInCore = filepath.Join(CoreLibExecDir, "etelpmoc.sh")
 	CompletersDir = filepath.Join(rootdir, "/usr/share/bash-completion/completions/")