Messages in this thread | | | Date | Fri, 3 May 2019 14:08:31 -0700 | From | Yury Norov <> | Subject | Re: [PATCH v3 1/2] kernel/sys: add PR_GET_TASK_SIZE option to prctl(2) |
| |
On Fri, May 03, 2019 at 02:10:20PM -0400, Joel Savitz wrote: > When PR_GET_TASK_SIZE is passed to prctl, the kernel will attempt to > copy the value of TASK_SIZE to the userspace address in arg2. > > It is important that we account for the case of the userspace task > running in 32-bit compat mode on a 64-bit kernel. As such, we must be > careful to copy the correct number of bytes to userspace to avoid stack > corruption. > > Suggested-by: Yuri Norov <yury.norov@gmail.com>
I actually didn't suggest that. If you _really_ need TASK_SIZE to be exposed, I would suggest to expose it in kernel headers. TASK_SIZE is a compile-time information, and it may available for userspace at compile time as well.
> Suggested-by: Alexey Dobriyan <adobriyan@gmail.com> > Signed-off-by: Joel Savitz <jsavitz@redhat.com> > --- > include/uapi/linux/prctl.h | 3 +++ > kernel/sys.c | 23 +++++++++++++++++++++++ > 2 files changed, 26 insertions(+) > > diff --git a/include/uapi/linux/prctl.h b/include/uapi/linux/prctl.h > index 094bb03b9cc2..2c261c461952 100644 > --- a/include/uapi/linux/prctl.h > +++ b/include/uapi/linux/prctl.h > @@ -229,4 +229,7 @@ struct prctl_mm_map { > # define PR_PAC_APDBKEY (1UL << 3) > # define PR_PAC_APGAKEY (1UL << 4) > > +/* Get the process virtual memory size (i.e. the highest usable VM address) */ > +#define PR_GET_TASK_SIZE 55 > + > #endif /* _LINUX_PRCTL_H */ > diff --git a/kernel/sys.c b/kernel/sys.c > index 12df0e5434b8..709584400070 100644 > --- a/kernel/sys.c > +++ b/kernel/sys.c > @@ -2252,6 +2252,26 @@ static int propagate_has_child_subreaper(struct task_struct *p, void *data) > return 1; > } > > +static int prctl_get_tasksize(void __user *uaddr) > +{ > + unsigned long current_task_size, current_word_size; > + > + current_task_size = TASK_SIZE; > + current_word_size = sizeof(unsigned long); > + > +#ifdef CONFIG_64BIT > + /* On 64-bit architecture, we must check whether the current thread > + * is running in 32-bit compat mode. If it is, we can simply cut > + * the size in half. This avoids corruption of the userspace stack. > + */ > + if (test_thread_flag(TIF_ADDR32))
It breaks build for all architectures except x86 since TIF_ADDR32 is defined for x86 only.
In comment to v2 I suggested you to stick to fixed-size data type to avoid exactly this problem.
NACK
Yury
> + current_word_size >>= 1; > +#endif > + > + return copy_to_user(uaddr, ¤t_task_size, current_word_size) ? -EFAULT : 0; > +} > + > int __weak arch_prctl_spec_ctrl_get(struct task_struct *t, unsigned long which) > { > return -EINVAL; > @@ -2486,6 +2506,9 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3, > return -EINVAL; > error = PAC_RESET_KEYS(me, arg2); > break; > + case PR_GET_TASK_SIZE: > + error = prctl_get_tasksize((void *)arg2); > + break; > default: > error = -EINVAL; > break; > -- > 2.18.1
| |