Re: [GIT PULL] execve updates for v6.13-rc1 (take 2)

[Linus Torvalds]

On Tue, 26 Nov 2024 at 12:11, Linus Torvalds
<torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
>
> So something like the attached. But it's *ENTIRELY* untested. [..]

It was also entirely un-attached. Here's the actual attachment.

           Linus
 fs/exec.c               | 27 +++++++++++++++++++++++----
 include/linux/binfmts.h |  1 +
 2 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/fs/exec.c b/fs/exec.c
index da51ca70489a..850421445ccb 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1194,12 +1194,22 @@ static int unshare_sighand(struct task_struct *me)
  * so that a new one can be started
  */
 
+/*
+ * No locking, instead this just makes sure that comm[] is always
+ * NUL-terminated. Note that 'strscpy_pad()' does not guarantee
+ * that at least right now - it may do the copy without a NUL and
+ * then overwrite the last character.
+ *
+ * One of the few cases where 'strncpy()' does the right thing.
+ *
+ * If you do concurrent set_task_comm() calls, the name may be
+ * a bit random, but it's going to be NUL-terminated.
+ */
 void __set_task_comm(struct task_struct *tsk, const char *buf, bool exec)
 {
-	task_lock(tsk);
 	trace_task_rename(tsk, buf);
-	strscpy_pad(tsk->comm, buf, sizeof(tsk->comm));
-	task_unlock(tsk);
+	tsk->comm[sizeof(tsk->comm)-1] = 0;
+	strncpy(tsk->comm, buf, sizeof(tsk->comm)-1);
 	perf_event_comm(tsk, exec);
 }
 
@@ -1337,7 +1347,7 @@ int begin_new_exec(struct linux_binprm * bprm)
 		set_dumpable(current->mm, SUID_DUMP_USER);
 
 	perf_event_exec();
-	__set_task_comm(me, kbasename(bprm->filename), true);
+	__set_task_comm(me, bprm->comm, true);
 
 	/* An exec changes our domain. We are no longer part of the thread
 	   group */
@@ -1510,6 +1520,11 @@ static struct linux_binprm *alloc_bprm(int fd, struct filename *filename, int fl
 
 	if (fd == AT_FDCWD || filename->name[0] == '/') {
 		bprm->filename = filename->name;
+		/*
+		 * FIXME(?): this could take advantage of 'filename->len'
+		 * to do better than kbasename()
+		 */
+		strscpy(bprm->comm, kbasename(filename->name));
 	} else {
 		if (filename->name[0] == '\0')
 			bprm->fdpath = kasprintf(GFP_KERNEL, "/dev/fd/%d", fd);
@@ -1532,6 +1547,10 @@ static struct linux_binprm *alloc_bprm(int fd, struct filename *filename, int fl
 			bprm->interp_flags |= BINPRM_FLAGS_PATH_INACCESSIBLE;
 
 		bprm->filename = bprm->fdpath;
+
+		rcu_read_lock();
+		strscpy(bprm->comm, smp_load_acquire(&file->f_path.dentry->d_name.name));
+		rcu_read_unlock();
 	}
 	bprm->interp = bprm->filename;
 
diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h
index e6c00e860951..3c8bc84ca9e4 100644
--- a/include/linux/binfmts.h
+++ b/include/linux/binfmts.h
@@ -61,6 +61,7 @@ struct linux_binprm {
 
 	struct rlimit rlim_stack; /* Saved RLIMIT_STACK used during exec. */
 
+	char comm[TASK_COMM_LEN];
 	char buf[BINPRM_BUF_SIZE];
 } __randomize_layout;