From ae7482b994e6a9bc8e033de9accd24b1e1ffe2ed Mon Sep 17 00:00:00 2001
From: Luca Boccassi <bluca@debian.org>
Date: Thu, 7 Dec 2023 22:19:11 +0000
Subject: [PATCH] core: do not make private /dev/ read-only too soon

The read-only bit is flipped after setting up all the mounts, so that
bind mounts can be added. Remove the early config, and add a unit
test.

Fixes https://github.com/systemd/systemd/issues/30372
---
 src/core/namespace.c                               |  5 -----
 src/test/test-execute.c                            |  2 ++
 test/test-execute/exec-privatedevices-bind.service | 10 ++++++++++
 3 files changed, 12 insertions(+), 5 deletions(-)
 create mode 100644 test/test-execute/exec-privatedevices-bind.service

diff --git a/src/core/namespace.c b/src/core/namespace.c
index 1a4d15a8004..50d7b05aa09 100644
--- a/src/core/namespace.c
+++ b/src/core/namespace.c
@@ -1070,11 +1070,6 @@ static int mount_private_dev(MountEntry *m, RuntimeScope scope) {
         if (r < 0)
                 log_debug_errno(r, "Failed to set up basic device tree at '%s', ignoring: %m", temporary_mount);
 
-        /* Make the bind mount read-only. */
-        r = mount_nofollow_verbose(LOG_DEBUG, NULL, dev, NULL, MS_REMOUNT|MS_BIND|MS_RDONLY, NULL);
-        if (r < 0)
-                return r;
-
         /* Create the /dev directory if missing. It is more likely to be missing when the service is started
          * with RootDirectory. This is consistent with mount units creating the mount points when missing. */
         (void) mkdir_p_label(mount_entry_path(m), 0755);
diff --git a/src/test/test-execute.c b/src/test/test-execute.c
index 9a03e291a03..88e4c8d4d93 100644
--- a/src/test/test-execute.c
+++ b/src/test/test-execute.c
@@ -438,6 +438,8 @@ static void test_exec_privatedevices(Manager *m) {
 
         test(m, "exec-privatedevices-yes.service", can_unshare ? 0 : MANAGER_IS_SYSTEM(m) ? EXIT_FAILURE : EXIT_NAMESPACE, CLD_EXITED);
         test(m, "exec-privatedevices-no.service", 0, CLD_EXITED);
+        if (access("/dev/kvm", F_OK) >= 0)
+                test(m, "exec-privatedevices-bind.service", can_unshare ? 0 : MANAGER_IS_SYSTEM(m) ? EXIT_FAILURE : EXIT_NAMESPACE, CLD_EXITED);
         test(m, "exec-privatedevices-disabled-by-prefix.service", can_unshare ? 0 : MANAGER_IS_SYSTEM(m) ? EXIT_FAILURE : EXIT_NAMESPACE, CLD_EXITED);
         test(m, "exec-privatedevices-yes-with-group.service", can_unshare ? 0 : MANAGER_IS_SYSTEM(m) ? EXIT_FAILURE : EXIT_NAMESPACE, CLD_EXITED);
 
diff --git a/test/test-execute/exec-privatedevices-bind.service b/test/test-execute/exec-privatedevices-bind.service
new file mode 100644
index 00000000000..dbbbb4ee33d
--- /dev/null
+++ b/test/test-execute/exec-privatedevices-bind.service
@@ -0,0 +1,10 @@
+# SPDX-License-Identifier: LGPL-2.1-or-later
+[Unit]
+Description=Test for PrivateDevices=yes with a bind mounted device
+
+[Service]
+ExecStart=/bin/sh -c 'test -c /dev/kmsg'
+ExecStart=/bin/sh -c 'test ! -w /dev/'
+Type=oneshot
+PrivateDevices=yes
+BindPaths=/dev/kmsg