[PATCH v2 4/7] ns: allocate the namespace id before the namespace_init hook

[Christian Brauner <brauner@kernel.org>]

Currently the namespace id is allocated only after __ns_common_init()
has called the namespace_init LSM hook. Most namespace types allocate it
in ns_tree_add() right before the namespace is added to the namespace
trees. So the namespace_init hook always sees an id of zero.

The namespace id is the only identifier of a namespace that is never
recycled. A bpf LSM wants to use it to track a namespace from
namespace_init until namespace_free, e.g., by using it as the key into a
map that associates data with the namespace. The inode number doesn't
work for that as it is quickly recycled.

So allocate the id in __ns_common_init() before calling the
namespace_init hook. Every namespace then has its id by the time it is
added to the namespace trees. The initial namespaces still get their
fixed ids, either statically from NS_COMMON_INIT() or for init_net from
ns_common_init(). So ns_tree_add() doesn't need to allocate an id
anymore. Ids are still handed out in creation order so mnt_ns_loop()
keeps working. Mount and ipc namespaces already allocated their id right
after ns_common_init() anyway.

If creating the namespace fails after the id has been allocated the id
is simply never used.

Landlock logs the namespace id when it denies the use of a namespace.
For a denied namespace creation that used to be zero. Now it's the id
that was allocated for the new namespace. That namespace never becomes
visible to userspace but it's the same id that other LSMs see in
namespace_init. Update the Landlock documentation and selftests
accordingly. The selftests can't know the id of a namespace whose
creation was denied so they only check that it isn't zero.

While at it, drop ns_tree_gen_id() as it has no users left.

Reproducer, 100 user and uts namespaces:

  bpftrace -e 'fentry:bpf_lsm_namespace_init {
      @[args.ns->ns_id ? "set" : "zero"] = count(); }' &
  for i in $(seq 100); do unshare -Uu true; done

  before: @[zero]: 200
  after:  @[set]: 200

Fixes: f675d2e95569 ("lsm: add LSM blob and hooks for namespaces")
Cc: <stable@kernel.org> # mainline only
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 Documentation/admin-guide/LSM/landlock.rst |  5 +++--
 Documentation/trace/events-landlock.rst    |  5 +++--
 fs/namespace.c                             |  2 --
 include/linux/ns_common.h                  | 16 +++++++++-------
 include/linux/nstree.h                     | 18 +++---------------
 include/trace/events/landlock.h            |  3 ++-
 ipc/namespace.c                            |  1 -
 kernel/nscommon.c                          |  7 +++++--
 net/core/net_namespace.c                   |  2 +-
 security/landlock/ns.c                     |  3 ++-
 tools/testing/selftests/landlock/ns_test.c |  7 +++++--
 11 files changed, 33 insertions(+), 36 deletions(-)

diff --git a/Documentation/admin-guide/LSM/landlock.rst b/Documentation/admin-guide/LSM/landlock.rst
index 718e0d23ffb5..946f3bf89650 100644
--- a/Documentation/admin-guide/LSM/landlock.rst
+++ b/Documentation/admin-guide/LSM/landlock.rst
@@ -73,8 +73,9 @@ AUDIT_LANDLOCK_ACCESS
           :manpage:`open_tree(2)`, or :manpage:`fsmount(2)`, or joining via
           :manpage:`setns(2)`);
           ``namespace_type`` indicates the type (hex ``CLONE_NEW*`` bitmask),
-          and ``namespace_id`` identifies the target namespace for
-          :manpage:`setns(2)` operations (zero for creation)
+          and ``namespace_id`` identifies the target namespace (for a denied
+          creation, the namespace never becomes reachable so its ID is not
+          directly visible from user space)
 
     **capability.*** - Capability restrictions (ABI 12+):
         - capability.use - Capability use was denied;
diff --git a/Documentation/trace/events-landlock.rst b/Documentation/trace/events-landlock.rst
index 9ccf2769a2d5..dc5f05a17c2c 100644
--- a/Documentation/trace/events-landlock.rst
+++ b/Documentation/trace/events-landlock.rst
@@ -187,8 +187,9 @@ therefore appears as zero while still advancing the ruleset version, and adding
 a member already present still emits an event.
 
 ``landlock_deny_permission_namespace`` reports the denied raw
-``CLONE_NEW*`` value.  ``namespace_id`` is zero for namespace creation and is
-the exact target namespace ID for :manpage:`setns(2)`.
+``CLONE_NEW*`` value.  ``namespace_id`` is the exact target namespace ID.  For
+a denied namespace creation, the namespace never becomes reachable so its ID
+is not directly visible from user space.
 ``landlock_deny_permission_capability`` reports the denied ``CAP_*`` number.
 Like every denial event, both include the denying domain, ``same_exec``, and
 ``logged``.
diff --git a/fs/namespace.c b/fs/namespace.c
index 9f6ec35c7b9c..f81d6a4bc3fe 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -4394,8 +4394,6 @@ static struct mnt_namespace *alloc_mnt_ns(struct user_namespace *user_ns, bool a
 		dec_mnt_namespaces(ucounts);
 		return ERR_PTR(ret);
 	}
-	ns_tree_gen_id(new_ns);
-
 	new_ns->is_anon = anon;
 	refcount_set(&new_ns->passive, 1);
 	new_ns->mounts = RB_ROOT;
diff --git a/include/linux/ns_common.h b/include/linux/ns_common.h
index c8e227a3f9e2..e8b2bf0da5ed 100644
--- a/include/linux/ns_common.h
+++ b/include/linux/ns_common.h
@@ -9,7 +9,8 @@
 #include <uapi/linux/nsfs.h>
 
 bool is_current_namespace(struct ns_common *ns);
-int __ns_common_init(struct ns_common *ns, u32 ns_type, const struct proc_ns_operations *ops, int inum);
+int __ns_common_init(struct ns_common *ns, u32 ns_type, const struct proc_ns_operations *ops,
+		     int inum, u64 ns_id);
 void __ns_common_free(struct ns_common *ns);
 struct ns_common *__must_check ns_owner(struct ns_common *ns);
 
@@ -41,17 +42,18 @@ static __always_inline bool is_ns_init_id(const struct ns_common *ns)
 	.ns_owner_root.ns_list_head	= LIST_HEAD_INIT(nsname.ns.ns_owner_root.ns_list_head),		\
 }
 
-#define ns_common_init(__ns)                     \
-	__ns_common_init(to_ns_common(__ns),     \
-			 ns_common_type(__ns),   \
-			 to_ns_operations(__ns), \
-			 (((__ns) == ns_init_ns(__ns)) ? ns_init_inum(__ns) : 0))
+#define ns_common_init(__ns)						\
+	__ns_common_init(to_ns_common(__ns),				\
+			 ns_common_type(__ns),				\
+			 to_ns_operations(__ns),			\
+			 (((__ns) == ns_init_ns(__ns)) ? ns_init_inum(__ns) : 0), \
+			 (((__ns) == ns_init_ns(__ns)) ? ns_init_id(__ns) : 0))
 
 #define ns_common_init_inum(__ns, __inum)        \
 	__ns_common_init(to_ns_common(__ns),     \
 			 ns_common_type(__ns),   \
 			 to_ns_operations(__ns), \
-			 __inum)
+			 __inum, 0)
 
 #define ns_common_free(__ns) __ns_common_free(to_ns_common((__ns)))
 
diff --git a/include/linux/nstree.h b/include/linux/nstree.h
index 5b64d4572881..3293df17397d 100644
--- a/include/linux/nstree.h
+++ b/include/linux/nstree.h
@@ -41,10 +41,6 @@ void ns_tree_node_del(struct ns_tree_node *node, struct ns_tree_root *root);
 		struct user_namespace *:   &(user_ns_tree),	\
 		struct uts_namespace *:    &(uts_ns_tree))
 
-#define ns_tree_gen_id(__ns)                 \
-	__ns_tree_gen_id(to_ns_common(__ns), \
-			 (((__ns) == ns_init_ns(__ns)) ? ns_init_id(__ns) : 0))
-
 u64 __ns_tree_gen_id(struct ns_common *ns, u64 id);
 void __ns_tree_add_raw(struct ns_common *ns, struct ns_tree_root *ns_tree);
 void __ns_tree_remove(struct ns_common *ns, struct ns_tree_root *ns_tree);
@@ -53,12 +49,6 @@ struct ns_common *__ns_tree_adjoined_rcu(struct ns_common *ns,
 					 struct ns_tree_root *ns_tree,
 					 bool previous);
 
-static inline void __ns_tree_add(struct ns_common *ns, struct ns_tree_root *ns_tree, u64 id)
-{
-	__ns_tree_gen_id(ns, id);
-	__ns_tree_add_raw(ns, ns_tree);
-}
-
 /**
  * ns_tree_add_raw - Add a namespace to a namespace
  * @__ns: Namespace to add
@@ -72,12 +62,10 @@ static inline void __ns_tree_add(struct ns_common *ns, struct ns_tree_root *ns_t
  * ns_tree_add - Add a namespace to a namespace tree
  * @__ns: Namespace to add
  *
- * This function assigns a new id to the namespace and adds it to the
- * appropriate namespace tree and list.
+ * This function adds a namespace to the appropriate namespace tree and
+ * list. The namespace must already have been assigned an id.
  */
-#define ns_tree_add(__ns)                                   \
-	__ns_tree_add(to_ns_common(__ns), to_ns_tree(__ns), \
-		      (((__ns) == ns_init_ns(__ns)) ? ns_init_id(__ns) : 0))
+#define ns_tree_add(__ns) __ns_tree_add_raw(to_ns_common(__ns), to_ns_tree(__ns))
 
 /**
  * ns_tree_remove - Remove a namespace from a namespace tree
diff --git a/include/trace/events/landlock.h b/include/trace/events/landlock.h
index 6d9d9e7c7ca2..b6fdc6e2eef8 100644
--- a/include/trace/events/landlock.h
+++ b/include/trace/events/landlock.h
@@ -987,7 +987,8 @@ TRACE_EVENT(landlock_deny_access_net,
  * @logged: Whether this denial was selected for audit logging.
  * @blockers: Request type and final missing permission subset (never NULL).
  * @namespace_type: CLONE_NEW* namespace type that was denied.
- * @namespace_id: Namespace ID, or 0 when creation was denied.
+ * @namespace_id: Namespace ID, not directly visible from user space when
+ *                creation was denied.
  *
  * Emitted when a Landlock domain denies namespace use.
  */
diff --git a/ipc/namespace.c b/ipc/namespace.c
index 75aa7b469348..9084e0643017 100644
--- a/ipc/namespace.c
+++ b/ipc/namespace.c
@@ -66,7 +66,6 @@ static struct ipc_namespace *create_ipc_ns(struct user_namespace *user_ns,
 	if (err)
 		goto fail_free;
 
-	ns_tree_gen_id(ns);
 	ns->user_ns = get_user_ns(user_ns);
 	ns->ucounts = ucounts;
 
diff --git a/kernel/nscommon.c b/kernel/nscommon.c
index e72426bba29a..4438c6b663f9 100644
--- a/kernel/nscommon.c
+++ b/kernel/nscommon.c
@@ -54,7 +54,8 @@ static void ns_debug(struct ns_common *ns, const struct proc_ns_operations *ops)
 }
 #endif
 
-int __ns_common_init(struct ns_common *ns, u32 ns_type, const struct proc_ns_operations *ops, int inum)
+int __ns_common_init(struct ns_common *ns, u32 ns_type, const struct proc_ns_operations *ops,
+		     int inum, u64 ns_id)
 {
 	int ret = 0;
 
@@ -64,7 +65,6 @@ int __ns_common_init(struct ns_common *ns, u32 ns_type, const struct proc_ns_ope
 	ns->ns_security = NULL;
 #endif
 	ns->ops = ops;
-	ns->ns_id = 0;
 	ns->ns_type = ns_type;
 	ns_tree_node_init(&ns->ns_tree_node);
 	ns_tree_node_init(&ns->ns_unified_node);
@@ -82,6 +82,9 @@ int __ns_common_init(struct ns_common *ns, u32 ns_type, const struct proc_ns_ope
 	if (ret)
 		return ret;
 
+	/* Allocate the id before the LSM hook so it can track the namespace by it. */
+	__ns_tree_gen_id(ns, ns_id);
+
 	ret = security_namespace_init(ns);
 	if (ret) {
 		if (!inum)
diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
index da5f881fbd3b..ec4077b81389 100644
--- a/net/core/net_namespace.c
+++ b/net/core/net_namespace.c
@@ -437,7 +437,7 @@ static __net_init int setup_net(struct net *net)
 	LIST_HEAD(net_exit_list);
 	int error = 0;
 
-	net->net_cookie = ns_tree_gen_id(net);
+	net->net_cookie = to_ns_common(net)->ns_id;
 
 	list_for_each_entry(ops, &pernet_list, list) {
 		error = ops_init(ops, net);
diff --git a/security/landlock/ns.c b/security/landlock/ns.c
index cd44c927f83a..06d9784746fe 100644
--- a/security/landlock/ns.c
+++ b/security/landlock/ns.c
@@ -94,7 +94,8 @@ static const struct access_masks ns_permission = {
  * Shared check for namespace_init (creation via clone(2), unshare(2),
  * open_tree(2), or fsmount(2)) and namespace_install (use via setns(2)): denies
  * when the namespace type is not in the domain's allowed set.  At allocation
- * time @ns->ns_id is still zero and is logged as such.
+ * time @ns->ns_id is already set even though the namespace is not reachable
+ * from user space yet.
  *
  * Return: 0 if allowed, -EPERM if denied.
  */
diff --git a/tools/testing/selftests/landlock/ns_test.c b/tools/testing/selftests/landlock/ns_test.c
index f5d4b86edd30..8f387fc47e21 100644
--- a/tools/testing/selftests/landlock/ns_test.c
+++ b/tools/testing/selftests/landlock/ns_test.c
@@ -1899,8 +1899,9 @@ TEST(setns_inherited_fd)
 
 static int matches_log_ns_create(int audit_fd, __u64 ns_type)
 {
+	/* A denied creation logs the ID allocated for the new namespace. */
 	static const char log_template[] = REGEX_LANDLOCK_PREFIX
-		" blockers=namespace\\.use namespace_type=0x%x namespace_id=0$";
+		" blockers=namespace\\.use namespace_type=0x%x namespace_id=[1-9][0-9]*$";
 	char log_match[sizeof(log_template) + 10];
 	int log_match_len;
 
@@ -2538,7 +2539,9 @@ static void test_ns_trace_denial(struct __test_metadata *const _metadata,
 	ASSERT_EQ(0, tracefs_extract_field(
 			     buf, REGEX_DENY_PERMISSION_NAMESPACE(TRACE_TASK),
 			     "namespace_id", field, sizeof(field)));
-	EXPECT_EQ(use_setns ? expected_ns_id : 0, strtoull(field, NULL, 10));
+	EXPECT_STRNE("0", field);
+	if (use_setns)
+		EXPECT_EQ(expected_ns_id, strtoull(field, NULL, 10));
 
 	free(buf);
 }

-- 
2.53.0