[PATCH v2 1/2] landlock: Fully release unused TSYNC work entries

[Mickaël Salaün]

If task_work_add() failed, ctx->task is put but the tsync_works struct
is not reset to its previous state.  The first consequence is that the
kernel allocates memory for dying threads, which could lead to
user-accounted memory exhaustion (not very useful nor specific to this
case).  The second consequence is that task_work_cancel(), called by
cancel_tsync_works(), can dereference a NULL task pointer.

Fix this issues by keeping a consistent works->size wrt the added task
work.  This is done in a new tsync_works_trim() helper which also cleans
up the shared_ctx and work fields.

As a safeguard, add a pointer check to cancel_tsync_works() and update
tsync_works_release() accordingly.

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

Changes since v1:
https://lore.kernel.org/all/20260216142641.2100407-1-mic@digikod.net/
- Move the return/release logic into a new tsync_works_trim() helper
  (suggested by Günther).
- Reset the whole ctx with memset().
- Add an unlinkely(err).
---
 security/landlock/tsync.c | 47 ++++++++++++++++++++++++++++++++++-----
 1 file changed, 41 insertions(+), 6 deletions(-)

diff --git a/security/landlock/tsync.c b/security/landlock/tsync.c
index 0d2b9c646030..42cc0ef0c704 100644
--- a/security/landlock/tsync.c
+++ b/security/landlock/tsync.c
@@ -203,6 +203,40 @@ static struct tsync_work *tsync_works_provide(struct tsync_works *s,
 	return ctx;
 }
 
+/**
+ * tsync_works_trim - Put the last tsync_work element
+ *
+ * @s: TSYNC works to trim.
+ *
+ * Put the last task and decrement the size of @s.
+ *
+ * This helper does not cancel a running task, but just reset the last element
+ * to zero.
+ */
+static void tsync_works_trim(struct tsync_works *s)
+{
+	struct tsync_work *ctx;
+
+	if (WARN_ON_ONCE(s->size <= 0))
+		return;
+
+	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.
+	 */
+	put_task_struct(ctx->task);
+	memset(ctx, 0, sizeof(*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.
+	 */
+	s->size--;
+}
+
 /*
  * tsync_works_grow_by - preallocates space for n more contexts in s
  *
@@ -276,7 +310,7 @@ static void tsync_works_release(struct tsync_works *s)
 	size_t i;
 
 	for (i = 0; i < s->size; i++) {
-		if (!s->works[i]->task)
+		if (WARN_ON_ONCE(!s->works[i]->task))
 			continue;
 
 		put_task_struct(s->works[i]->task);
@@ -379,16 +413,14 @@ static bool schedule_task_work(struct tsync_works *works,
 
 		init_task_work(&ctx->work, restrict_one_thread_callback);
 		err = task_work_add(thread, &ctx->work, TWA_SIGNAL);
-		if (err) {
+		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.  For consistency, remove the task from ctx
-			 * so that it does not look like we handed it a task_work.
+			 * next loop iteration.
 			 */
-			put_task_struct(ctx->task);
-			ctx->task = NULL;
+			tsync_works_trim(works);
 
 			atomic_dec(&shared_ctx->num_preparing);
 			atomic_dec(&shared_ctx->num_unfinished);
@@ -412,6 +444,9 @@ static void cancel_tsync_works(struct tsync_works *works,
 	int i;
 
 	for (i = 0; i < works->size; i++) {
+		if (WARN_ON_ONCE(!works->works[i]->task))
+			continue;
+
 		if (!task_work_cancel(works->works[i]->task,
 				      &works->works[i]->work))
 			continue;
-- 
2.53.0