snapd

snapd:release/2.59

  1. Git
  2. lp:snapd
  3. release/2.59
Last commit made on 2023-12-05
Get this branch:
git clone -b release/2.59 https://git.launchpad.net/snapd

Branch merges

Branch information

Name:
release/2.59
Repository:
lp:snapd

Recent commits

8f6adb2... by Valentin David

cmd/snapd-generator: read mountinfo for pid 1

Systemd now runs generators in a sandbox. That means
`/proc/self/mountinfo` does not represent correctly the mounts of the
system.

3a88987... by Michael Vogt

install: lazy unmount() in writeFilesystemContent() if needed (#12939)

* install: lazy unmount() in writeFilesystemContent() if needed

The existing code in writeFilesystemContent() will error when
the filesystem cannot be unmounted. However in practise this
is problematic as the live-system can keep the mount point
busy: https://bugs.launchpad.net/snapd/+bug/2025402

As a pragmatic solution this commit unmounts the filesystem
with the `--lazy` option if a normal unmount does not work.

This is what live-editor is doing:
https://github.com/mwhudson/livefs-editor/pull/26

Alternatively we could do a bunch of retries and wait for
the process that keep the filesystem busy to go away.

* install: log unmount errors (thanks to Alfonso!)

* install: also lazy unmount in MountVolumes()

* install: fix silly typo

* install: improve logging on install failure

c9d4b35... by Michael Vogt

many: stop using `-O no-expr-simplify` in apparmor_parser

We recently ran into a real world profile bug where the option
`-O no-expr-simplify` causes a 10x increase in apparmor_parser
runtime and memory usage [1] that breaks existing customers.

The decision to use `-O no-expr-simplify` was taken in 2014 [2]
and the profiles back then where simpler. This commit will
make some profile generation slower but it will avoid going
into the exponential memory usage when compiled with
`apparmor_parser -O no-expr-simplify`.

[1] https://bugs.launchpad.net/snapd/+bug/2025030
[2] https://bugs.launchpad.net/ubuntu-rtm/+source/apparmor/+bug/1383858

1848b1a... by Oliver Calder

interfaces/builtin: fix custom-device default udev kernel rules (#12833)

* interfaces/builtin: fix custom-device default udev kernel rules

The KERNEL value in udev rules must be the basename of the device path.
For devices for which there is not a matching kernel value specified in
the custom-device `udev-tagging` section, a default udev kernel rule is
generated. Previously, https://github.com/snapcore/snapd/pull/12734
(and prior) generated these default rules by using the complete device
path relative to `/dev/`. However, for device paths which are in
subdirectories of `/dev/`, this means that the kernel values were not
basenames, which violates the udev spec.

This commit changes this behavior to instead generate udev kernel rules
using the basename of each specified device.

Since ambiguity would arise if multiple devices had the same basename,
this change introduces a check to ensure that all the specified devices
have unique basenames.

Additionally, this commit introduces a check to ensure that all
specified kernel values in the `udev-tagging` section are basenames.

It is still the case that each specified kernel value must match one of
the specified devices.

There are currently problems with `vet` where it is claimed that several
of the `[]string` variables in `validateUDevDevicesUniqueBasenames()`
are unused. These variables are used in a several ways, so further
investigation is required as to why this is the case.

Signed-off-by: Oliver Calder <email address hidden>

* prompting/storage: fixed missing variable assignment from append()

Signed-off-by: Oliver Calder <email address hidden>

* prompting/storage: fixed custom device duplicate basename error message

Signed-off-by: Oliver Calder <email address hidden>

* interfaces/builtin: fixed custom-device unit tests introduced by commas in filepaths PR

Signed-off-by: Oliver Calder <email address hidden>

* interfaces/builtin: adjusted custom-device comment for kernel not matching any devices

Signed-off-by: Oliver Calder <email address hidden>

* interfaces/builtin: fixed unit test for when custom-device kernel does not match any device

Signed-off-by: Oliver Calder <email address hidden>

* interfaces/builtin: moved custom-device unique basename check

This change moves the check for whether all specified devices have
unique basenames out of `validateUDevTaggingRule()` (which is called
once for each udev rule) into `BeforePrepareSlot()`, immediately after
the list of device paths is assembled and each path validated. Thus, it
is only called once, before any rule validation begins.

Signed-off-by: Oliver Calder <email address hidden>

---------

Signed-off-by: Oliver Calder <email address hidden>

b6122dc... by Sergio Cazzolato

tests: skip microk8s-smoke test in external devices #12821

The test fails with timeouts caused for lack of resources on those
instances/vms.

The idea is to run this test in google backend where the machines have
4GB of RAM and 2 cores.

a29dc7b... by Sergio Cazzolato

tests: skip snap-quota-services in external devices (#12820)

* Skip snap-quota-services in external devices

As the other quota tests, we need to disable snap-quota-services too for
external devices.

This is the error in the test

+ snap set-quota test-top --memory=400MB test-snapd-stressd
error: cannot create quota group: cannot create quota group "test-top":
cannot
       use memory quota: memory cgroup is disabled on this system

* skip it for real using backends

11e95cc... by Oliver Calder

interfaces/utils: allow commas in filepaths (#12697)

* interfaces/utils: allow commas in filepaths

Some device paths contain commas outside of groups (i.e. {a,b}) or
classes (i.e. [,.:;'"]). For example, `/dev/foo,bar` is a valid device
path which one might with to use with the custom-device interface.

Most filesystems allow commas in filepaths, as does apparmor:
https://gitlab.com/apparmor/apparmor/-/blob/master/parser/parser_regex.c#L340

Previously, createRegex() would throw an error if a comma was used
outside of a group or class. This commit removes that error and instead
treats commas outside of groups and classes as literal commas. The
accompanying tests are also adjusted to reflect this change.

Signed-off-by: Oliver Calder <email address hidden>

* interfaces/utils: added argument to allow commas in filepaths

Rather than allowing any caller of `NewPathPattern()` to successfully
validate paths containing commas, this change adds a boolean argument
which explicitly specifies whether commas should be allowed in the
filepath.

There are some risks involved with allowing commas in filepaths (see
discussion at https://github.com/snapcore/snapd/pull/12697), so it is
desirable to restrict when commas are allowed based on the caller. In
particular, superprivileged interfaces (such as `custom-device` and
`mount-control`) have valid needs for commas in filepaths, and users of
these interfaces are individually verified, so it is safe for them to
use `NewPathPattern()` with commas allowed. Other callers (particularly
unprivileged interfaces) should probably not allow commas.

I was unsure whether `overlord/hookstate/ctlcmd/mount.go` should call
`NewPathPattern()` with commas allowed or not, but since commas had
previously been disallowed and tests continue to pass with
`allowCommas=false`, then I decided to leave it as `false`.

Signed-off-by: Oliver Calder <email address hidden>

* interfaces/{builtin,utils}: added named variables for allowCommas

Also, switched `overlord/hookstate/ctlcmd/mount.go` to allow commas
(previously did not, but this should match what is allowed in
`interfaces/builtin/mount_control.go`.

Signed-off-by: Oliver Calder <email address hidden>

* interfaces/utils: added unit tests for commas in paths

Signed-off-by: Oliver Calder <email address hidden>

* interfaces/utils: remove `QuoteMeta` when adding `","` to path regex

Since `,` is not a regex special character, the `QuoteMeta` call is
unnecessary.

Signed-off-by: Oliver Calder <email address hidden>

* interfaces/utils: renamed TestCommasInRegex to TestCreateRegexWithCommas

Signed-off-by: Oliver Calder <email address hidden>

* many: added unit tests for callers of NewPathPattern with allowCommas=true

Signed-off-by: Oliver Calder <email address hidden>

---------

Signed-off-by: Oliver Calder <email address hidden>
Co-authored-by: Michael Vogt <email address hidden>

d1c4159... by Oliver Calder

interfaces/builtin: custom-device kernel in subdir (#12734)

* interfaces/builtin: custom-device kernel in subdir

Devices may be in subdirectories of `/dev/`, rather than directly in
`/dev/` itself. This change allows matching the `kernel` attribute of
`udev-tagging` to the basename of a device path in `devices`, rather
than matching `/dev/${kernel}` to a device in `devices`. Snapd maps the
value of the `kernel` key to the string used as the `KERNEL==` udev
rule. To the best of my knowledge, this change makes custom-device udev
kernel validation match the behavior of the `KERNEL` udev rule.

Also adds an additional test which should fail on the `subsystem` key,
thus indicating that the `kernel` key was successfully validated.

Signed-off-by: Oliver Calder <email address hidden>

* interfaces/builtin: use filepath.Base instead of custom code

Signed-off-by: Oliver Calder <email address hidden>

* interfaces/builtin: fixed udev kernel device matching

This change allows udev-tagging rules to have `kernel: deviceName` values
which match the end of device paths, rather than necessarily matching
either `"/dev/" + deviceName` or `deviceName` matching the basename of
the device.

Before, a basic udev `KERNEL==` rule was generated for each device and
then overwritten by the `kernel: deviceName` rules in the `udev-tagging`
section which matched `"/dev/" + deviceName` to the device path. Now,
the `kernel: deviceName` rules are converted to udev rules first, and
then any remaining devices which have not been matched by a rule are
given the basic `KERNEL==` udev rule.

If the number of devices are matched by the same `deviceName` from a
`kernel: deviceName` specification is not 1, then an error is thrown.
This is similar to previous handling of `deviceName` not matching any
devices, but now also includes checking that `deviceName` does not match
more than one device, as this is presumably not desired.

Signed-off-by: Oliver Calder <email address hidden>

* interfaces/builtin: custom-device stricter udev kernel rules

This change restricts the `udev-tagging` `kernel` attribute matching
rules so that `kernel: deviceName` only matches a `devicePath` from the
device list of `"/dev/"+deviceName == devicePath` or `deviceName` is
equal to the basename of `devicePath`.

Previously, `deviceName` matched `devicePath` if `deviceName` was the
string suffix of `devicePath` (not suffix in the path sense). This
allowed, for example, `bar/name` to match `/dev/foo/bar/name`, where
now it would only match `/dev/bar/name`.

It is still the case that a `deviceName` might match more than one
device, in which case an error will be thrown. For example,
`kernel: name` would match every device in
`devices: [/dev/name, /dev/foo/name, /dev/bar/name]`, which would throw
an error, since only one match is allowed for a given kernel
`deviceName`.

Note: if `deviceName` contains a subdir (e.g. `foo/bar`), then it must
match a complete path relative to `/dev/`, since it will never match the
basename of a device.

Signed-off-by: Oliver Calder <email address hidden>

* interfaces: make UDevConnectedPlug slightly more readable

---------

Signed-off-by: Oliver Calder <email address hidden>
Co-authored-by: Michael Vogt <email address hidden>

7c0ffa4... by Oliver Calder

interfaces/builtin: userspace and fs-specific mount options (#12712)

* interfaces/builtins: mount_control userspace options

Added both fs-independent mount options (ie. nofail, auto, defaults) and
fs-specific mount options. The latter are implemented on a per-FS
basis, where only options explicitly allowed by the filesystem type
specified in the mount attributes are allowed. If no filesystem type is
specified, then the allowed options are those of all the filesystems
listed in `defaultFSTypes`.

Mount options are treated as invalid if they fail to match any allowed
kernel mount options (as before), any fs-independent mount options, and
any fs-specific mount options corresponding to the specified filesystem
type.

Signed-off-by: Oliver Calder <email address hidden>

* interfaces/builtin: pass only kernel mount options to apparmor

Since only kernel mount options are validated by apparmor in production,
userspace and filesystem-specific mount options should not be included
in the apparmor profile.

This change filters the mount options to include only kernel mount
options when creating the option string to pass into apparmor.

Note: `BeforeConnectPlug()` is responsible for validating all options
beforehand, including userspace and fs-specific options, so
`AppArmorConnectedPlug()` performs no validation and merely filters the
option list as described above, ignoring userspace and fs-specific
options.

Signed-off-by: Oliver Calder <email address hidden>

* interfaces/builtin: refactored mount option error handling

Signed-off-by: Oliver Calder <email address hidden>

* interfaces/builtin: added comment about kernel mount option filtering

Signed-off-by: Oliver Calder <email address hidden>

* interfaces/builtin: improved mount option handling

This commit changes fs-specific option handling so that the option
validation fails if a mount option is _not_ one of:
1) a kernel mount option,
2) a userspace mount option, or
3) a fs-specific mount option which is compatible with every specified
   filesystem for the given mount configuration.

Previously, mount options were allowed if they were found on the option
list for any of the specified filesystems. This was not correct, as
mount options should only be allowed if they are allowed for all the
possible filesystems for a given mount specification.

Additionally, this commit adjusts the corresponding unit tests to revert
to their original form those which existed before the userspace mount
options changes were added, and to explicitly add new tests which check
userspace mount options (`nofail` in particular) and filesystem-specific
mount options according to the rules described above.

Signed-off-by: Oliver Calder <email address hidden>

* interfaces/builtin: removed `nofail` from kernel mount options

The `nofail` mount option is a userspace option which is not passed to
the kernel, and thus should not be included in the apparmor mount rules.

Since previous commits related to userspace mount options introduced
mount option filtering to include only kernel mount options in the
apparmor mount rules (after all mount options are validated), it is
important that `nofail` not be included in the kernel mount option list.

Signed-off-by: Oliver Calder <email address hidden>

* interfaces/builtin: reverted functionfs optionsWithoutFsType workaround

This change removes unsupported kernel mount options from
`optionsWithoutFsType`. Previously, commit 1026f5cd7dd added these
unsupported options as part of a change which allowed mount configs with
filesystem type `functionfs` to bypass kernel mount option validation,
and thus use any mount option, including those forbidden kernel options
which are affected by this change. This meant that if a mount config
with type `functionfs` used one of these mount options, such as `move`,
it would then be caught by the `optionIncompatibleWithFsType` check, as
a filesystem type (`functionfs`) was explicitly specified.

With more recent commits related to userspace mount options,
`functionfs` is now handled similarly to other filesystems: mount
options for `functionfs` are now validated against kernel, userspace,
and `functionfs`-specific mount options. Thus, the forbidden options
affected by this change will no longer make it past validation, even
with `functionfs` specified, and thus are removed from
optionsWithoutFsType` to increase clarity.

The test checking a mount with type `functionfs` and option
`make-private` has been adjusted according to the fact that
`make-private` is now rejected as unknown, similarly to any other
unsupported mount option, rather than rejected for a fstype being
specified.

Signed-off-by: Oliver Calder <email address hidden>

* interfaces/builtin: adjusted comments and naming for clarity

Signed-off-by: Oliver Calder <email address hidden>

* interfaces/builtin: refactored fs mount option validation

Signed-off-by: Oliver Calder <email address hidden>

* interfaces/builtin: fixed typo in mount-control

Signed-off-by: Oliver Calder <email address hidden>

* interfaces/builtin: fixed aufs mount options which use : as separator

Signed-off-by: Oliver Calder <email address hidden>

* interfaces/builtin: differentiate mount options with{,out} optargs

By splitting after the `=` in mount options such as `uid=1000`, we can
differentiate an allowed mount option which requires an optarg, such as
`uid=1000` for functionfs, from `uid`, which is not allowed. For
options with optional optargs, such as btrfs `compress`, by including
both `compress` and `compress=` in the list of allowed filesystem mount
options.

Signed-off-by: Oliver Calder <email address hidden>

* interfaces/builtin: generalize handling for filesystems with colon-separated mount options

Signed-off-by: Oliver Calder <email address hidden>

---------

Signed-off-by: Oliver Calder <email address hidden>
Co-authored-by: Michael Vogt <email address hidden>

8a7cdf2... by Michael Vogt

release: 2.59.5