linux-kernel - [PATCH] exit: call add_device_randomness() without tasklist

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250128140701.3136924-1-mjguzik@gmail.com>
Date: Tue, 28 Jan 2025 15:07:01 +0100
From: Mateusz Guzik <mjguzik@...il.com>
To: brauner@...nel.org,
	oleg@...hat.com,
	akpm@...ux-foundation.org
Cc: linux-kernel@...r.kernel.org,
	Mateusz Guzik <mjguzik@...il.com>
Subject: [PATCH] exit: call add_device_randomness() without tasklist_lock

Parallel calls to add_device_randomness() contend in their own right.

The clone side aleady runs outside of tasklist_lock, which in turn means
any caller on the exit side extends the tasklist_lock hold time while
contending on the random-private lock.

Fixing this problem bumps thread creation/destruction rate by 4% on a
24 core vm.

Bench (plop into will-it-scale):
$ cat tests/threadspawn1.c

char *testcase_description = "Thread creation and teardown";

static void *worker(void *arg)
{
        return (NULL);
}

void testcase(unsigned long long *iterations, unsigned long nr)
{
        pthread_t thread;
        int error;

        while (1) {
                error = pthread_create(&thread, NULL, worker, NULL);
                assert(error == 0);
                error = pthread_join(thread, NULL);
                assert(error == 0);
                (*iterations)++;
        }
}

Signed-off-by: Mateusz Guzik <mjguzik@...il.com>
---
 kernel/exit.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/kernel/exit.c b/kernel/exit.c
index 1dcddfe537ee..8a9ac55dc26e 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -141,13 +141,14 @@ static void __unhash_process(struct task_struct *p, bool group_dead)
 /*
  * This function expects the tasklist_lock write-locked.
  */
-static void __exit_signal(struct task_struct *tsk)
+static unsigned long long __exit_signal(struct task_struct *tsk)
 {
 	struct signal_struct *sig = tsk->signal;
 	bool group_dead = thread_group_leader(tsk);
 	struct sighand_struct *sighand;
 	struct tty_struct *tty;
 	u64 utime, stime;
+	unsigned long long randomness;
 
 	sighand = rcu_dereference_check(tsk->sighand,
 					lockdep_tasklist_lock_is_held());
@@ -174,8 +175,7 @@ static void __exit_signal(struct task_struct *tsk)
 			sig->curr_target = next_thread(tsk);
 	}
 
-	add_device_randomness((const void*) &tsk->se.sum_exec_runtime,
-			      sizeof(unsigned long long));
+	randomness = tsk->se.sum_exec_runtime;
 
 	/*
 	 * Accumulate here the counters for all threads as they die. We could
@@ -214,6 +214,8 @@ static void __exit_signal(struct task_struct *tsk)
 		flush_sigqueue(&sig->shared_pending);
 		tty_kref_put(tty);
 	}
+
+	return randomness;
 }
 
 static void delayed_put_task_struct(struct rcu_head *rhp)
@@ -242,6 +244,7 @@ void release_task(struct task_struct *p)
 	struct task_struct *leader;
 	struct pid *thread_pid;
 	int zap_leader;
+	unsigned long long randomness;
 repeat:
 	/* don't need to get the RCU readlock here - the process is dead and
 	 * can't be modifying its own credentials. But shut RCU-lockdep up */
@@ -254,7 +257,7 @@ void release_task(struct task_struct *p)
 	write_lock_irq(&tasklist_lock);
 	ptrace_release_task(p);
 	thread_pid = get_pid(p->thread_pid);
-	__exit_signal(p);
+	randomness = __exit_signal(p);
 
 	/*
 	 * If we are the last non-leader member of the thread
@@ -280,6 +283,8 @@ void release_task(struct task_struct *p)
 	put_pid(thread_pid);
 	release_thread(p);
 	put_task_struct_rcu_user(p);
+	add_device_randomness((const void*) &randomness,
+			      sizeof(unsigned long long));
 
 	p = leader;
 	if (unlikely(zap_leader))
-- 
2.43.0