Re: [PATCH v5 5/6] prctl: Add support for long user thread names

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Expanding the task comm buffer size without updating the caller size check allows out-of-bounds reads from 16-byte source buffers.
- [Medium] PR_GET_EXT_NAME writes a fixed 64 bytes to a user-provided pointer without taking a buffer length parameter, risking userspace buffer overflows.
- [Low] Calls to `get_task_comm()` were not updated in the smack security module, causing a build failure.
--

commit 14b42cecc71f504b5e0c3127a6bba6b0901cce09
Author: André Almeida <andrealmeid@igalia.com>

prctl: Add support for long user thread names

This commit adds support for getting and setting long user thread names
using the new PR_SET_EXT_NAME and PR_GET_EXT_NAME prctl options.

> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index c41d61e9babf9..4f2cd802f1721 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -2032,7 +2032,7 @@ extern void kick_process(struct task_struct *tsk);
>  
>  extern void __set_task_comm(struct task_struct *tsk, const char *from, bool exec);
>  #define set_task_comm(tsk, from) ({			\
> -	BUILD_BUG_ON(sizeof(from) != TASK_COMM_LEN);	\
> +	BUILD_BUG_ON(sizeof(from) < TASK_COMM_LEN);	\
>  	__set_task_comm(tsk, from, false);		\
>  })

[Severity: High]
Does this change allow out-of-bounds reads from 16-byte source buffers?

The macro validates that the buffer is at least TASK_COMM_LEN (16 bytes),
but the internal implementation __set_task_comm() uses the enlarged
destination struct size for bounding:

fs/exec.c:__set_task_comm() {
    ...
    size_t len = strnlen(buf, sizeof(tsk->comm) - 1);
    trace_task_rename(tsk, buf);
    memcpy(tsk->comm, buf, len);
    ...
}

If a caller passes a 16-byte array without a NUL terminator, will
strnlen() read past the end of the buffer up to 63 bytes, copying
out-of-bounds stack or heap memory into tsk->comm?

[ ... ]

> diff --git a/kernel/sys.c b/kernel/sys.c
> index 5ebd3c1dfddf7..4caf6ef836f1a 100644
> --- a/kernel/sys.c
> +++ b/kernel/sys.c
[ ... ]
> @@ -2613,6 +2613,19 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3,
>  		if (copy_to_user((char __user *)arg2, comm, TASK_COMM_LEN))
>  			return -EFAULT;
>  		break;
> +	case PR_SET_EXT_NAME:
> +		comm[TASK_COMM_EXT_LEN - 1] = 0;
> +		if (strncpy_from_user(comm, (char __user *)arg2,
> +				      TASK_COMM_EXT_LEN - 1) < 0)
> +			return -EFAULT;
> +		set_task_comm(me, comm);
> +		proc_comm_connector(me);
> +		break;
> +	case PR_GET_EXT_NAME:
> +		strscpy_pad(comm, me->comm, TASK_COMM_EXT_LEN);
> +		if (copy_to_user((char __user *)arg2, comm, TASK_COMM_EXT_LEN))
> +			return -EFAULT;
> +		break;

[Severity: Medium]
Could this lead to userspace buffer overflows?

This option writes a fixed 64 bytes to a user-provided pointer without
taking a buffer length parameter. If a userspace application calls
prctl(PR_GET_EXT_NAME, buf) with a buffer smaller than 64 bytes (e.g.,
assuming it behaves like PR_GET_NAME which uses 16 bytes), will this
blindly overwrite userspace memory and cause silent corruption?

[ ... ]

[Severity: Low]
Is the build broken for CONFIG_SECURITY_SMACK=y?

Calls to get_task_comm() do not seem to have been updated in the smack
security module after the macro was removed in a previous patch in this
series.

security/smack/smack_access.c:smack_log() {
    ...
    audit_log_untrustedstring(ab,
        get_task_comm(comm, sad->subj_tsk));
    ...
}

security/smack/smack_lsm.c:smk_bu_tsk_to_obj() {
    ...
    pr_info("Smack %s: (%s %s %s) %s %s\n", smk_bu_mess[rc],
        smk_of_task(tsp)->smk_known, oskp->smk_known,
        acc, get_task_comm(comm, tsk), note);
    ...
}

Will these dangling usages cause compile-time failures?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260827-tonyk-long_name-v5-0-5fa843782a00@igalia.com?part=5