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 <[email protected]>
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/[email protected]?part=5
