cmd/snap-confine: lazy set up of device cgroup, only when devices were assigned #10968
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…e assigned Historically, a snap process ended up in a device cgroup (with device filtering) only when there were assigned devices for it. On systems where CURRENT_TAGS is supported and set by systemd/udev, snap-confine needs to do 2 passes on the list of assigned devices. It may happen, that despite snap tag being present in the TAGS list, it will not be present in the CURRENT_TAGS, in which case we may end up in a scenario when no devices are actually assigned to the snap. The current code would incorrectly handle such situation, and move the process into device cgroup. The branch introduces a lazy initialization of device cgroup and moves the process to the group (or sets up device filtering on v2) only when there were any assigned device. Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
bboozzoo
added
the
Needs security review
Codecov Report
@@ Coverage Diff @@
## master #10968 +/- ##
==========================================
- Coverage 78.18% 78.17% -0.01%
==========================================
Files 910 910
Lines 102788 102788
==========================================
- Hits 80360 80358 -2
- Misses 17405 17406 +1
- Partials 5023 5024 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
Now that device cgroup assignment when no devices match is done properly, i.e. we do not end up in a cgroup with just the common set of devices if none are assigned, the test needs to be updated as we correctly observe first the AppArmor denial, and then a device cgroup one. Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
…fine-cgroup-only-devices-were-assigned
stolowski
approved these changes
Oct 25, 2021
alexmurray
removed
the
Needs security review
|
some unrelated test failers, @mvo5 can you land this branch? |
|
Also I think we should cherry-pick this onto 2.53 for 2.53.1 |
|
or sorry for 2.53.2 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Historically, a snap process ended up in a device cgroup (with device filtering)
only when there were assigned devices for it. On systems where CURRENT_TAGS is
supported and set by systemd/udev, snap-confine needs to do 2 passes on the list
of assigned devices. It may happen, that despite snap tag being present in the
TAGS list, it will not be present in the CURRENT_TAGS, in which case we may end
up in a scenario when no devices are actually assigned to the snap. The current
code would incorrectly handle such situation, and move the process into device
cgroup.
The branch introduces a lazy initialization of device cgroup and moves the
process to the group (or sets up device filtering on v2) only when there were
any assigned device.