mount event propagation on snapd-mounted tmpfs is incorrect

Bug #1828354 reported by Zygmunt Krynicki
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
snapd
Confirmed
High
Zygmunt Krynicki

Bug Description

When snapd (via tools such as snap-update-ns) mounts a tmpfs, either for an explicit layout or for a writable mimic, it does not change the mount event propagation for that mount point. This has consequences on mount event visibility in derived, per-user, mount namespace used by non-root applications.

As a simple example consider an application that uses a tmpfs layout on $SNAP/extra or accepts content connections to that directory (in that second variant a writable mimic is established either on $SNAP to create $SNAP/extra or on $SNAP/extra directly, if it exists, to create mount points for connected content slots).

During startup of this snap application there are three mount namespaces in existence:

- the initial mount namespace of the system (let's call it NS:sys)
- the mount namespace of the snap, unshared from NS:sys (let's call it NS:snap)
- the mount namespace of the user using the snap, unshared from NS:snap (NS:snap-user)

Due to bug https://bugs.launchpad.net/snapd/+bug/1828352 there is also a 2nd copy of NS:snap-user but let's ignore it as it is not having an impact on this bug.

On snap application startup, if the snap is not using classic confinement, NS:sys is unshared and becomes NS:snap. That namespace is modified according to bootstrap rules and per-snap mount profiles and saved to /run/snapd/ns/$SNAP_INSTANCE_NAME.mnt

If the calling user is not root then NS:snap is unshared again to become NS:snap-user. That namespace is again modified according to bootstrap rules and per-snap, per-user mount profiles and is optionally saved to /run/snapd/ns/$SNAP_INSTANCE_NAME.$UID.mnt (it's an experimental feature for now).

When snapd interface connections change they can impact the per-snap mount profile. Snapd then uses snap-update-ns to change the existing mount namespace, possibly inhabited by some processes (in practice only services, hooks and apps started by root are using that namespace directly because user processes will instead inhabit NS:snap-user).

Snapd can mount a tmpfs in one of two several cases:
 - a layout explicitly requires one
 - a mount profile describes a mount point that doesn't exist and cannot be created because it is in a read-only filesystem (e.g. $SNAP/extra is missing but we want to mount $SNAP/extra/a)
 - a mount profile describes a mount point that doesn't exist and cannot be created because it affects a directory visible from NS:sys. A prime example is layout demanding something to be created in /etc/, e.g. /etc/foo. If /etc/foo doesn't exist in NS:sys we must first create a writable mimic in NS:snap so that our modifications to /etc/ (creating an empty file, empty directory or symlink) do not show up in NS:sys.

In each of the cases described above the tmpfs would be created with a system call that corresponds to shell command: mount -t tmpfs tmpfs $MOUNT_POINT.

In all of the cases possible now, that mount would happen in NS:snap. Per-user mount namespace is not persisted yet and has a single limited use case that is not affected by this bug.

When a snapd interface connection causes a mount event to happen underneath such tmpfs, that event, which will happen in NS:snap, will not propagate to NS:snap-user because by default new mount points are not shared.

Anyone inspecting the mount NS:snap mount namespace will see the mount being present.
Anyone starting new snap commands, because NS:snap-user is not persisted, will seed the mount being present because NS:snap-user is re-created by deriving it from NS:snap.
Anyone inhabiting a particular instance of NS:snap-user will NOT see the mount being present.

As a simple solution, whenever a tmpfs is mounted, the mount event propagation must be set to MS_SHARED, so that slave mount namespace NS:snap-user, will see modifications to it. This was determined experimentally with a simple patch to snap-update-ns.

Zygmunt Krynicki (zyga)
Changed in snapd:
importance: Undecided → Critical
status: New → In Progress
assignee: nobody → Zygmunt Krynicki (zyga)
Zygmunt Krynicki (zyga)
description: updated
Revision history for this message
Zygmunt Krynicki (zyga) wrote :

I've proposed a fix for this in https://github.com/snapcore/snapd/pull/6891

Revision history for this message
Zygmunt Krynicki (zyga) wrote :

This took a while but the fix is close to being mergeable. I will open a new PR early next week, as prerequisites get merged.

Revision history for this message
Zygmunt Krynicki (zyga) wrote :

The v2 version of the fix is being worked on here: https://github.com/snapcore/snapd/pull/7436

Revision history for this message
Zygmunt Krynicki (zyga) wrote :

This is still in progress (some related bugs in the repository have been fixed recently) but has not been merged and is really not super critical. Demoting to high and marking as confirmed. I will change this back when I can work on the related branch again.

Changed in snapd:
importance: Critical → High
status: In Progress → Confirmed
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.