чт, 2 мая 2019 г. в 14:23, Joel Savitz <jsav...@redhat.com>:
>
> Yes, this the change, thanks to the suggestion of Yury Norov.
Joel, could you please stop top-posting?
> I also now explicitly mention the expected userspace destination type
> in the manpage patch.
>
> Best,
> Joel Savitz
>
>
> On Thu, May 2, 2019 at 5:10 PM Cyrill Gorcunov <gorcu...@gmail.com> wrote:
> >
> > On Thu, May 02, 2019 at 05:01:38PM -0400, Waiman Long wrote:
> > >
> > > What did you change in v2 versus v1?
> >
> > Seems unsigned long long has been changed to unsigned long.
Sorry guys, I replied to Joel, but accidentally dropped the folks.
Find discussion below.
чт, 2 мая 2019 г. в 13:50, Joel Savitz <jsav...@redhat.com>:
>
> While I disagree that kernel memory is exposed, as the 8-byte
> (unsigned long long) value of task_size is initialized by the
> assignment of TASK_SIZE, I agree with your suggestion, as the current
> code may corrupt the userspace stack of the caller unless provided
> with the address of an unsigned long long, an unusual type to store a
> value of word size.
>
> As such, I have adopted your suggestion and added type information to
> my manpage patch. Expect the v2 to be posted shortly.
>
> Thank you for your review.
>
> Best,
> Joel Savitz
>
> On Thu, May 2, 2019 at 3:41 PM Yury Norov <norov.maill...@gmail.com> wrote:
> >
> > чт, 2 мая 2019 г. в 12:15, Joel Savitz <jsav...@redhat.com>:
> > >
> > > 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.
> >
> > but you copy the value of task_size.
> >
> > > Suggested-by: Alexey Dobriyan <adobri...@gmail.com>
> > > Signed-off-by: Joel Savitz <jsav...@redhat.com>
> > > ---
> > > include/uapi/linux/prctl.h | 3 +++
> > > kernel/sys.c | 10 ++++++++++
> > > 2 files changed, 13 insertions(+)
> > >
> > > diff --git a/include/uapi/linux/prctl.h b/include/uapi/linux/prctl.h
> > > index 094bb03b9cc2..2335fe0a8db8 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 */
> > > +#define PR_GET_TASK_SIZE 55
> > > +
> > > #endif /* _LINUX_PRCTL_H */
> > > diff --git a/kernel/sys.c b/kernel/sys.c
> > > index 12df0e5434b8..7ced7dbd035d 100644
> > > --- a/kernel/sys.c
> > > +++ b/kernel/sys.c
> > > @@ -2252,6 +2252,13 @@ static int propagate_has_child_subreaper(struct
> > > task_struct *p, void *data)
> > > return 1;
> > > }
> > >
> > > +static int prctl_get_tasksize(void __user * uaddr)
> > > +{
> > > + unsigned long long task_size = TASK_SIZE;
> > > + return copy_to_user(uaddr, &task_size, sizeof(unsigned long long))
> > > + ? -EFAULT : 0;
> > > +}
> > > +
> >
> > task_size is unsigned long. On 32-bit systems you will end up exposing 4
> > bytes
> > of kernel memory. You should switch to sizeof(unsigned long).
> >
> > Your code is broken for compat arches. Take a look at the definition
> > of TASK_SIZE
> > for arm64, for example.
> >
> > Thanks,
> > Yury
