[PATCH v1 1/4] landlock: Fix kernel-doc warning for pointer-to-array parameters

[PATCH v1 1/4] landlock: Fix kernel-doc warning for pointer-to-array parameters

[Mickaël Salaün]

The insert_rule() and create_rule() functions take a
pointer-to-flexible-array parameter declared as:

  const struct landlock_layer (*const layers)[]

The kernel-doc parser cannot handle a qualifier between * and the
parameter name in this syntax, producing spurious "Invalid param" and
"not described" warnings.

Introduce landlock_layer_array_t as a typedef for the flexible array
type so the parameter can be written as:

  const landlock_layer_array_t *const layers

This is the same type but kernel-doc parses it correctly, while
preserving the pointer-to-array type safety that prevents callers from
accidentally passing a pointer to a single element.

Cc: Günther Noack <gnoack@google.com>
Signed-off-by: Mickaël Salaün <mic@digikod.net>
---
 security/landlock/ruleset.c | 4 ++--
 security/landlock/ruleset.h | 8 ++++++++
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c
index 419b237de635..a61ced492f41 100644
--- a/security/landlock/ruleset.c
+++ b/security/landlock/ruleset.c
@@ -108,7 +108,7 @@ static bool is_object_pointer(const enum landlock_key_type key_type)
 
 static struct landlock_rule *
 create_rule(const struct landlock_id id,
-	    const struct landlock_layer (*const layers)[], const u32 num_layers,
+	    const landlock_layer_array_t *const layers, const u32 num_layers,
 	    const struct landlock_layer *const new_layer)
 {
 	struct landlock_rule *new_rule;
@@ -205,7 +205,7 @@ static void build_check_ruleset(void)
  */
 static int insert_rule(struct landlock_ruleset *const ruleset,
 		       const struct landlock_id id,
-		       const struct landlock_layer (*const layers)[],
+		       const landlock_layer_array_t *const layers,
 		       const size_t num_layers)
 {
 	struct rb_node **walker_node;
diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h
index 9d6dc632684c..87d52031fb5a 100644
--- a/security/landlock/ruleset.h
+++ b/security/landlock/ruleset.h
@@ -37,6 +37,14 @@ struct landlock_layer {
 	access_mask_t access;
 };
 
+/*
+ * Flexible array of Landlock layers, used for pointer-to-array function
+ * parameters that reference either a stack-allocated layer array or a rule's
+ * flexible array member (struct landlock_rule.layers).  This typedef avoids
+ * the complex (*const name)[] syntax that the kernel-doc parser cannot handle.
+ */
+typedef struct landlock_layer landlock_layer_array_t[];
+
 /**
  * union landlock_key - Key of a ruleset's red-black tree
  */
-- 
2.53.0

[PATCH v1 2/4] landlock: Add missing kernel-doc "Return:" sections

[Mickaël Salaün]

The kernel-doc -Wreturn check warns about functions with documentation
comments that lack a "Return:" section.  Add "Return:" documentation to
all functions missing it so that kernel-doc -Wreturn passes cleanly.

Convert existing function descriptions into a formal "Return:" section.
Also fix the inaccurate return documentation for
landlock_merge_ruleset() which claimed to return @parent directly, and
document the previously missing ERR_PTR() error return path.  Document
the ABI version and errata return paths for landlock_create_ruleset()
which were previously only implied by the prose.

Cc: Günther Noack <gnoack@google.com>
Signed-off-by: Mickaël Salaün <mic@digikod.net>
---
 security/landlock/domain.c   |  2 ++
 security/landlock/fs.c       |  2 +-
 security/landlock/ruleset.c  |  8 +++++---
 security/landlock/syscalls.c | 17 +++++++++++------
 security/landlock/task.c     |  9 +++++----
 5 files changed, 24 insertions(+), 14 deletions(-)

diff --git a/security/landlock/domain.c b/security/landlock/domain.c
index 79cb3bbdf4c5..343a1aabaac6 100644
--- a/security/landlock/domain.c
+++ b/security/landlock/domain.c
@@ -115,6 +115,8 @@ static struct landlock_details *get_current_details(void)
  * restriction.  The subjective credentials must not be in an overridden state.
  *
  * @hierarchy->parent and @hierarchy->usage should already be set.
+ *
+ * Return: 0 on success, -errno on failure.
  */
 int landlock_init_hierarchy_log(struct landlock_hierarchy *const hierarchy)
 {
diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index e764470f588c..cfe69075bf4e 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -1568,7 +1568,7 @@ static int hook_path_truncate(const struct path *const path)
  *
  * @file: File being opened.
  *
- * Returns the access rights that are required for opening the given file,
+ * Return: The access rights that are required for opening the given file,
  * depending on the file type and open mode.
  */
 static access_mask_t
diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c
index a61ced492f41..de8386af2f30 100644
--- a/security/landlock/ruleset.c
+++ b/security/landlock/ruleset.c
@@ -202,6 +202,8 @@ static void build_check_ruleset(void)
  * When merging a ruleset in a domain, or copying a domain, @layers will be
  * added to @ruleset as new constraints, similarly to a boolean AND between
  * access rights.
+ *
+ * Return: 0 on success, -errno on failure.
  */
 static int insert_rule(struct landlock_ruleset *const ruleset,
 		       const struct landlock_id id,
@@ -531,8 +533,8 @@ void landlock_put_ruleset_deferred(struct landlock_ruleset *const ruleset)
  * The current task is requesting to be restricted.  The subjective credentials
  * must not be in an overridden state. cf. landlock_init_hierarchy_log().
  *
- * Returns the intersection of @parent and @ruleset, or returns @parent if
- * @ruleset is empty, or returns a duplicate of @ruleset if @parent is empty.
+ * Return: A new domain merging @parent and @ruleset on success, or ERR_PTR()
+ * on failure.  If @parent is NULL, the new domain duplicates @ruleset.
  */
 struct landlock_ruleset *
 landlock_merge_ruleset(struct landlock_ruleset *const parent,
@@ -623,7 +625,7 @@ landlock_find_rule(const struct landlock_ruleset *const ruleset,
  * @rule: A rule that grants a set of access rights for each layer
  * @masks: A matrix of unfulfilled access rights for each layer
  *
- * Returns true if the request is allowed (i.e. the access rights granted all
+ * Return: True if the request is allowed (i.e. the access rights granted all
  * remaining unfulfilled access rights and masks has no leftover set bits).
  */
 bool landlock_unmask_layers(const struct landlock_rule *const rule,
diff --git a/security/landlock/syscalls.c b/security/landlock/syscalls.c
index 0d66a68677b7..3b33839b80c7 100644
--- a/security/landlock/syscalls.c
+++ b/security/landlock/syscalls.c
@@ -60,6 +60,8 @@ static bool is_initialized(void)
  * @ksize_min: Minimal required size to be copied.
  * @src: User space pointer or NULL.
  * @usize: (Alleged) size of the data pointed to by @src.
+ *
+ * Return: 0 on success, -errno on failure.
  */
 static __always_inline int
 copy_min_struct_from_user(void *const dst, const size_t ksize,
@@ -178,16 +180,19 @@ const int landlock_abi_version = 8;
  *         - %LANDLOCK_CREATE_RULESET_VERSION
  *         - %LANDLOCK_CREATE_RULESET_ERRATA
  *
- * This system call enables to create a new Landlock ruleset, and returns the
- * related file descriptor on success.
+ * This system call enables to create a new Landlock ruleset.
  *
  * If %LANDLOCK_CREATE_RULESET_VERSION or %LANDLOCK_CREATE_RULESET_ERRATA is
  * set, then @attr must be NULL and @size must be 0.
  *
- * Possible returned errors are:
+ * Return: The ruleset file descriptor on success, the Landlock ABI version if
+ * %LANDLOCK_CREATE_RULESET_VERSION is set, the errata value if
+ * %LANDLOCK_CREATE_RULESET_ERRATA is set, or -errno on failure.  Possible
+ * returned errors are:
  *
  * - %EOPNOTSUPP: Landlock is supported by the kernel but disabled at boot time;
- * - %EINVAL: unknown @flags, or unknown access, or unknown scope, or too small @size;
+ * - %EINVAL: unknown @flags, or unknown access, or unknown scope, or too small
+ *   @size;
  * - %E2BIG: @attr or @size inconsistencies;
  * - %EFAULT: @attr or @size inconsistencies;
  * - %ENOMSG: empty &landlock_ruleset_attr.handled_access_fs.
@@ -398,7 +403,7 @@ static int add_rule_net_port(struct landlock_ruleset *ruleset,
  * This system call enables to define a new rule and add it to an existing
  * ruleset.
  *
- * Possible returned errors are:
+ * Return: 0 on success, or -errno on failure.  Possible returned errors are:
  *
  * - %EOPNOTSUPP: Landlock is supported by the kernel but disabled at boot time;
  * - %EAFNOSUPPORT: @rule_type is %LANDLOCK_RULE_NET_PORT but TCP/IP is not
@@ -464,7 +469,7 @@ SYSCALL_DEFINE4(landlock_add_rule, const int, ruleset_fd,
  * namespace or is running with no_new_privs.  This avoids scenarios where
  * unprivileged tasks can affect the behavior of privileged children.
  *
- * Possible returned errors are:
+ * Return: 0 on success, or -errno on failure.  Possible returned errors are:
  *
  * - %EOPNOTSUPP: Landlock is supported by the kernel but disabled at boot time;
  * - %EINVAL: @flags contains an unknown bit.
diff --git a/security/landlock/task.c b/security/landlock/task.c
index 833bc0cfe5c9..bf7c3db7ce46 100644
--- a/security/landlock/task.c
+++ b/security/landlock/task.c
@@ -37,6 +37,9 @@
  *
  * Checks if the @parent domain is less or equal to (i.e. an ancestor, which
  * means a subset of) the @child domain.
+ *
+ * Return: True if @parent is an ancestor of or equal to @child, false
+ * otherwise.
  */
 static bool domain_scope_le(const struct landlock_ruleset *const parent,
 			    const struct landlock_ruleset *const child)
@@ -79,8 +82,7 @@ static int domain_ptrace(const struct landlock_ruleset *const parent,
  * If the current task has Landlock rules, then the child must have at least
  * the same rules.  Else denied.
  *
- * Determines whether a process may access another, returning 0 if permission
- * granted, -errno if denied.
+ * Return: 0 if permission is granted, -errno if denied.
  */
 static int hook_ptrace_access_check(struct task_struct *const child,
 				    const unsigned int mode)
@@ -129,8 +131,7 @@ static int hook_ptrace_access_check(struct task_struct *const child,
  * If the parent has Landlock rules, then the current task must have the same
  * or more rules.  Else denied.
  *
- * Determines whether the nominated task is permitted to trace the current
- * process, returning 0 if permission is granted, -errno if denied.
+ * Return: 0 if permission is granted, -errno if denied.
  */
 static int hook_ptrace_traceme(struct task_struct *const parent)
 {
-- 
2.53.0

[PATCH v1 3/4] landlock: Improve kernel-doc "Return:" section consistency

[Mickaël Salaün]

The canonical kernel-doc form is "Return:" (singular, without trailing
"s").  Normalize all existing "Returns:" occurrences across the Landlock
source tree to the canonical form.

Also fix capitalization for consistency.  Balance descriptions to
describe all possible returned values.

Consolidate bullet-point return descriptions into inline text for
functions with simple two-value or three-value returns for consistency.

Cc: Günther Noack <gnoack@google.com>
Signed-off-by: Mickaël Salaün <mic@digikod.net>
---
 security/landlock/cred.h    |  2 +-
 security/landlock/domain.c  |  4 ++--
 security/landlock/fs.c      | 26 +++++++++++---------------
 security/landlock/id.c      |  2 +-
 security/landlock/ruleset.c |  2 +-
 security/landlock/ruleset.h |  2 +-
 security/landlock/task.c    |  4 ++--
 security/landlock/tsync.c   | 17 ++++++-----------
 8 files changed, 25 insertions(+), 34 deletions(-)

diff --git a/security/landlock/cred.h b/security/landlock/cred.h
index c10a06727eb1..f287c56b5fd4 100644
--- a/security/landlock/cred.h
+++ b/security/landlock/cred.h
@@ -115,7 +115,7 @@ static inline bool landlocked(const struct task_struct *const task)
  * @handle_layer: returned youngest layer handling a subset of @masks.  Not set
  *                if the function returns NULL.
  *
- * Returns: landlock_cred(@cred) if any access rights specified in @masks is
+ * Return: landlock_cred(@cred) if any access rights specified in @masks is
  * handled, or NULL otherwise.
  */
 static inline const struct landlock_cred_security *
diff --git a/security/landlock/domain.c b/security/landlock/domain.c
index 343a1aabaac6..8b9939005aa8 100644
--- a/security/landlock/domain.c
+++ b/security/landlock/domain.c
@@ -34,7 +34,7 @@
  * @exe_size: Returned size of @exe_str (including the trailing null
  *            character), if any.
  *
- * Returns: A pointer to an allocated buffer where @exe_str point to, %NULL if
+ * Return: A pointer to an allocated buffer where @exe_str point to, %NULL if
  * there is no executable path, or an error otherwise.
  */
 static const void *get_current_exe(const char **const exe_str,
@@ -73,7 +73,7 @@ static const void *get_current_exe(const char **const exe_str,
 }
 
 /*
- * Returns: A newly allocated object describing a domain, or an error
+ * Return: A newly allocated object describing a domain, or an error
  * otherwise.
  */
 static struct landlock_details *get_current_details(void)
diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index cfe69075bf4e..a03ec664c78e 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -119,8 +119,8 @@ static const struct landlock_object_underops landlock_fs_underops = {
  * Any new IOCTL commands that are implemented in fs/ioctl.c's do_vfs_ioctl()
  * should be considered for inclusion here.
  *
- * Returns: true if the IOCTL @cmd can not be restricted with Landlock for
- * device files.
+ * Return: True if the IOCTL @cmd can not be restricted with Landlock for
+ * device files, false otherwise.
  */
 static __attribute_const__ bool is_masked_device_ioctl(const unsigned int cmd)
 {
@@ -428,10 +428,10 @@ static bool may_refer(const struct layer_access_masks *const src_parent,
  * Check that a destination file hierarchy has more restrictions than a source
  * file hierarchy.  This is only used for link and rename actions.
  *
- * Returns: true if child1 may be moved from parent1 to parent2 without
- * increasing its access rights.  If child2 is set, an additional condition is
+ * Return: True if child1 may be moved from parent1 to parent2 without
+ * increasing its access rights (if child2 is set, an additional condition is
  * that child2 may be used from parent2 to parent1 without increasing its access
- * rights.
+ * rights), false otherwise.
  */
 static bool no_more_access(const struct layer_access_masks *const parent1,
 			   const struct layer_access_masks *const child1,
@@ -734,9 +734,7 @@ static void test_is_eacces_with_write(struct kunit *const test)
  * checks that the collected accesses and the remaining ones are enough to
  * allow the request.
  *
- * Returns:
- * - true if the access request is granted;
- * - false otherwise.
+ * Return: True if the access request is granted, false otherwise.
  */
 static bool
 is_access_to_paths_allowed(const struct landlock_ruleset *const domain,
@@ -1022,9 +1020,8 @@ static access_mask_t maybe_remove(const struct dentry *const dentry)
  * only handles walking on the same mount point and only checks one set of
  * accesses.
  *
- * Returns:
- * - true if all the domain access rights are allowed for @dir;
- * - false if the walk reached @mnt_root.
+ * Return: True if all the domain access rights are allowed for @dir, false if
+ * the walk reached @mnt_root.
  */
 static bool collect_domain_accesses(const struct landlock_ruleset *const domain,
 				    const struct dentry *const mnt_root,
@@ -1120,10 +1117,9 @@ static bool collect_domain_accesses(const struct landlock_ruleset *const domain,
  * ephemeral matrices take some space on the stack, which limits the number of
  * layers to a deemed reasonable number: 16.
  *
- * Returns:
- * - 0 if access is allowed;
- * - -EXDEV if @old_dentry would inherit new access rights from @new_dir;
- * - -EACCES if file removal or creation is denied.
+ * Return: 0 if access is allowed, -EXDEV if @old_dentry would inherit new
+ * access rights from @new_dir, or -EACCES if file removal or creation is
+ * denied.
  */
 static int current_check_refer_path(struct dentry *const old_dentry,
 				    const struct path *const new_dir,
diff --git a/security/landlock/id.c b/security/landlock/id.c
index 838c3ed7bb82..6c8769777fdc 100644
--- a/security/landlock/id.c
+++ b/security/landlock/id.c
@@ -258,7 +258,7 @@ static void test_range2_rand16(struct kunit *const test)
  *
  * @number_of_ids: Number of IDs to hold.  Must be greater than one.
  *
- * Returns: The first ID in the range.
+ * Return: The first ID in the range.
  */
 u64 landlock_get_id_range(size_t number_of_ids)
 {
diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c
index de8386af2f30..52e48ffcc3aa 100644
--- a/security/landlock/ruleset.c
+++ b/security/landlock/ruleset.c
@@ -675,7 +675,7 @@ get_access_mask_t(const struct landlock_ruleset *const ruleset,
  * @masks: Layer access masks to populate.
  * @key_type: The key type to switch between access masks of different types.
  *
- * Returns: An access mask where each access right bit is set which is handled
+ * Return: An access mask where each access right bit is set which is handled
  * in any of the active layers in @domain.
  */
 access_mask_t
diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h
index 87d52031fb5a..5e63f78f7e1a 100644
--- a/security/landlock/ruleset.h
+++ b/security/landlock/ruleset.h
@@ -232,7 +232,7 @@ static inline void landlock_get_ruleset(struct landlock_ruleset *const ruleset)
  *
  * @domain: Landlock ruleset (used as a domain)
  *
- * Returns: an access_masks result of the OR of all the domain's access masks.
+ * Return: An access_masks result of the OR of all the domain's access masks.
  */
 static inline struct access_masks
 landlock_union_access_masks(const struct landlock_ruleset *const domain)
diff --git a/security/landlock/task.c b/security/landlock/task.c
index bf7c3db7ce46..f2dbdebf2770 100644
--- a/security/landlock/task.c
+++ b/security/landlock/task.c
@@ -174,8 +174,8 @@ static int hook_ptrace_traceme(struct task_struct *const parent)
  * @server: IPC receiver domain.
  * @scope: The scope restriction criteria.
  *
- * Returns: True if @server is in a different domain from @client, and @client
- * is scoped to access @server (i.e. access should be denied).
+ * Return: True if @server is in a different domain from @client and @client
+ * is scoped to access @server (i.e. access should be denied), false otherwise.
  */
 static bool domain_is_scoped(const struct landlock_ruleset *const client,
 			     const struct landlock_ruleset *const server,
diff --git a/security/landlock/tsync.c b/security/landlock/tsync.c
index b06a0fa4cedb..359aecbb1e4b 100644
--- a/security/landlock/tsync.c
+++ b/security/landlock/tsync.c
@@ -183,10 +183,8 @@ struct tsync_works {
  * capacity.  This can legitimately happen if new threads get started after we
  * grew the capacity.
  *
- * Returns:
- *   A pointer to the preallocated context struct, with task filled in.
- *
- *   NULL, if we ran out of preallocated context structs.
+ * Return: A pointer to the preallocated context struct with task filled in, or
+ * NULL if preallocated context structs ran out.
  */
 static struct tsync_work *tsync_works_provide(struct tsync_works *s,
 					      struct task_struct *task)
@@ -243,11 +241,8 @@ static void tsync_works_trim(struct tsync_works *s)
  * On a successful return, the subsequent n calls to tsync_works_provide() are
  * guaranteed to succeed.  (size + n <= capacity)
  *
- * Returns:
- *   -ENOMEM if the (re)allocation fails
-
- *   0       if the allocation succeeds, partially succeeds, or no reallocation
- *           was needed
+ * Return: 0 on success or partial success, -ENOMEM if the (re)allocation
+ * fails.
  */
 static int tsync_works_grow_by(struct tsync_works *s, size_t n, gfp_t flags)
 {
@@ -363,8 +358,8 @@ static size_t count_additional_threads(const struct tsync_works *works)
  * For each added task_work, atomically increments shared_ctx->num_preparing and
  * shared_ctx->num_unfinished.
  *
- * Returns:
- *     true, if at least one eligible sibling thread was found
+ * Return: True if at least one eligible sibling thread was found, false
+ * otherwise.
  */
 static bool schedule_task_work(struct tsync_works *works,
 			       struct tsync_shared_context *shared_ctx)
-- 
2.53.0

[PATCH v1 4/4] landlock: Fix formatting in tsync.c

[Mickaël Salaün]

Fix comment formatting in tsync.c to fit in 80 columns.

Cc: Günther Noack <gnoack@google.com>
Signed-off-by: Mickaël Salaün <mic@digikod.net>
---

My previous squashed fix was wrong.
---
 security/landlock/tsync.c | 121 +++++++++++++++++++++-----------------
 1 file changed, 66 insertions(+), 55 deletions(-)

diff --git a/security/landlock/tsync.c b/security/landlock/tsync.c
index 359aecbb1e4b..50445ae167dd 100644
--- a/security/landlock/tsync.c
+++ b/security/landlock/tsync.c
@@ -85,12 +85,14 @@ static void restrict_one_thread(struct tsync_shared_context *ctx)
 		/*
 		 * Switch out old_cred with new_cred, if possible.
 		 *
-		 * In the common case, where all threads initially point to the same
-		 * struct cred, this optimization avoids creating separate redundant
-		 * credentials objects for each, which would all have the same contents.
+		 * In the common case, where all threads initially point to the
+		 * same struct cred, this optimization avoids creating separate
+		 * redundant credentials objects for each, which would all have
+		 * the same contents.
 		 *
-		 * Note: We are intentionally dropping the const qualifier here, because
-		 * it is required by commit_creds() and abort_creds().
+		 * Note: We are intentionally dropping the const qualifier
+		 * here, because it is required by commit_creds() and
+		 * abort_creds().
 		 */
 		cred = (struct cred *)get_cred(ctx->new_cred);
 	} else {
@@ -101,8 +103,8 @@ static void restrict_one_thread(struct tsync_shared_context *ctx)
 			atomic_set(&ctx->preparation_error, -ENOMEM);
 
 			/*
-			 * Even on error, we need to adhere to the protocol and coordinate
-			 * with concurrently running invocations.
+			 * Even on error, we need to adhere to the protocol and
+			 * coordinate with concurrently running invocations.
 			 */
 			if (atomic_dec_return(&ctx->num_preparing) == 0)
 				complete_all(&ctx->all_prepared);
@@ -135,9 +137,9 @@ static void restrict_one_thread(struct tsync_shared_context *ctx)
 	}
 
 	/*
-	 * Make sure that all sibling tasks fulfill the no_new_privs prerequisite.
-	 * (This is in line with Seccomp's SECCOMP_FILTER_FLAG_TSYNC logic in
-	 * kernel/seccomp.c)
+	 * Make sure that all sibling tasks fulfill the no_new_privs
+	 * prerequisite.  (This is in line with Seccomp's
+	 * SECCOMP_FILTER_FLAG_TSYNC logic in kernel/seccomp.c)
 	 */
 	if (ctx->set_no_new_privs)
 		task_set_no_new_privs(current);
@@ -221,16 +223,17 @@ static void tsync_works_trim(struct tsync_works *s)
 	ctx = s->works[s->size - 1];
 
 	/*
-	 * For consistency, remove the task from ctx so that it does not look like
-	 * we handed it a task_work.
+	 * For consistency, remove the task from ctx so that it does not look
+	 * like we handed it a task_work.
 	 */
 	put_task_struct(ctx->task);
 	*ctx = (typeof(*ctx)){};
 
 	/*
-	 * Cancel the tsync_works_provide() change to recycle the reserved memory
-	 * for the next thread, if any.  This also ensures that cancel_tsync_works()
-	 * and tsync_works_release() do not see any NULL task pointers.
+	 * Cancel the tsync_works_provide() change to recycle the reserved
+	 * memory for the next thread, if any.  This also ensures that
+	 * cancel_tsync_works() and tsync_works_release() do not see any NULL
+	 * task pointers.
 	 */
 	s->size--;
 }
@@ -388,17 +391,17 @@ static bool schedule_task_work(struct tsync_works *works,
 			continue;
 
 		/*
-		 * We found a sibling thread that is not doing its task_work yet, and
-		 * which might spawn new threads before our task work runs, so we need
-		 * at least one more round in the outer loop.
+		 * We found a sibling thread that is not doing its task_work
+		 * yet, and which might spawn new threads before our task work
+		 * runs, so we need at least one more round in the outer loop.
 		 */
 		found_more_threads = true;
 
 		ctx = tsync_works_provide(works, thread);
 		if (!ctx) {
 			/*
-			 * We ran out of preallocated contexts -- we need to try again with
-			 * this thread at a later time!
+			 * We ran out of preallocated contexts -- we need to
+			 * try again with this thread at a later time!
 			 * found_more_threads is already true at this point.
 			 */
 			break;
@@ -413,10 +416,10 @@ static bool schedule_task_work(struct tsync_works *works,
 		err = task_work_add(thread, &ctx->work, TWA_SIGNAL);
 		if (unlikely(err)) {
 			/*
-			 * task_work_add() only fails if the task is about to exit.  We
-			 * checked that earlier, but it can happen as a race.  Resume
-			 * without setting an error, as the task is probably gone in the
-			 * next loop iteration.
+			 * task_work_add() only fails if the task is about to
+			 * exit.  We checked that earlier, but it can happen as
+			 * a race.  Resume without setting an error, as the
+			 * task is probably gone in the next loop iteration.
 			 */
 			tsync_works_trim(works);
 
@@ -497,24 +500,25 @@ int landlock_restrict_sibling_threads(const struct cred *old_cred,
 	 *    After this barrier is reached, it's safe to read
 	 *    shared_ctx.preparation_error.
 	 *
-	 * 4) reads shared_ctx.preparation_error and then either does commit_creds()
-	 *    or abort_creds().
+	 * 4) reads shared_ctx.preparation_error and then either does
+	 *    commit_creds() or abort_creds().
 	 *
 	 * 5) signals that it's done altogether (barrier synchronization
 	 *    "all_finished")
 	 *
-	 * Unlike seccomp, which modifies sibling tasks directly, we do not need to
-	 * acquire the cred_guard_mutex and sighand->siglock:
+	 * Unlike seccomp, which modifies sibling tasks directly, we do not
+	 * need to acquire the cred_guard_mutex and sighand->siglock:
 	 *
-	 * - As in our case, all threads are themselves exchanging their own struct
-	 *   cred through the credentials API, no locks are needed for that.
+	 * - As in our case, all threads are themselves exchanging their own
+	 *   struct cred through the credentials API, no locks are needed for
+	 *   that.
 	 * - Our for_each_thread() loops are protected by RCU.
-	 * - We do not acquire a lock to keep the list of sibling threads stable
-	 *   between our for_each_thread loops.  If the list of available sibling
-	 *   threads changes between these for_each_thread loops, we make up for
-	 *   that by continuing to look for threads until they are all discovered
-	 *   and have entered their task_work, where they are unable to spawn new
-	 *   threads.
+	 * - We do not acquire a lock to keep the list of sibling threads
+	 *   stable between our for_each_thread loops.  If the list of
+	 *   available sibling threads changes between these for_each_thread
+	 *   loops, we make up for that by continuing to look for threads until
+	 *   they are all discovered and have entered their task_work, where
+	 *   they are unable to spawn new threads.
 	 */
 	do {
 		/* In RCU read-lock, count the threads we need. */
@@ -531,43 +535,50 @@ int landlock_restrict_sibling_threads(const struct cred *old_cred,
 		}
 
 		/*
-		 * The "all_prepared" barrier is used locally to the loop body, this use
-		 * of for_each_thread().  We can reset it on each loop iteration because
-		 * all previous loop iterations are done with it already.
+		 * The "all_prepared" barrier is used locally to the loop body,
+		 * this use of for_each_thread().  We can reset it on each loop
+		 * iteration because all previous loop iterations are done with
+		 * it already.
 		 *
-		 * num_preparing is initialized to 1 so that the counter can not go to 0
-		 * and mark the completion as done before all task works are registered.
-		 * We decrement it at the end of the loop body.
+		 * num_preparing is initialized to 1 so that the counter can
+		 * not go to 0 and mark the completion as done before all task
+		 * works are registered.  We decrement it at the end of the
+		 * loop body.
 		 */
 		atomic_set(&shared_ctx.num_preparing, 1);
 		reinit_completion(&shared_ctx.all_prepared);
 
 		/*
-		 * In RCU read-lock, schedule task work on newly discovered sibling
-		 * tasks.
+		 * In RCU read-lock, schedule task work on newly discovered
+		 * sibling tasks.
 		 */
 		found_more_threads = schedule_task_work(&works, &shared_ctx);
 
 		/*
-		 * Decrement num_preparing for current, to undo that we initialized it
-		 * to 1 a few lines above.
+		 * Decrement num_preparing for current, to undo that we
+		 * initialized it to 1 a few lines above.
 		 */
 		if (atomic_dec_return(&shared_ctx.num_preparing) > 0) {
 			if (wait_for_completion_interruptible(
 				    &shared_ctx.all_prepared)) {
-				/* In case of interruption, we need to retry the system call. */
+				/*
+				 * In case of interruption, we need to retry
+				 * the system call.
+				 */
 				atomic_set(&shared_ctx.preparation_error,
 					   -ERESTARTNOINTR);
 
 				/*
-				 * Cancel task works for tasks that did not start running yet,
-				 * and decrement all_prepared and num_unfinished accordingly.
+				 * Cancel task works for tasks that did not
+				 * start running yet, and decrement
+				 * all_prepared and num_unfinished accordingly.
 				 */
 				cancel_tsync_works(&works, &shared_ctx);
 
 				/*
-				 * The remaining task works have started running, so waiting for
-				 * their completion will finish.
+				 * The remaining task works have started
+				 * running, so waiting for their completion
+				 * will finish.
 				 */
 				wait_for_completion(&shared_ctx.all_prepared);
 			}
@@ -576,14 +587,14 @@ int landlock_restrict_sibling_threads(const struct cred *old_cred,
 		 !atomic_read(&shared_ctx.preparation_error));
 
 	/*
-	 * We now have all sibling threads blocking and in "prepared" state in the
-	 * task work. Ask all threads to commit.
+	 * We now have all sibling threads blocking and in "prepared" state in
+	 * the task work. Ask all threads to commit.
 	 */
 	complete_all(&shared_ctx.ready_to_commit);
 
 	/*
-	 * Decrement num_unfinished for current, to undo that we initialized it to 1
-	 * at the beginning.
+	 * Decrement num_unfinished for current, to undo that we initialized it
+	 * to 1 at the beginning.
 	 */
 	if (atomic_dec_return(&shared_ctx.num_unfinished) > 0)
 		wait_for_completion(&shared_ctx.all_finished);
-- 
2.53.0

Re: [PATCH v1 4/4] landlock: Fix formatting in tsync.c

[Günther Noack]

On Wed, Mar 04, 2026 at 08:31:27PM +0100, Mickaël Salaün wrote:
> Fix comment formatting in tsync.c to fit in 80 columns.
> 
> Cc: Günther Noack <gnoack@google.com>
> Signed-off-by: Mickaël Salaün <mic@digikod.net>
> ---
> 
> My previous squashed fix was wrong.
> ---
>  security/landlock/tsync.c | 121 +++++++++++++++++++++-----------------
>  1 file changed, 66 insertions(+), 55 deletions(-)
> 
> diff --git a/security/landlock/tsync.c b/security/landlock/tsync.c
> index 359aecbb1e4b..50445ae167dd 100644
> --- a/security/landlock/tsync.c
> +++ b/security/landlock/tsync.c
> @@ -85,12 +85,14 @@ static void restrict_one_thread(struct tsync_shared_context *ctx)
>  		/*
>  		 * Switch out old_cred with new_cred, if possible.
>  		 *
> -		 * In the common case, where all threads initially point to the same
> -		 * struct cred, this optimization avoids creating separate redundant
> -		 * credentials objects for each, which would all have the same contents.
> +		 * In the common case, where all threads initially point to the
> +		 * same struct cred, this optimization avoids creating separate
> +		 * redundant credentials objects for each, which would all have
> +		 * the same contents.
>  		 *
> -		 * Note: We are intentionally dropping the const qualifier here, because
> -		 * it is required by commit_creds() and abort_creds().
> +		 * Note: We are intentionally dropping the const qualifier
> +		 * here, because it is required by commit_creds() and
> +		 * abort_creds().
>  		 */
>  		cred = (struct cred *)get_cred(ctx->new_cred);
>  	} else {
> @@ -101,8 +103,8 @@ static void restrict_one_thread(struct tsync_shared_context *ctx)
>  			atomic_set(&ctx->preparation_error, -ENOMEM);
>  
>  			/*
> -			 * Even on error, we need to adhere to the protocol and coordinate
> -			 * with concurrently running invocations.
> +			 * Even on error, we need to adhere to the protocol and
> +			 * coordinate with concurrently running invocations.
>  			 */
>  			if (atomic_dec_return(&ctx->num_preparing) == 0)
>  				complete_all(&ctx->all_prepared);
> @@ -135,9 +137,9 @@ static void restrict_one_thread(struct tsync_shared_context *ctx)
>  	}
>  
>  	/*
> -	 * Make sure that all sibling tasks fulfill the no_new_privs prerequisite.
> -	 * (This is in line with Seccomp's SECCOMP_FILTER_FLAG_TSYNC logic in
> -	 * kernel/seccomp.c)
> +	 * Make sure that all sibling tasks fulfill the no_new_privs
> +	 * prerequisite.  (This is in line with Seccomp's
> +	 * SECCOMP_FILTER_FLAG_TSYNC logic in kernel/seccomp.c)
>  	 */
>  	if (ctx->set_no_new_privs)
>  		task_set_no_new_privs(current);
> @@ -221,16 +223,17 @@ static void tsync_works_trim(struct tsync_works *s)
>  	ctx = s->works[s->size - 1];
>  
>  	/*
> -	 * For consistency, remove the task from ctx so that it does not look like
> -	 * we handed it a task_work.
> +	 * For consistency, remove the task from ctx so that it does not look
> +	 * like we handed it a task_work.
>  	 */
>  	put_task_struct(ctx->task);
>  	*ctx = (typeof(*ctx)){};
>  
>  	/*
> -	 * Cancel the tsync_works_provide() change to recycle the reserved memory
> -	 * for the next thread, if any.  This also ensures that cancel_tsync_works()
> -	 * and tsync_works_release() do not see any NULL task pointers.
> +	 * Cancel the tsync_works_provide() change to recycle the reserved
> +	 * memory for the next thread, if any.  This also ensures that
> +	 * cancel_tsync_works() and tsync_works_release() do not see any NULL
> +	 * task pointers.
>  	 */
>  	s->size--;
>  }
> @@ -388,17 +391,17 @@ static bool schedule_task_work(struct tsync_works *works,
>  			continue;
>  
>  		/*
> -		 * We found a sibling thread that is not doing its task_work yet, and
> -		 * which might spawn new threads before our task work runs, so we need
> -		 * at least one more round in the outer loop.
> +		 * We found a sibling thread that is not doing its task_work
> +		 * yet, and which might spawn new threads before our task work
> +		 * runs, so we need at least one more round in the outer loop.
>  		 */
>  		found_more_threads = true;
>  
>  		ctx = tsync_works_provide(works, thread);
>  		if (!ctx) {
>  			/*
> -			 * We ran out of preallocated contexts -- we need to try again with
> -			 * this thread at a later time!
> +			 * We ran out of preallocated contexts -- we need to
> +			 * try again with this thread at a later time!
>  			 * found_more_threads is already true at this point.
>  			 */
>  			break;
> @@ -413,10 +416,10 @@ static bool schedule_task_work(struct tsync_works *works,
>  		err = task_work_add(thread, &ctx->work, TWA_SIGNAL);
>  		if (unlikely(err)) {
>  			/*
> -			 * task_work_add() only fails if the task is about to exit.  We
> -			 * checked that earlier, but it can happen as a race.  Resume
> -			 * without setting an error, as the task is probably gone in the
> -			 * next loop iteration.
> +			 * task_work_add() only fails if the task is about to
> +			 * exit.  We checked that earlier, but it can happen as
> +			 * a race.  Resume without setting an error, as the
> +			 * task is probably gone in the next loop iteration.
>  			 */
>  			tsync_works_trim(works);
>  
> @@ -497,24 +500,25 @@ int landlock_restrict_sibling_threads(const struct cred *old_cred,
>  	 *    After this barrier is reached, it's safe to read
>  	 *    shared_ctx.preparation_error.
>  	 *
> -	 * 4) reads shared_ctx.preparation_error and then either does commit_creds()
> -	 *    or abort_creds().
> +	 * 4) reads shared_ctx.preparation_error and then either does
> +	 *    commit_creds() or abort_creds().
>  	 *
>  	 * 5) signals that it's done altogether (barrier synchronization
>  	 *    "all_finished")
>  	 *
> -	 * Unlike seccomp, which modifies sibling tasks directly, we do not need to
> -	 * acquire the cred_guard_mutex and sighand->siglock:
> +	 * Unlike seccomp, which modifies sibling tasks directly, we do not
> +	 * need to acquire the cred_guard_mutex and sighand->siglock:
>  	 *
> -	 * - As in our case, all threads are themselves exchanging their own struct
> -	 *   cred through the credentials API, no locks are needed for that.
> +	 * - As in our case, all threads are themselves exchanging their own
> +	 *   struct cred through the credentials API, no locks are needed for
> +	 *   that.
>  	 * - Our for_each_thread() loops are protected by RCU.
> -	 * - We do not acquire a lock to keep the list of sibling threads stable
> -	 *   between our for_each_thread loops.  If the list of available sibling
> -	 *   threads changes between these for_each_thread loops, we make up for
> -	 *   that by continuing to look for threads until they are all discovered
> -	 *   and have entered their task_work, where they are unable to spawn new
> -	 *   threads.
> +	 * - We do not acquire a lock to keep the list of sibling threads
> +	 *   stable between our for_each_thread loops.  If the list of
> +	 *   available sibling threads changes between these for_each_thread
> +	 *   loops, we make up for that by continuing to look for threads until
> +	 *   they are all discovered and have entered their task_work, where
> +	 *   they are unable to spawn new threads.
>  	 */
>  	do {
>  		/* In RCU read-lock, count the threads we need. */
> @@ -531,43 +535,50 @@ int landlock_restrict_sibling_threads(const struct cred *old_cred,
>  		}
>  
>  		/*
> -		 * The "all_prepared" barrier is used locally to the loop body, this use
> -		 * of for_each_thread().  We can reset it on each loop iteration because
> -		 * all previous loop iterations are done with it already.
> +		 * The "all_prepared" barrier is used locally to the loop body,
> +		 * this use of for_each_thread().  We can reset it on each loop
> +		 * iteration because all previous loop iterations are done with
> +		 * it already.
>  		 *
> -		 * num_preparing is initialized to 1 so that the counter can not go to 0
> -		 * and mark the completion as done before all task works are registered.
> -		 * We decrement it at the end of the loop body.
> +		 * num_preparing is initialized to 1 so that the counter can
> +		 * not go to 0 and mark the completion as done before all task
> +		 * works are registered.  We decrement it at the end of the
> +		 * loop body.
>  		 */
>  		atomic_set(&shared_ctx.num_preparing, 1);
>  		reinit_completion(&shared_ctx.all_prepared);
>  
>  		/*
> -		 * In RCU read-lock, schedule task work on newly discovered sibling
> -		 * tasks.
> +		 * In RCU read-lock, schedule task work on newly discovered
> +		 * sibling tasks.
>  		 */
>  		found_more_threads = schedule_task_work(&works, &shared_ctx);
>  
>  		/*
> -		 * Decrement num_preparing for current, to undo that we initialized it
> -		 * to 1 a few lines above.
> +		 * Decrement num_preparing for current, to undo that we
> +		 * initialized it to 1 a few lines above.
>  		 */
>  		if (atomic_dec_return(&shared_ctx.num_preparing) > 0) {
>  			if (wait_for_completion_interruptible(
>  				    &shared_ctx.all_prepared)) {
> -				/* In case of interruption, we need to retry the system call. */
> +				/*
> +				 * In case of interruption, we need to retry
> +				 * the system call.
> +				 */
>  				atomic_set(&shared_ctx.preparation_error,
>  					   -ERESTARTNOINTR);
>  
>  				/*
> -				 * Cancel task works for tasks that did not start running yet,
> -				 * and decrement all_prepared and num_unfinished accordingly.
> +				 * Cancel task works for tasks that did not
> +				 * start running yet, and decrement
> +				 * all_prepared and num_unfinished accordingly.
>  				 */
>  				cancel_tsync_works(&works, &shared_ctx);
>  
>  				/*
> -				 * The remaining task works have started running, so waiting for
> -				 * their completion will finish.
> +				 * The remaining task works have started
> +				 * running, so waiting for their completion
> +				 * will finish.
>  				 */
>  				wait_for_completion(&shared_ctx.all_prepared);
>  			}
> @@ -576,14 +587,14 @@ int landlock_restrict_sibling_threads(const struct cred *old_cred,
>  		 !atomic_read(&shared_ctx.preparation_error));
>  
>  	/*
> -	 * We now have all sibling threads blocking and in "prepared" state in the
> -	 * task work. Ask all threads to commit.
> +	 * We now have all sibling threads blocking and in "prepared" state in
> +	 * the task work. Ask all threads to commit.
>  	 */
>  	complete_all(&shared_ctx.ready_to_commit);
>  
>  	/*
> -	 * Decrement num_unfinished for current, to undo that we initialized it to 1
> -	 * at the beginning.
> +	 * Decrement num_unfinished for current, to undo that we initialized it
> +	 * to 1 at the beginning.
>  	 */
>  	if (atomic_dec_return(&shared_ctx.num_unfinished) > 0)
>  		wait_for_completion(&shared_ctx.all_finished);
> -- 
> 2.53.0
> 

Reviewed-by: Günther Noack <gnoack3000@gmail.com>

Thanks!  (It's irritating that the default clang-format configuration
does not fix these.  Do you use a special tool for this?)

–Günther

Re: [PATCH v1 3/4] landlock: Improve kernel-doc "Return:" section consistency

[Günther Noack]

Just a few comment phrasing nits below

On Wed, Mar 04, 2026 at 08:31:26PM +0100, Mickaël Salaün wrote:
> The canonical kernel-doc form is "Return:" (singular, without trailing
> "s").  Normalize all existing "Returns:" occurrences across the Landlock
> source tree to the canonical form.
> 
> Also fix capitalization for consistency.  Balance descriptions to
> describe all possible returned values.
> 
> Consolidate bullet-point return descriptions into inline text for
> functions with simple two-value or three-value returns for consistency.
> 
> Cc: Günther Noack <gnoack@google.com>
> Signed-off-by: Mickaël Salaün <mic@digikod.net>
> ---
>  security/landlock/cred.h    |  2 +-
>  security/landlock/domain.c  |  4 ++--
>  security/landlock/fs.c      | 26 +++++++++++---------------
>  security/landlock/id.c      |  2 +-
>  security/landlock/ruleset.c |  2 +-
>  security/landlock/ruleset.h |  2 +-
>  security/landlock/task.c    |  4 ++--
>  security/landlock/tsync.c   | 17 ++++++-----------
>  8 files changed, 25 insertions(+), 34 deletions(-)
> 
> diff --git a/security/landlock/cred.h b/security/landlock/cred.h
> index c10a06727eb1..f287c56b5fd4 100644
> --- a/security/landlock/cred.h
> +++ b/security/landlock/cred.h
> @@ -115,7 +115,7 @@ static inline bool landlocked(const struct task_struct *const task)
>   * @handle_layer: returned youngest layer handling a subset of @masks.  Not set
>   *                if the function returns NULL.
>   *
> - * Returns: landlock_cred(@cred) if any access rights specified in @masks is
> + * Return: landlock_cred(@cred) if any access rights specified in @masks is
>   * handled, or NULL otherwise.
>   */
>  static inline const struct landlock_cred_security *
> diff --git a/security/landlock/domain.c b/security/landlock/domain.c
> index 343a1aabaac6..8b9939005aa8 100644
> --- a/security/landlock/domain.c
> +++ b/security/landlock/domain.c
> @@ -34,7 +34,7 @@
>   * @exe_size: Returned size of @exe_str (including the trailing null
>   *            character), if any.
>   *
> - * Returns: A pointer to an allocated buffer where @exe_str point to, %NULL if
> + * Return: A pointer to an allocated buffer where @exe_str point to, %NULL if
>   * there is no executable path, or an error otherwise.
>   */
>  static const void *get_current_exe(const char **const exe_str,
> @@ -73,7 +73,7 @@ static const void *get_current_exe(const char **const exe_str,
>  }
>  
>  /*
> - * Returns: A newly allocated object describing a domain, or an error
> + * Return: A newly allocated object describing a domain, or an error
>   * otherwise.
>   */
>  static struct landlock_details *get_current_details(void)
> diff --git a/security/landlock/fs.c b/security/landlock/fs.c
> index cfe69075bf4e..a03ec664c78e 100644
> --- a/security/landlock/fs.c
> +++ b/security/landlock/fs.c
> @@ -119,8 +119,8 @@ static const struct landlock_object_underops landlock_fs_underops = {
>   * Any new IOCTL commands that are implemented in fs/ioctl.c's do_vfs_ioctl()
>   * should be considered for inclusion here.
>   *
> - * Returns: true if the IOCTL @cmd can not be restricted with Landlock for
> - * device files.
> + * Return: True if the IOCTL @cmd can not be restricted with Landlock for
> + * device files, false otherwise.
>   */
>  static __attribute_const__ bool is_masked_device_ioctl(const unsigned int cmd)
>  {
> @@ -428,10 +428,10 @@ static bool may_refer(const struct layer_access_masks *const src_parent,
>   * Check that a destination file hierarchy has more restrictions than a source
>   * file hierarchy.  This is only used for link and rename actions.
>   *
> - * Returns: true if child1 may be moved from parent1 to parent2 without
> - * increasing its access rights.  If child2 is set, an additional condition is
> + * Return: True if child1 may be moved from parent1 to parent2 without
> + * increasing its access rights (if child2 is set, an additional condition is
>   * that child2 may be used from parent2 to parent1 without increasing its access
> - * rights.
> + * rights), false otherwise.
>   */
>  static bool no_more_access(const struct layer_access_masks *const parent1,
>  			   const struct layer_access_masks *const child1,
> @@ -734,9 +734,7 @@ static void test_is_eacces_with_write(struct kunit *const test)
>   * checks that the collected accesses and the remaining ones are enough to
>   * allow the request.
>   *
> - * Returns:
> - * - true if the access request is granted;
> - * - false otherwise.
> + * Return: True if the access request is granted, false otherwise.
>   */
>  static bool
>  is_access_to_paths_allowed(const struct landlock_ruleset *const domain,
> @@ -1022,9 +1020,8 @@ static access_mask_t maybe_remove(const struct dentry *const dentry)
>   * only handles walking on the same mount point and only checks one set of
>   * accesses.
>   *
> - * Returns:
> - * - true if all the domain access rights are allowed for @dir;
> - * - false if the walk reached @mnt_root.
> + * Return: True if all the domain access rights are allowed for @dir, false if
> + * the walk reached @mnt_root.
>   */
>  static bool collect_domain_accesses(const struct landlock_ruleset *const domain,
>  				    const struct dentry *const mnt_root,
> @@ -1120,10 +1117,9 @@ static bool collect_domain_accesses(const struct landlock_ruleset *const domain,
>   * ephemeral matrices take some space on the stack, which limits the number of
>   * layers to a deemed reasonable number: 16.
>   *
> - * Returns:
> - * - 0 if access is allowed;
> - * - -EXDEV if @old_dentry would inherit new access rights from @new_dir;
> - * - -EACCES if file removal or creation is denied.
> + * Return: 0 if access is allowed, -EXDEV if @old_dentry would inherit new
> + * access rights from @new_dir, or -EACCES if file removal or creation is
> + * denied.
>   */
>  static int current_check_refer_path(struct dentry *const old_dentry,
>  				    const struct path *const new_dir,
> diff --git a/security/landlock/id.c b/security/landlock/id.c
> index 838c3ed7bb82..6c8769777fdc 100644
> --- a/security/landlock/id.c
> +++ b/security/landlock/id.c
> @@ -258,7 +258,7 @@ static void test_range2_rand16(struct kunit *const test)
>   *
>   * @number_of_ids: Number of IDs to hold.  Must be greater than one.
>   *
> - * Returns: The first ID in the range.
> + * Return: The first ID in the range.
>   */
>  u64 landlock_get_id_range(size_t number_of_ids)
>  {
> diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c
> index de8386af2f30..52e48ffcc3aa 100644
> --- a/security/landlock/ruleset.c
> +++ b/security/landlock/ruleset.c
> @@ -675,7 +675,7 @@ get_access_mask_t(const struct landlock_ruleset *const ruleset,
>   * @masks: Layer access masks to populate.
>   * @key_type: The key type to switch between access masks of different types.
>   *
> - * Returns: An access mask where each access right bit is set which is handled
> + * Return: An access mask where each access right bit is set which is handled
>   * in any of the active layers in @domain.
>   */
>  access_mask_t
> diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h
> index 87d52031fb5a..5e63f78f7e1a 100644
> --- a/security/landlock/ruleset.h
> +++ b/security/landlock/ruleset.h
> @@ -232,7 +232,7 @@ static inline void landlock_get_ruleset(struct landlock_ruleset *const ruleset)
>   *
>   * @domain: Landlock ruleset (used as a domain)
>   *
> - * Returns: an access_masks result of the OR of all the domain's access masks.
> + * Return: An access_masks result of the OR of all the domain's access masks.
>   */
>  static inline struct access_masks
>  landlock_union_access_masks(const struct landlock_ruleset *const domain)
> diff --git a/security/landlock/task.c b/security/landlock/task.c
> index bf7c3db7ce46..f2dbdebf2770 100644
> --- a/security/landlock/task.c
> +++ b/security/landlock/task.c
> @@ -174,8 +174,8 @@ static int hook_ptrace_traceme(struct task_struct *const parent)
>   * @server: IPC receiver domain.
>   * @scope: The scope restriction criteria.
>   *
> - * Returns: True if @server is in a different domain from @client, and @client
> - * is scoped to access @server (i.e. access should be denied).
> + * Return: True if @server is in a different domain from @client and @client
> + * is scoped to access @server (i.e. access should be denied), false otherwise.
>   */
>  static bool domain_is_scoped(const struct landlock_ruleset *const client,
>  			     const struct landlock_ruleset *const server,
> diff --git a/security/landlock/tsync.c b/security/landlock/tsync.c
> index b06a0fa4cedb..359aecbb1e4b 100644
> --- a/security/landlock/tsync.c
> +++ b/security/landlock/tsync.c
> @@ -183,10 +183,8 @@ struct tsync_works {
>   * capacity.  This can legitimately happen if new threads get started after we
>   * grew the capacity.
>   *
> - * Returns:
> - *   A pointer to the preallocated context struct, with task filled in.
> - *
> - *   NULL, if we ran out of preallocated context structs.
> + * Return: A pointer to the preallocated context struct with task filled in, or
> + * NULL if preallocated context structs ran out.
>   */
>  static struct tsync_work *tsync_works_provide(struct tsync_works *s,
>  					      struct task_struct *task)
> @@ -243,11 +241,8 @@ static void tsync_works_trim(struct tsync_works *s)
>   * On a successful return, the subsequent n calls to tsync_works_provide() are
>   * guaranteed to succeed.  (size + n <= capacity)
>   *
> - * Returns:
> - *   -ENOMEM if the (re)allocation fails
> -
> - *   0       if the allocation succeeds, partially succeeds, or no reallocation
> - *           was needed
> + * Return: 0 on success or partial success, -ENOMEM if the (re)allocation
> + * fails.

tsync_works_grow_by:

I don't know what I meant when I wrote "partially succeeds" here in
the original patch.  Would suggest this phrasing:

  Return: 0 if sufficient space for n more elements could be provided,
  -ENOMEM on allocation errors, -EOVERFLOW in case of integer
  overflow.

With this function, the success criterium is that it can establish
that invariant.  We also don't return a success if we only could
allocate space for fewer elements.

>   */
>  static int tsync_works_grow_by(struct tsync_works *s, size_t n, gfp_t flags)
>  {
> @@ -363,8 +358,8 @@ static size_t count_additional_threads(const struct tsync_works *works)
>   * For each added task_work, atomically increments shared_ctx->num_preparing and
>   * shared_ctx->num_unfinished.
>   *
> - * Returns:
> - *     true, if at least one eligible sibling thread was found
> + * Return: True if at least one eligible sibling thread was found, false
> + * otherwise.
>   */
>  static bool schedule_task_work(struct tsync_works *works,
>  			       struct tsync_shared_context *shared_ctx)
> -- 
> 2.53.0
> 

Reviewed-by: Günther Noack <gnoack3000@gmail.com>

Re: [PATCH v1 2/4] landlock: Add missing kernel-doc "Return:" sections

[Günther Noack]

On Wed, Mar 04, 2026 at 08:31:25PM +0100, Mickaël Salaün wrote:
> The kernel-doc -Wreturn check warns about functions with documentation
> comments that lack a "Return:" section.  Add "Return:" documentation to
> all functions missing it so that kernel-doc -Wreturn passes cleanly.
> 
> Convert existing function descriptions into a formal "Return:" section.
> Also fix the inaccurate return documentation for
> landlock_merge_ruleset() which claimed to return @parent directly, and
> document the previously missing ERR_PTR() error return path.  Document
> the ABI version and errata return paths for landlock_create_ruleset()
> which were previously only implied by the prose.
> 
> Cc: Günther Noack <gnoack@google.com>
> Signed-off-by: Mickaël Salaün <mic@digikod.net>
> ---
>  security/landlock/domain.c   |  2 ++
>  security/landlock/fs.c       |  2 +-
>  security/landlock/ruleset.c  |  8 +++++---
>  security/landlock/syscalls.c | 17 +++++++++++------
>  security/landlock/task.c     |  9 +++++----
>  5 files changed, 24 insertions(+), 14 deletions(-)
> 
> diff --git a/security/landlock/domain.c b/security/landlock/domain.c
> index 79cb3bbdf4c5..343a1aabaac6 100644
> --- a/security/landlock/domain.c
> +++ b/security/landlock/domain.c
> @@ -115,6 +115,8 @@ static struct landlock_details *get_current_details(void)
>   * restriction.  The subjective credentials must not be in an overridden state.
>   *
>   * @hierarchy->parent and @hierarchy->usage should already be set.
> + *
> + * Return: 0 on success, -errno on failure.
>   */
>  int landlock_init_hierarchy_log(struct landlock_hierarchy *const hierarchy)
>  {
> diff --git a/security/landlock/fs.c b/security/landlock/fs.c
> index e764470f588c..cfe69075bf4e 100644
> --- a/security/landlock/fs.c
> +++ b/security/landlock/fs.c
> @@ -1568,7 +1568,7 @@ static int hook_path_truncate(const struct path *const path)
>   *
>   * @file: File being opened.
>   *
> - * Returns the access rights that are required for opening the given file,
> + * Return: The access rights that are required for opening the given file,
>   * depending on the file type and open mode.
>   */
>  static access_mask_t
> diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c
> index a61ced492f41..de8386af2f30 100644
> --- a/security/landlock/ruleset.c
> +++ b/security/landlock/ruleset.c
> @@ -202,6 +202,8 @@ static void build_check_ruleset(void)
>   * When merging a ruleset in a domain, or copying a domain, @layers will be
>   * added to @ruleset as new constraints, similarly to a boolean AND between
>   * access rights.
> + *
> + * Return: 0 on success, -errno on failure.
>   */
>  static int insert_rule(struct landlock_ruleset *const ruleset,
>  		       const struct landlock_id id,
> @@ -531,8 +533,8 @@ void landlock_put_ruleset_deferred(struct landlock_ruleset *const ruleset)
>   * The current task is requesting to be restricted.  The subjective credentials
>   * must not be in an overridden state. cf. landlock_init_hierarchy_log().
>   *
> - * Returns the intersection of @parent and @ruleset, or returns @parent if
> - * @ruleset is empty, or returns a duplicate of @ruleset if @parent is empty.
> + * Return: A new domain merging @parent and @ruleset on success, or ERR_PTR()
> + * on failure.  If @parent is NULL, the new domain duplicates @ruleset.
>   */
>  struct landlock_ruleset *
>  landlock_merge_ruleset(struct landlock_ruleset *const parent,
> @@ -623,7 +625,7 @@ landlock_find_rule(const struct landlock_ruleset *const ruleset,
>   * @rule: A rule that grants a set of access rights for each layer
>   * @masks: A matrix of unfulfilled access rights for each layer
>   *
> - * Returns true if the request is allowed (i.e. the access rights granted all
> + * Return: True if the request is allowed (i.e. the access rights granted all
>   * remaining unfulfilled access rights and masks has no leftover set bits).
>   */
>  bool landlock_unmask_layers(const struct landlock_rule *const rule,
> diff --git a/security/landlock/syscalls.c b/security/landlock/syscalls.c
> index 0d66a68677b7..3b33839b80c7 100644
> --- a/security/landlock/syscalls.c
> +++ b/security/landlock/syscalls.c
> @@ -60,6 +60,8 @@ static bool is_initialized(void)
>   * @ksize_min: Minimal required size to be copied.
>   * @src: User space pointer or NULL.
>   * @usize: (Alleged) size of the data pointed to by @src.
> + *
> + * Return: 0 on success, -errno on failure.
>   */
>  static __always_inline int
>  copy_min_struct_from_user(void *const dst, const size_t ksize,
> @@ -178,16 +180,19 @@ const int landlock_abi_version = 8;
>   *         - %LANDLOCK_CREATE_RULESET_VERSION
>   *         - %LANDLOCK_CREATE_RULESET_ERRATA
>   *
> - * This system call enables to create a new Landlock ruleset, and returns the
> - * related file descriptor on success.
> + * This system call enables to create a new Landlock ruleset.
>   *
>   * If %LANDLOCK_CREATE_RULESET_VERSION or %LANDLOCK_CREATE_RULESET_ERRATA is
>   * set, then @attr must be NULL and @size must be 0.
>   *
> - * Possible returned errors are:
> + * Return: The ruleset file descriptor on success, the Landlock ABI version if
> + * %LANDLOCK_CREATE_RULESET_VERSION is set, the errata value if
> + * %LANDLOCK_CREATE_RULESET_ERRATA is set, or -errno on failure.  Possible
> + * returned errors are:
>   *
>   * - %EOPNOTSUPP: Landlock is supported by the kernel but disabled at boot time;
> - * - %EINVAL: unknown @flags, or unknown access, or unknown scope, or too small @size;
> + * - %EINVAL: unknown @flags, or unknown access, or unknown scope, or too small
> + *   @size;
>   * - %E2BIG: @attr or @size inconsistencies;
>   * - %EFAULT: @attr or @size inconsistencies;
>   * - %ENOMSG: empty &landlock_ruleset_attr.handled_access_fs.
> @@ -398,7 +403,7 @@ static int add_rule_net_port(struct landlock_ruleset *ruleset,
>   * This system call enables to define a new rule and add it to an existing
>   * ruleset.
>   *
> - * Possible returned errors are:
> + * Return: 0 on success, or -errno on failure.  Possible returned errors are:
>   *
>   * - %EOPNOTSUPP: Landlock is supported by the kernel but disabled at boot time;
>   * - %EAFNOSUPPORT: @rule_type is %LANDLOCK_RULE_NET_PORT but TCP/IP is not
> @@ -464,7 +469,7 @@ SYSCALL_DEFINE4(landlock_add_rule, const int, ruleset_fd,
>   * namespace or is running with no_new_privs.  This avoids scenarios where
>   * unprivileged tasks can affect the behavior of privileged children.
>   *
> - * Possible returned errors are:
> + * Return: 0 on success, or -errno on failure.  Possible returned errors are:
>   *
>   * - %EOPNOTSUPP: Landlock is supported by the kernel but disabled at boot time;
>   * - %EINVAL: @flags contains an unknown bit.
> diff --git a/security/landlock/task.c b/security/landlock/task.c
> index 833bc0cfe5c9..bf7c3db7ce46 100644
> --- a/security/landlock/task.c
> +++ b/security/landlock/task.c
> @@ -37,6 +37,9 @@
>   *
>   * Checks if the @parent domain is less or equal to (i.e. an ancestor, which
>   * means a subset of) the @child domain.
> + *
> + * Return: True if @parent is an ancestor of or equal to @child, false
> + * otherwise.
>   */
>  static bool domain_scope_le(const struct landlock_ruleset *const parent,
>  			    const struct landlock_ruleset *const child)
> @@ -79,8 +82,7 @@ static int domain_ptrace(const struct landlock_ruleset *const parent,
>   * If the current task has Landlock rules, then the child must have at least
>   * the same rules.  Else denied.
>   *
> - * Determines whether a process may access another, returning 0 if permission
> - * granted, -errno if denied.
> + * Return: 0 if permission is granted, -errno if denied.

Good simplification! (the removed text is already mentioned in the
short summary next to the function name).

>   */
>  static int hook_ptrace_access_check(struct task_struct *const child,
>  				    const unsigned int mode)
> @@ -129,8 +131,7 @@ static int hook_ptrace_access_check(struct task_struct *const child,
>   * If the parent has Landlock rules, then the current task must have the same
>   * or more rules.  Else denied.
>   *
> - * Determines whether the nominated task is permitted to trace the current
> - * process, returning 0 if permission is granted, -errno if denied.
> + * Return: 0 if permission is granted, -errno if denied.

Ditto.

>   */
>  static int hook_ptrace_traceme(struct task_struct *const parent)
>  {
> -- 
> 2.53.0
> 

Looks good, thanks!

Reviewed-by: Günther Noack <gnoack3000@gmail.com>

Re: [PATCH v1 3/4] landlock: Improve kernel-doc "Return:" section consistency

[Mickaël Salaün]

On Fri, Mar 06, 2026 at 08:30:29AM +0100, Günther Noack wrote:
> Just a few comment phrasing nits below
> 
> On Wed, Mar 04, 2026 at 08:31:26PM +0100, Mickaël Salaün wrote:
> > The canonical kernel-doc form is "Return:" (singular, without trailing
> > "s").  Normalize all existing "Returns:" occurrences across the Landlock
> > source tree to the canonical form.
> > 
> > Also fix capitalization for consistency.  Balance descriptions to
> > describe all possible returned values.
> > 
> > Consolidate bullet-point return descriptions into inline text for
> > functions with simple two-value or three-value returns for consistency.
> > 
> > Cc: Günther Noack <gnoack@google.com>
> > Signed-off-by: Mickaël Salaün <mic@digikod.net>
> > ---
> >  security/landlock/cred.h    |  2 +-
> >  security/landlock/domain.c  |  4 ++--
> >  security/landlock/fs.c      | 26 +++++++++++---------------
> >  security/landlock/id.c      |  2 +-
> >  security/landlock/ruleset.c |  2 +-
> >  security/landlock/ruleset.h |  2 +-
> >  security/landlock/task.c    |  4 ++--
> >  security/landlock/tsync.c   | 17 ++++++-----------
> >  8 files changed, 25 insertions(+), 34 deletions(-)
> > 
> > diff --git a/security/landlock/cred.h b/security/landlock/cred.h
> > index c10a06727eb1..f287c56b5fd4 100644
> > --- a/security/landlock/cred.h
> > +++ b/security/landlock/cred.h
> > @@ -115,7 +115,7 @@ static inline bool landlocked(const struct task_struct *const task)
> >   * @handle_layer: returned youngest layer handling a subset of @masks.  Not set
> >   *                if the function returns NULL.
> >   *
> > - * Returns: landlock_cred(@cred) if any access rights specified in @masks is
> > + * Return: landlock_cred(@cred) if any access rights specified in @masks is
> >   * handled, or NULL otherwise.
> >   */
> >  static inline const struct landlock_cred_security *
> > diff --git a/security/landlock/domain.c b/security/landlock/domain.c
> > index 343a1aabaac6..8b9939005aa8 100644
> > --- a/security/landlock/domain.c
> > +++ b/security/landlock/domain.c
> > @@ -34,7 +34,7 @@
> >   * @exe_size: Returned size of @exe_str (including the trailing null
> >   *            character), if any.
> >   *
> > - * Returns: A pointer to an allocated buffer where @exe_str point to, %NULL if
> > + * Return: A pointer to an allocated buffer where @exe_str point to, %NULL if
> >   * there is no executable path, or an error otherwise.
> >   */
> >  static const void *get_current_exe(const char **const exe_str,
> > @@ -73,7 +73,7 @@ static const void *get_current_exe(const char **const exe_str,
> >  }
> >  
> >  /*
> > - * Returns: A newly allocated object describing a domain, or an error
> > + * Return: A newly allocated object describing a domain, or an error
> >   * otherwise.
> >   */
> >  static struct landlock_details *get_current_details(void)
> > diff --git a/security/landlock/fs.c b/security/landlock/fs.c
> > index cfe69075bf4e..a03ec664c78e 100644
> > --- a/security/landlock/fs.c
> > +++ b/security/landlock/fs.c
> > @@ -119,8 +119,8 @@ static const struct landlock_object_underops landlock_fs_underops = {
> >   * Any new IOCTL commands that are implemented in fs/ioctl.c's do_vfs_ioctl()
> >   * should be considered for inclusion here.
> >   *
> > - * Returns: true if the IOCTL @cmd can not be restricted with Landlock for
> > - * device files.
> > + * Return: True if the IOCTL @cmd can not be restricted with Landlock for
> > + * device files, false otherwise.
> >   */
> >  static __attribute_const__ bool is_masked_device_ioctl(const unsigned int cmd)
> >  {
> > @@ -428,10 +428,10 @@ static bool may_refer(const struct layer_access_masks *const src_parent,
> >   * Check that a destination file hierarchy has more restrictions than a source
> >   * file hierarchy.  This is only used for link and rename actions.
> >   *
> > - * Returns: true if child1 may be moved from parent1 to parent2 without
> > - * increasing its access rights.  If child2 is set, an additional condition is
> > + * Return: True if child1 may be moved from parent1 to parent2 without
> > + * increasing its access rights (if child2 is set, an additional condition is
> >   * that child2 may be used from parent2 to parent1 without increasing its access
> > - * rights.
> > + * rights), false otherwise.
> >   */
> >  static bool no_more_access(const struct layer_access_masks *const parent1,
> >  			   const struct layer_access_masks *const child1,
> > @@ -734,9 +734,7 @@ static void test_is_eacces_with_write(struct kunit *const test)
> >   * checks that the collected accesses and the remaining ones are enough to
> >   * allow the request.
> >   *
> > - * Returns:
> > - * - true if the access request is granted;
> > - * - false otherwise.
> > + * Return: True if the access request is granted, false otherwise.
> >   */
> >  static bool
> >  is_access_to_paths_allowed(const struct landlock_ruleset *const domain,
> > @@ -1022,9 +1020,8 @@ static access_mask_t maybe_remove(const struct dentry *const dentry)
> >   * only handles walking on the same mount point and only checks one set of
> >   * accesses.
> >   *
> > - * Returns:
> > - * - true if all the domain access rights are allowed for @dir;
> > - * - false if the walk reached @mnt_root.
> > + * Return: True if all the domain access rights are allowed for @dir, false if
> > + * the walk reached @mnt_root.
> >   */
> >  static bool collect_domain_accesses(const struct landlock_ruleset *const domain,
> >  				    const struct dentry *const mnt_root,
> > @@ -1120,10 +1117,9 @@ static bool collect_domain_accesses(const struct landlock_ruleset *const domain,
> >   * ephemeral matrices take some space on the stack, which limits the number of
> >   * layers to a deemed reasonable number: 16.
> >   *
> > - * Returns:
> > - * - 0 if access is allowed;
> > - * - -EXDEV if @old_dentry would inherit new access rights from @new_dir;
> > - * - -EACCES if file removal or creation is denied.
> > + * Return: 0 if access is allowed, -EXDEV if @old_dentry would inherit new
> > + * access rights from @new_dir, or -EACCES if file removal or creation is
> > + * denied.
> >   */
> >  static int current_check_refer_path(struct dentry *const old_dentry,
> >  				    const struct path *const new_dir,
> > diff --git a/security/landlock/id.c b/security/landlock/id.c
> > index 838c3ed7bb82..6c8769777fdc 100644
> > --- a/security/landlock/id.c
> > +++ b/security/landlock/id.c
> > @@ -258,7 +258,7 @@ static void test_range2_rand16(struct kunit *const test)
> >   *
> >   * @number_of_ids: Number of IDs to hold.  Must be greater than one.
> >   *
> > - * Returns: The first ID in the range.
> > + * Return: The first ID in the range.
> >   */
> >  u64 landlock_get_id_range(size_t number_of_ids)
> >  {
> > diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c
> > index de8386af2f30..52e48ffcc3aa 100644
> > --- a/security/landlock/ruleset.c
> > +++ b/security/landlock/ruleset.c
> > @@ -675,7 +675,7 @@ get_access_mask_t(const struct landlock_ruleset *const ruleset,
> >   * @masks: Layer access masks to populate.
> >   * @key_type: The key type to switch between access masks of different types.
> >   *
> > - * Returns: An access mask where each access right bit is set which is handled
> > + * Return: An access mask where each access right bit is set which is handled
> >   * in any of the active layers in @domain.
> >   */
> >  access_mask_t
> > diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h
> > index 87d52031fb5a..5e63f78f7e1a 100644
> > --- a/security/landlock/ruleset.h
> > +++ b/security/landlock/ruleset.h
> > @@ -232,7 +232,7 @@ static inline void landlock_get_ruleset(struct landlock_ruleset *const ruleset)
> >   *
> >   * @domain: Landlock ruleset (used as a domain)
> >   *
> > - * Returns: an access_masks result of the OR of all the domain's access masks.
> > + * Return: An access_masks result of the OR of all the domain's access masks.
> >   */
> >  static inline struct access_masks
> >  landlock_union_access_masks(const struct landlock_ruleset *const domain)
> > diff --git a/security/landlock/task.c b/security/landlock/task.c
> > index bf7c3db7ce46..f2dbdebf2770 100644
> > --- a/security/landlock/task.c
> > +++ b/security/landlock/task.c
> > @@ -174,8 +174,8 @@ static int hook_ptrace_traceme(struct task_struct *const parent)
> >   * @server: IPC receiver domain.
> >   * @scope: The scope restriction criteria.
> >   *
> > - * Returns: True if @server is in a different domain from @client, and @client
> > - * is scoped to access @server (i.e. access should be denied).
> > + * Return: True if @server is in a different domain from @client and @client
> > + * is scoped to access @server (i.e. access should be denied), false otherwise.
> >   */
> >  static bool domain_is_scoped(const struct landlock_ruleset *const client,
> >  			     const struct landlock_ruleset *const server,
> > diff --git a/security/landlock/tsync.c b/security/landlock/tsync.c
> > index b06a0fa4cedb..359aecbb1e4b 100644
> > --- a/security/landlock/tsync.c
> > +++ b/security/landlock/tsync.c
> > @@ -183,10 +183,8 @@ struct tsync_works {
> >   * capacity.  This can legitimately happen if new threads get started after we
> >   * grew the capacity.
> >   *
> > - * Returns:
> > - *   A pointer to the preallocated context struct, with task filled in.
> > - *
> > - *   NULL, if we ran out of preallocated context structs.
> > + * Return: A pointer to the preallocated context struct with task filled in, or
> > + * NULL if preallocated context structs ran out.
> >   */
> >  static struct tsync_work *tsync_works_provide(struct tsync_works *s,
> >  					      struct task_struct *task)
> > @@ -243,11 +241,8 @@ static void tsync_works_trim(struct tsync_works *s)
> >   * On a successful return, the subsequent n calls to tsync_works_provide() are
> >   * guaranteed to succeed.  (size + n <= capacity)
> >   *
> > - * Returns:
> > - *   -ENOMEM if the (re)allocation fails
> > -
> > - *   0       if the allocation succeeds, partially succeeds, or no reallocation
> > - *           was needed
> > + * Return: 0 on success or partial success, -ENOMEM if the (re)allocation
> > + * fails.
> 
> tsync_works_grow_by:
> 
> I don't know what I meant when I wrote "partially succeeds" here in
> the original patch.  Would suggest this phrasing:
> 
>   Return: 0 if sufficient space for n more elements could be provided,
>   -ENOMEM on allocation errors, -EOVERFLOW in case of integer
>   overflow.
> 
> With this function, the success criterium is that it can establish
> that invariant.  We also don't return a success if we only could
> allocate space for fewer elements.

Good, I'll fold this in the commit.

> 
> >   */
> >  static int tsync_works_grow_by(struct tsync_works *s, size_t n, gfp_t flags)
> >  {
> > @@ -363,8 +358,8 @@ static size_t count_additional_threads(const struct tsync_works *works)
> >   * For each added task_work, atomically increments shared_ctx->num_preparing and
> >   * shared_ctx->num_unfinished.
> >   *
> > - * Returns:
> > - *     true, if at least one eligible sibling thread was found
> > + * Return: True if at least one eligible sibling thread was found, false
> > + * otherwise.
> >   */
> >  static bool schedule_task_work(struct tsync_works *works,
> >  			       struct tsync_shared_context *shared_ctx)
> > -- 
> > 2.53.0
> > 
> 
> Reviewed-by: Günther Noack <gnoack3000@gmail.com>
> 

Re: [PATCH v1 4/4] landlock: Fix formatting in tsync.c

[Mickaël Salaün]

On Fri, Mar 06, 2026 at 08:13:59AM +0100, Günther Noack wrote:
> On Wed, Mar 04, 2026 at 08:31:27PM +0100, Mickaël Salaün wrote:
> > Fix comment formatting in tsync.c to fit in 80 columns.
> > 
> > Cc: Günther Noack <gnoack@google.com>
> > Signed-off-by: Mickaël Salaün <mic@digikod.net>
> > ---
> > 
> > My previous squashed fix was wrong.
> > ---
> >  security/landlock/tsync.c | 121 +++++++++++++++++++++-----------------
> >  1 file changed, 66 insertions(+), 55 deletions(-)
> > 
> > diff --git a/security/landlock/tsync.c b/security/landlock/tsync.c
> > index 359aecbb1e4b..50445ae167dd 100644
> > --- a/security/landlock/tsync.c
> > +++ b/security/landlock/tsync.c
> > @@ -85,12 +85,14 @@ static void restrict_one_thread(struct tsync_shared_context *ctx)
> >  		/*
> >  		 * Switch out old_cred with new_cred, if possible.
> >  		 *
> > -		 * In the common case, where all threads initially point to the same
> > -		 * struct cred, this optimization avoids creating separate redundant
> > -		 * credentials objects for each, which would all have the same contents.
> > +		 * In the common case, where all threads initially point to the
> > +		 * same struct cred, this optimization avoids creating separate
> > +		 * redundant credentials objects for each, which would all have
> > +		 * the same contents.
> >  		 *
> > -		 * Note: We are intentionally dropping the const qualifier here, because
> > -		 * it is required by commit_creds() and abort_creds().
> > +		 * Note: We are intentionally dropping the const qualifier
> > +		 * here, because it is required by commit_creds() and
> > +		 * abort_creds().
> >  		 */
> >  		cred = (struct cred *)get_cred(ctx->new_cred);
> >  	} else {
> > @@ -101,8 +103,8 @@ static void restrict_one_thread(struct tsync_shared_context *ctx)
> >  			atomic_set(&ctx->preparation_error, -ENOMEM);
> >  
> >  			/*
> > -			 * Even on error, we need to adhere to the protocol and coordinate
> > -			 * with concurrently running invocations.
> > +			 * Even on error, we need to adhere to the protocol and
> > +			 * coordinate with concurrently running invocations.
> >  			 */
> >  			if (atomic_dec_return(&ctx->num_preparing) == 0)
> >  				complete_all(&ctx->all_prepared);
> > @@ -135,9 +137,9 @@ static void restrict_one_thread(struct tsync_shared_context *ctx)
> >  	}
> >  
> >  	/*
> > -	 * Make sure that all sibling tasks fulfill the no_new_privs prerequisite.
> > -	 * (This is in line with Seccomp's SECCOMP_FILTER_FLAG_TSYNC logic in
> > -	 * kernel/seccomp.c)
> > +	 * Make sure that all sibling tasks fulfill the no_new_privs
> > +	 * prerequisite.  (This is in line with Seccomp's
> > +	 * SECCOMP_FILTER_FLAG_TSYNC logic in kernel/seccomp.c)
> >  	 */
> >  	if (ctx->set_no_new_privs)
> >  		task_set_no_new_privs(current);
> > @@ -221,16 +223,17 @@ static void tsync_works_trim(struct tsync_works *s)
> >  	ctx = s->works[s->size - 1];
> >  
> >  	/*
> > -	 * For consistency, remove the task from ctx so that it does not look like
> > -	 * we handed it a task_work.
> > +	 * For consistency, remove the task from ctx so that it does not look
> > +	 * like we handed it a task_work.
> >  	 */
> >  	put_task_struct(ctx->task);
> >  	*ctx = (typeof(*ctx)){};
> >  
> >  	/*
> > -	 * Cancel the tsync_works_provide() change to recycle the reserved memory
> > -	 * for the next thread, if any.  This also ensures that cancel_tsync_works()
> > -	 * and tsync_works_release() do not see any NULL task pointers.
> > +	 * Cancel the tsync_works_provide() change to recycle the reserved
> > +	 * memory for the next thread, if any.  This also ensures that
> > +	 * cancel_tsync_works() and tsync_works_release() do not see any NULL
> > +	 * task pointers.
> >  	 */
> >  	s->size--;
> >  }
> > @@ -388,17 +391,17 @@ static bool schedule_task_work(struct tsync_works *works,
> >  			continue;
> >  
> >  		/*
> > -		 * We found a sibling thread that is not doing its task_work yet, and
> > -		 * which might spawn new threads before our task work runs, so we need
> > -		 * at least one more round in the outer loop.
> > +		 * We found a sibling thread that is not doing its task_work
> > +		 * yet, and which might spawn new threads before our task work
> > +		 * runs, so we need at least one more round in the outer loop.
> >  		 */
> >  		found_more_threads = true;
> >  
> >  		ctx = tsync_works_provide(works, thread);
> >  		if (!ctx) {
> >  			/*
> > -			 * We ran out of preallocated contexts -- we need to try again with
> > -			 * this thread at a later time!
> > +			 * We ran out of preallocated contexts -- we need to
> > +			 * try again with this thread at a later time!
> >  			 * found_more_threads is already true at this point.
> >  			 */
> >  			break;
> > @@ -413,10 +416,10 @@ static bool schedule_task_work(struct tsync_works *works,
> >  		err = task_work_add(thread, &ctx->work, TWA_SIGNAL);
> >  		if (unlikely(err)) {
> >  			/*
> > -			 * task_work_add() only fails if the task is about to exit.  We
> > -			 * checked that earlier, but it can happen as a race.  Resume
> > -			 * without setting an error, as the task is probably gone in the
> > -			 * next loop iteration.
> > +			 * task_work_add() only fails if the task is about to
> > +			 * exit.  We checked that earlier, but it can happen as
> > +			 * a race.  Resume without setting an error, as the
> > +			 * task is probably gone in the next loop iteration.
> >  			 */
> >  			tsync_works_trim(works);
> >  
> > @@ -497,24 +500,25 @@ int landlock_restrict_sibling_threads(const struct cred *old_cred,
> >  	 *    After this barrier is reached, it's safe to read
> >  	 *    shared_ctx.preparation_error.
> >  	 *
> > -	 * 4) reads shared_ctx.preparation_error and then either does commit_creds()
> > -	 *    or abort_creds().
> > +	 * 4) reads shared_ctx.preparation_error and then either does
> > +	 *    commit_creds() or abort_creds().
> >  	 *
> >  	 * 5) signals that it's done altogether (barrier synchronization
> >  	 *    "all_finished")
> >  	 *
> > -	 * Unlike seccomp, which modifies sibling tasks directly, we do not need to
> > -	 * acquire the cred_guard_mutex and sighand->siglock:
> > +	 * Unlike seccomp, which modifies sibling tasks directly, we do not
> > +	 * need to acquire the cred_guard_mutex and sighand->siglock:
> >  	 *
> > -	 * - As in our case, all threads are themselves exchanging their own struct
> > -	 *   cred through the credentials API, no locks are needed for that.
> > +	 * - As in our case, all threads are themselves exchanging their own
> > +	 *   struct cred through the credentials API, no locks are needed for
> > +	 *   that.
> >  	 * - Our for_each_thread() loops are protected by RCU.
> > -	 * - We do not acquire a lock to keep the list of sibling threads stable
> > -	 *   between our for_each_thread loops.  If the list of available sibling
> > -	 *   threads changes between these for_each_thread loops, we make up for
> > -	 *   that by continuing to look for threads until they are all discovered
> > -	 *   and have entered their task_work, where they are unable to spawn new
> > -	 *   threads.
> > +	 * - We do not acquire a lock to keep the list of sibling threads
> > +	 *   stable between our for_each_thread loops.  If the list of
> > +	 *   available sibling threads changes between these for_each_thread
> > +	 *   loops, we make up for that by continuing to look for threads until
> > +	 *   they are all discovered and have entered their task_work, where
> > +	 *   they are unable to spawn new threads.
> >  	 */
> >  	do {
> >  		/* In RCU read-lock, count the threads we need. */
> > @@ -531,43 +535,50 @@ int landlock_restrict_sibling_threads(const struct cred *old_cred,
> >  		}
> >  
> >  		/*
> > -		 * The "all_prepared" barrier is used locally to the loop body, this use
> > -		 * of for_each_thread().  We can reset it on each loop iteration because
> > -		 * all previous loop iterations are done with it already.
> > +		 * The "all_prepared" barrier is used locally to the loop body,
> > +		 * this use of for_each_thread().  We can reset it on each loop
> > +		 * iteration because all previous loop iterations are done with
> > +		 * it already.
> >  		 *
> > -		 * num_preparing is initialized to 1 so that the counter can not go to 0
> > -		 * and mark the completion as done before all task works are registered.
> > -		 * We decrement it at the end of the loop body.
> > +		 * num_preparing is initialized to 1 so that the counter can
> > +		 * not go to 0 and mark the completion as done before all task
> > +		 * works are registered.  We decrement it at the end of the
> > +		 * loop body.
> >  		 */
> >  		atomic_set(&shared_ctx.num_preparing, 1);
> >  		reinit_completion(&shared_ctx.all_prepared);
> >  
> >  		/*
> > -		 * In RCU read-lock, schedule task work on newly discovered sibling
> > -		 * tasks.
> > +		 * In RCU read-lock, schedule task work on newly discovered
> > +		 * sibling tasks.
> >  		 */
> >  		found_more_threads = schedule_task_work(&works, &shared_ctx);
> >  
> >  		/*
> > -		 * Decrement num_preparing for current, to undo that we initialized it
> > -		 * to 1 a few lines above.
> > +		 * Decrement num_preparing for current, to undo that we
> > +		 * initialized it to 1 a few lines above.
> >  		 */
> >  		if (atomic_dec_return(&shared_ctx.num_preparing) > 0) {
> >  			if (wait_for_completion_interruptible(
> >  				    &shared_ctx.all_prepared)) {
> > -				/* In case of interruption, we need to retry the system call. */
> > +				/*
> > +				 * In case of interruption, we need to retry
> > +				 * the system call.
> > +				 */
> >  				atomic_set(&shared_ctx.preparation_error,
> >  					   -ERESTARTNOINTR);
> >  
> >  				/*
> > -				 * Cancel task works for tasks that did not start running yet,
> > -				 * and decrement all_prepared and num_unfinished accordingly.
> > +				 * Cancel task works for tasks that did not
> > +				 * start running yet, and decrement
> > +				 * all_prepared and num_unfinished accordingly.
> >  				 */
> >  				cancel_tsync_works(&works, &shared_ctx);
> >  
> >  				/*
> > -				 * The remaining task works have started running, so waiting for
> > -				 * their completion will finish.
> > +				 * The remaining task works have started
> > +				 * running, so waiting for their completion
> > +				 * will finish.
> >  				 */
> >  				wait_for_completion(&shared_ctx.all_prepared);
> >  			}
> > @@ -576,14 +587,14 @@ int landlock_restrict_sibling_threads(const struct cred *old_cred,
> >  		 !atomic_read(&shared_ctx.preparation_error));
> >  
> >  	/*
> > -	 * We now have all sibling threads blocking and in "prepared" state in the
> > -	 * task work. Ask all threads to commit.
> > +	 * We now have all sibling threads blocking and in "prepared" state in
> > +	 * the task work. Ask all threads to commit.
> >  	 */
> >  	complete_all(&shared_ctx.ready_to_commit);
> >  
> >  	/*
> > -	 * Decrement num_unfinished for current, to undo that we initialized it to 1
> > -	 * at the beginning.
> > +	 * Decrement num_unfinished for current, to undo that we initialized it
> > +	 * to 1 at the beginning.
> >  	 */
> >  	if (atomic_dec_return(&shared_ctx.num_unfinished) > 0)
> >  		wait_for_completion(&shared_ctx.all_finished);
> > -- 
> > 2.53.0
> > 
> 
> Reviewed-by: Günther Noack <gnoack3000@gmail.com>
> 
> Thanks!  (It's irritating that the default clang-format configuration
> does not fix these.  Do you use a special tool for this?)

I mostly use vim's colorcolumn but I should update check-linux.sh with
such check.

I guess you're also OK with the first patch?

> 
> –Günther
> 

Re: [PATCH v1 1/4] landlock: Fix kernel-doc warning for pointer-to-array parameters

[Günther Noack]

On Wed, Mar 04, 2026 at 08:31:24PM +0100, Mickaël Salaün wrote:
> The insert_rule() and create_rule() functions take a
> pointer-to-flexible-array parameter declared as:
> 
>   const struct landlock_layer (*const layers)[]
> 
> The kernel-doc parser cannot handle a qualifier between * and the
> parameter name in this syntax, producing spurious "Invalid param" and
> "not described" warnings.
> 
> Introduce landlock_layer_array_t as a typedef for the flexible array
> type so the parameter can be written as:
> 
>   const landlock_layer_array_t *const layers
> 
> This is the same type but kernel-doc parses it correctly, while
> preserving the pointer-to-array type safety that prevents callers from
> accidentally passing a pointer to a single element.
> 
> Cc: Günther Noack <gnoack@google.com>
> Signed-off-by: Mickaël Salaün <mic@digikod.net>
> ---
>  security/landlock/ruleset.c | 4 ++--
>  security/landlock/ruleset.h | 8 ++++++++
>  2 files changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c
> index 419b237de635..a61ced492f41 100644
> --- a/security/landlock/ruleset.c
> +++ b/security/landlock/ruleset.c
> @@ -108,7 +108,7 @@ static bool is_object_pointer(const enum landlock_key_type key_type)
>  
>  static struct landlock_rule *
>  create_rule(const struct landlock_id id,
> -	    const struct landlock_layer (*const layers)[], const u32 num_layers,
> +	    const landlock_layer_array_t *const layers, const u32 num_layers,
>  	    const struct landlock_layer *const new_layer)
>  {
>  	struct landlock_rule *new_rule;
> @@ -205,7 +205,7 @@ static void build_check_ruleset(void)
>   */
>  static int insert_rule(struct landlock_ruleset *const ruleset,
>  		       const struct landlock_id id,
> -		       const struct landlock_layer (*const layers)[],
> +		       const landlock_layer_array_t *const layers,
>  		       const size_t num_layers)
>  {
>  	struct rb_node **walker_node;
> diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h
> index 9d6dc632684c..87d52031fb5a 100644
> --- a/security/landlock/ruleset.h
> +++ b/security/landlock/ruleset.h
> @@ -37,6 +37,14 @@ struct landlock_layer {
>  	access_mask_t access;
>  };
>  
> +/*
> + * Flexible array of Landlock layers, used for pointer-to-array function
> + * parameters that reference either a stack-allocated layer array or a rule's
> + * flexible array member (struct landlock_rule.layers).  This typedef avoids
> + * the complex (*const name)[] syntax that the kernel-doc parser cannot handle.
> + */
> +typedef struct landlock_layer landlock_layer_array_t[];
> +
>  /**
>   * union landlock_key - Key of a ruleset's red-black tree
>   */
> -- 
> 2.53.0
> 

Thanks for the reminder on the other thread; I skipped over this one
indeed. I am hesitant about this patch because it seems to be at odds
with the Linux kernel coding style on the use of typedef:

https://www.kernel.org/doc/html/v4.17/process/coding-style.html#typedefs

It says:

    the rule should basically be to NEVER EVER use a typedef unless
    you can clearly match one of those rules.

The rules being:

    (a) totally opaque object whose contents we want to hide
        (I don't think that is the purpose here; the example in
	the style guide is to keep generic code from playing with
	hardware-specific page table entry structures)
    (b) integer types (not applicable)
    (c) when using sparse (not applicable)
    (d) some types identical to C99 types (not applicable)
    (e) types safe for use in userspace (not applicable)

It seems that the easier option might be to drop the "const" between
the pointer and the type, if apparently we are the only ones doing
this?

FWIW, I have put these consts as well to be consistent with Landlock
style, but I am also not convinced that they buy us much;

* In a type like "const u8 *buf", when the type is part of a function
  signature, that is a guarantee to the caller that the function won't
  modify the buffer contents through the pointer.

* However, in a type like "u8 *const buf", the const is not a
  guarantee to the caller, but only a constraint on the function
  implementation that the pointer is not rewired to point elsewhere.
  It is not clear to me that this adds much in implementation safety.

WDYT?

—Günther

Re: [PATCH v1 1/4] landlock: Fix kernel-doc warning for pointer-to-array parameters

[Mickaël Salaün]

On Tue, Mar 10, 2026 at 02:18:54PM +0100, Günther Noack wrote:
> On Wed, Mar 04, 2026 at 08:31:24PM +0100, Mickaël Salaün wrote:
> > The insert_rule() and create_rule() functions take a
> > pointer-to-flexible-array parameter declared as:
> > 
> >   const struct landlock_layer (*const layers)[]
> > 
> > The kernel-doc parser cannot handle a qualifier between * and the
> > parameter name in this syntax, producing spurious "Invalid param" and
> > "not described" warnings.
> > 
> > Introduce landlock_layer_array_t as a typedef for the flexible array
> > type so the parameter can be written as:
> > 
> >   const landlock_layer_array_t *const layers
> > 
> > This is the same type but kernel-doc parses it correctly, while
> > preserving the pointer-to-array type safety that prevents callers from
> > accidentally passing a pointer to a single element.
> > 
> > Cc: Günther Noack <gnoack@google.com>
> > Signed-off-by: Mickaël Salaün <mic@digikod.net>
> > ---
> >  security/landlock/ruleset.c | 4 ++--
> >  security/landlock/ruleset.h | 8 ++++++++
> >  2 files changed, 10 insertions(+), 2 deletions(-)
> > 
> > diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c
> > index 419b237de635..a61ced492f41 100644
> > --- a/security/landlock/ruleset.c
> > +++ b/security/landlock/ruleset.c
> > @@ -108,7 +108,7 @@ static bool is_object_pointer(const enum landlock_key_type key_type)
> >  
> >  static struct landlock_rule *
> >  create_rule(const struct landlock_id id,
> > -	    const struct landlock_layer (*const layers)[], const u32 num_layers,
> > +	    const landlock_layer_array_t *const layers, const u32 num_layers,
> >  	    const struct landlock_layer *const new_layer)
> >  {
> >  	struct landlock_rule *new_rule;
> > @@ -205,7 +205,7 @@ static void build_check_ruleset(void)
> >   */
> >  static int insert_rule(struct landlock_ruleset *const ruleset,
> >  		       const struct landlock_id id,
> > -		       const struct landlock_layer (*const layers)[],
> > +		       const landlock_layer_array_t *const layers,
> >  		       const size_t num_layers)
> >  {
> >  	struct rb_node **walker_node;
> > diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h
> > index 9d6dc632684c..87d52031fb5a 100644
> > --- a/security/landlock/ruleset.h
> > +++ b/security/landlock/ruleset.h
> > @@ -37,6 +37,14 @@ struct landlock_layer {
> >  	access_mask_t access;
> >  };
> >  
> > +/*
> > + * Flexible array of Landlock layers, used for pointer-to-array function
> > + * parameters that reference either a stack-allocated layer array or a rule's
> > + * flexible array member (struct landlock_rule.layers).  This typedef avoids
> > + * the complex (*const name)[] syntax that the kernel-doc parser cannot handle.
> > + */
> > +typedef struct landlock_layer landlock_layer_array_t[];
> > +
> >  /**
> >   * union landlock_key - Key of a ruleset's red-black tree
> >   */
> > -- 
> > 2.53.0
> > 
> 
> Thanks for the reminder on the other thread; I skipped over this one
> indeed. I am hesitant about this patch because it seems to be at odds
> with the Linux kernel coding style on the use of typedef:
> 
> https://www.kernel.org/doc/html/v4.17/process/coding-style.html#typedefs
> 
> It says:
> 
>     the rule should basically be to NEVER EVER use a typedef unless
>     you can clearly match one of those rules.
> 
> The rules being:
> 
>     (a) totally opaque object whose contents we want to hide
>         (I don't think that is the purpose here; the example in
> 	the style guide is to keep generic code from playing with
> 	hardware-specific page table entry structures)
>     (b) integer types (not applicable)
>     (c) when using sparse (not applicable)
>     (d) some types identical to C99 types (not applicable)
>     (e) types safe for use in userspace (not applicable)
> 
> It seems that the easier option might be to drop the "const" between
> the pointer and the type, if apparently we are the only ones doing
> this?

Yeah, this is simpler.

> 
> FWIW, I have put these consts as well to be consistent with Landlock
> style, but I am also not convinced that they buy us much;
> 
> * In a type like "const u8 *buf", when the type is part of a function
>   signature, that is a guarantee to the caller that the function won't
>   modify the buffer contents through the pointer.
> 
> * However, in a type like "u8 *const buf", the const is not a
>   guarantee to the caller, but only a constraint on the function
>   implementation that the pointer is not rewired to point elsewhere.
>   It is not clear to me that this adds much in implementation safety.

I prefer to have const variables where possible to look for changes in
patches that could then have indirect impact on initial invariants.

But for this case, I prefer to have the doc linter covering C files.

I'll send a v2 for this change only, I'll merge the other patches.

> 
> WDYT?
> 
> —Günther
>