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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] prctl: missing arg validation and buffer size UAPI in PR_{SET,GET}_EXT_NAME
--

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

prctl: Add support for long user thread names

This commit adds the PR_SET_EXT_NAME and PR_GET_EXT_NAME prctl() options to
support getting and setting extended user thread names up to
TASK_COMM_EXT_LEN bytes.

> diff --git a/include/uapi/linux/prctl.h b/include/uapi/linux/prctl.h
> index b6ec6f6937195..a07f8edadd652 100644
> --- a/include/uapi/linux/prctl.h
> +++ b/include/uapi/linux/prctl.h
> @@ -56,6 +56,9 @@
>  #define PR_SET_NAME    15		/* Set process name */
>  #define PR_GET_NAME    16		/* Get process name */
>  
> +#define PR_SET_EXT_NAME    17		/* Set extended process name */
> +#define PR_GET_EXT_NAME    18		/* Get extended process name */
> +
>  /* Get/set process endian */
>  #define PR_GET_ENDIAN	19
>  #define PR_SET_ENDIAN	20
> diff --git a/kernel/sys.c b/kernel/sys.c
> index 6a72c7e644e70..fc9e3c5a91353 100644
> --- a/kernel/sys.c
> +++ b/kernel/sys.c

[ ... ]

> @@ -2616,6 +2616,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]
Should PR_SET_EXT_NAME and PR_GET_EXT_NAME in SYSCALL_DEFINE5(prctl) validate
that the unused arguments (arg3, arg4, and arg5) are zero and return -EINVAL
otherwise? Because libc's prctl() wrapper is variadic, 2-argument calls from
userspace can pass arbitrary register values in arg3..arg5, which prevents
extending these options with flags or length parameters in the future.

In addition, PR_GET_EXT_NAME unconditionally copies TASK_COMM_EXT_LEN (64)
bytes to the userspace buffer at arg2, but TASK_COMM_EXT_LEN is only defined
in include/linux/sched.h and is not exposed in include/uapi/linux/prctl.h:

include/linux/sched.h:
    enum {
        TASK_COMM_LEN = 16,
        TASK_COMM_EXT_LEN = 64,
    };

Without a UAPI constant or a buffer size argument in PR_GET_EXT_NAME, could a
userspace caller passing a smaller buffer (or passing a buffer length in arg3)
have 64 bytes written past the end of its buffer, or break if
TASK_COMM_EXT_LEN is ever increased?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261030-tonyk-long_name-v8-0-c9b030332254@igalia.com?part=5