[PATCH 1/6] fs/exec: Drop task_lock() inside __get_task_comm()

Yafang Shao laoar.shao at gmail.com
Sun Jun 2 02:37:49 UTC 2024


Quoted from Linus [0]:

  Since user space can randomly change their names anyway, using locking
  was always wrong for readers (for writers it probably does make sense
  to have some lock - although practically speaking nobody cares there
  either, but at least for a writer some kind of race could have
  long-term mixed results

Suggested-by: Linus Torvalds <torvalds at linux-foundation.org>
Link: https://lore.kernel.org/all/CAHk-=wivfrF0_zvf+oj6==Sh=-npJooP8chLPEfaFV0oNYTTBA@mail.gmail.com [0]
Signed-off-by: Yafang Shao <laoar.shao at gmail.com>
Cc: Alexander Viro <viro at zeniv.linux.org.uk>
Cc: Christian Brauner <brauner at kernel.org>
Cc: Jan Kara <jack at suse.cz>
Cc: Eric Biederman <ebiederm at xmission.com>
Cc: Kees Cook <keescook at chromium.org>
---
 fs/exec.c             | 7 +++++--
 include/linux/sched.h | 2 +-
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/fs/exec.c b/fs/exec.c
index b3c40fbb325f..b43992d35a8a 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1227,12 +1227,15 @@ static int unshare_sighand(struct task_struct *me)
 	return 0;
 }
 
+/*
+ * User space can randomly change their names anyway, so locking for readers
+ * doesn't make sense. For writers, locking is probably necessary, as a race
+ * condition could lead to long-term mixed results.
+ */
 char *__get_task_comm(char *buf, size_t buf_size, struct task_struct *tsk)
 {
-	task_lock(tsk);
 	/* Always NUL terminated and zero-padded */
 	strscpy_pad(buf, tsk->comm, buf_size);
-	task_unlock(tsk);
 	return buf;
 }
 EXPORT_SYMBOL_GPL(__get_task_comm);
diff --git a/include/linux/sched.h b/include/linux/sched.h
index c75fd46506df..56a927393a38 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1083,7 +1083,7 @@ struct task_struct {
 	 *
 	 * - normally initialized setup_new_exec()
 	 * - access it with [gs]et_task_comm()
-	 * - lock it with task_lock()
+	 * - lock it with task_lock() for writing
 	 */
 	char				comm[TASK_COMM_LEN];
 
-- 
2.39.1




More information about the Linux-security-module-archive mailing list