Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

libct/cg/sd: set the DeviceAllow property before DevicePolicy #4612

Merged
merged 1 commit into from
Feb 7, 2025

Conversation

kolyshkin
Copy link
Contributor

@kolyshkin kolyshkin commented Feb 5, 2025

(this is a carry of #4569 which adds a better comment and a test case)

Every unit created by runc need daemon reload since systemd v230. This breaks support for NVIDIA GPUs, see
#3708 (comment)

A workaround is to set DeviceAllow before DevicePolicy.

Also:

  • add a test case (which fails before the fix) by @kolyshkin
  • better explain why we need empty DeviceAllow (by @cyphar)

Fixes #4568.

Reported-by: Jian Wen wenjianhn@gmail.com
Co-authored-by: Jian Wen wenjianhn@gmail.com
Co-authored-by: Aleksa Sarai cyphar@cyphar.com

1.2 backport: #4615

@kolyshkin
Copy link
Contributor Author

Before the fix, the test case fails like this:

root@kir-tp1:/home/kir/git/runc# export RUNC_USE_SYSTEMD=yes
root@kir-tp1:/home/kir/git/runc# bats tests/integration/dev.bats 
dev.bats
 ✓ runc run [redundant default /dev/tty]
 ✓ runc run [redundant default /dev/ptmx]
 ✓ runc run/update [device cgroup deny]
 ✓ runc run [device cgroup allow rw char device]
 ✓ runc run [device cgroup allow rm block device]
 ✓ runc exec vs systemctl daemon-reload
 ✗ runc run [systemd daemon-reload not needed]
   (from function `check_systemd_value' in file tests/integration/helpers.bash, line 283,
    in test file tests/integration/dev.bats, line 154)
     `check_systemd_value "NeedDaemonReload" "no"' failed
   runc spec (status=0):
   
   runc run -d --console-socket /tmp/bats-run-rnDsXP/runc.NyH2cg/tty/sock test_need_reload (status=0):
   
   systemd NeedDaemonReload: got yes, want no
   --- teardown ---

7 tests, 1 failure

@kolyshkin kolyshkin marked this pull request as ready for review February 5, 2025 02:45
@kolyshkin
Copy link
Contributor Author

As this is a small change, and fixes a real issue, I think we can backport it to release-1.2.

@kolyshkin kolyshkin added area/systemd backport/1.2-todo A PR in main branch which needs to be backported to release-1.2 labels Feb 5, 2025
Copy link
Member

@rata rata left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks! And I agree to 1.2 backporting too :)

Every unit created by runc need daemon reload since systemd v230.
This breaks support for NVIDIA GPUs, see
opencontainers#3708 (comment)

A workaround is to set DeviceAllow before DevicePolicy.

Also:
 - add a test case (which fails before the fix) by @kolyshkin
 - better explain why we need empty DeviceAllow (by @cyphar)

Fixes 4568.

Reported-by: Jian Wen <wenjianhn@gmail.com>
Co-authored-by: Jian Wen <wenjianhn@gmail.com>
Co-authored-by: Aleksa Sarai <cyphar@cyphar.com>
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
@kolyshkin kolyshkin added backport/1.2-done A PR in main branch which has been backported to release-1.2 and removed backport/1.2-todo A PR in main branch which needs to be backported to release-1.2 labels Feb 5, 2025
@kolyshkin
Copy link
Contributor Author

@giuseppe FYI I've checked (using the test case from this PR) that crun is not affected (probably because it doesn't add an empty DeviceAllow, although I'm not entirely sure).

@kolyshkin
Copy link
Contributor Author

@cyphar @AkihiroSuda PTAL (I want 1.2.5 to have this)

@AkihiroSuda AkihiroSuda merged commit dadea50 into opencontainers:main Feb 7, 2025
40 checks passed
@kolyshkin kolyshkin mentioned this pull request Feb 13, 2025
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
area/systemd backport/1.2-done A PR in main branch which has been backported to release-1.2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Every unit created by runc need daemon reload since systemd v230.
3 participants