On Fri, May 03, 2019 at 02:08:31PM -0700, Yury Norov wrote:
> 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.no...@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.
>
TASK_SIZE is a runtime resolved macro, dependent on the thread currently
running on the CPU. It's not a compile time constant.
Anyways, it's proven that going prctl(2), although interesting, as
suggested by Alexey, wasn't worth the hassle as it poses more issues
than it can possibly solve.
A better way to get this value exposed to userspace is really through
/proc/<pid>/status, where one can utilize TASK_SIZE_OF(mm->owner), or
simply mm->task_size, which seems to be properly assigned for each arch
> > Suggested-by: Alexey Dobriyan <adobri...@gmail.com>
> > Signed-off-by: Joel Savitz <jsav...@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
