From 06384eb3c5044f632f50304a0210a402460f1189 Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Wed, 22 May 2024 00:17:10 +0100 Subject: [PATCH 1/2] lock-util: do not expect EACCES when it cannot happen As per the documentation, EACCES is only returned when F_SETLK is used, and only on some platforms, which doesn't seem to include Linux: https://github.com/torvalds/linux/blob/master/fs/locks.c F_OFD_SETLK is documented to only return EAGAIN, and F_SETLKW/F_OFD_SETLKW are blocking operations so this logic doesn't apply to them in the first place. Hence, only automatically convert EACCES into EAGAIN for F_SETLK operations, and propagate the original error in the other cases. This is important because in some cases we catch permission errors and gracefully fallback, which is not possible if the original error is lost. This is an issue in practice because, due to a kernel bug present before v6.2, AppArmor denies locking on file descriptors to LXC containers. We support all currently maintained LTS kernels, including v6.1, where despite a lot of effort and attempts over almost a year, the bugfix still hasn't been backported, as it is complex and requires large changes to AppArmor. On affected kernels, all services running with PrivateNetwork=yes fail and do not recover, instead of the normal behaviour of gracefully downgrading to PrivateNetwork=no. The integration tests in the Debian CI fail due to this issue: https://ci.debian.net/packages/s/systemd/testing/arm64/46828037/ --- src/basic/lock-util.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/basic/lock-util.c b/src/basic/lock-util.c index 7bffe85461..aef395d78e 100644 --- a/src/basic/lock-util.c +++ b/src/basic/lock-util.c @@ -139,7 +139,14 @@ static int fcntl_lock(int fd, int operation, bool ofd) { .l_len = 0, })); - if (r == -EACCES) /* Treat EACCESS/EAGAIN the same as per man page. */ + /* If we are doing non-blocking operations, treat EACCES/EAGAIN the same as per man page. But if + * not, propagate EACCES back, as it will likely be due to an LSM denying the operation (for example + * LXC with AppArmor when running on kernel < 6.2), and in some cases we want to gracefully + * fallback (e.g.: PrivateNetwork=yes). As per documentation, it's only the non-blocking operation + * F_SETLK that might return EACCES on some platforms (although the Linux implementation doesn't + * seem to), as F_SETLKW and F_OFD_SETLKW block so this is not an issue, and F_OFD_SETLK is documented + * to only return EAGAIN if the lock is already held. */ + if ((operation & LOCK_NB) && r == -EACCES) r = -EAGAIN; return r; From 6ab21f20bd982bc1a9ece47dcffa1137a76cc48a Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Tue, 21 May 2024 01:43:24 +0100 Subject: [PATCH 2/2] test: do not fail network namespace test with permission issues When running in LXC with AppArmor we'll most likely get an error when creating a network namespace due to a kernel regression in < v6.2 affecting AppArmor, resulting in denials. Like other tests, avoid failing in case of permission issues and handle it gracefully. --- src/test/test-namespace.c | 34 +++++++++++++++++++++++++--------- 1 file changed, 25 insertions(+), 9 deletions(-) diff --git a/src/test/test-namespace.c b/src/test/test-namespace.c index 65d08259d4..2a684ce096 100644 --- a/src/test/test-namespace.c +++ b/src/test/test-namespace.c @@ -1,6 +1,7 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ #include +#include #include #include @@ -84,6 +85,7 @@ TEST(tmpdir) { static void test_shareable_ns(unsigned long nsflag) { _cleanup_close_pair_ int s[2] = EBADF_PAIR; + bool permission_denied = false; pid_t pid1, pid2, pid3; int r, n = 0; siginfo_t si; @@ -100,8 +102,8 @@ static void test_shareable_ns(unsigned long nsflag) { if (pid1 == 0) { r = setup_shareable_ns(s, nsflag); - assert_se(r >= 0); - _exit(r); + assert_se(r >= 0 || ERRNO_IS_NEG_PRIVILEGE(r)); + _exit(r >= 0 ? r : EX_NOPERM); } pid2 = fork(); @@ -109,8 +111,8 @@ static void test_shareable_ns(unsigned long nsflag) { if (pid2 == 0) { r = setup_shareable_ns(s, nsflag); - assert_se(r >= 0); - exit(r); + assert_se(r >= 0 || ERRNO_IS_NEG_PRIVILEGE(r)); + _exit(r >= 0 ? r : EX_NOPERM); } pid3 = fork(); @@ -118,24 +120,38 @@ static void test_shareable_ns(unsigned long nsflag) { if (pid3 == 0) { r = setup_shareable_ns(s, nsflag); - assert_se(r >= 0); - exit(r); + assert_se(r >= 0 || ERRNO_IS_NEG_PRIVILEGE(r)); + _exit(r >= 0 ? r : EX_NOPERM); } r = wait_for_terminate(pid1, &si); assert_se(r >= 0); assert_se(si.si_code == CLD_EXITED); - n += si.si_status; + if (si.si_status == EX_NOPERM) + permission_denied = true; + else + n += si.si_status; r = wait_for_terminate(pid2, &si); assert_se(r >= 0); assert_se(si.si_code == CLD_EXITED); - n += si.si_status; + if (si.si_status == EX_NOPERM) + permission_denied = true; + else + n += si.si_status; r = wait_for_terminate(pid3, &si); assert_se(r >= 0); assert_se(si.si_code == CLD_EXITED); - n += si.si_status; + if (si.si_status == EX_NOPERM) + permission_denied = true; + else + n += si.si_status; + + /* LSMs can cause setup_shareable_ns() to fail with permission denied, do not fail the test in that + * case (e.g.: LXC with AppArmor on kernel < v6.2). */ + if (permission_denied) + return (void) log_tests_skipped("insufficient privileges"); assert_se(n == 1); }