Re: [PATCH v2 0/2] sys/prctl: expose TASK_SIZE value to userspace
чт, 2 мая 2019 г. в 14:23, Joel Savitz :
>
> 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 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 :
>
> 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 wrote:
> >
> > чт, 2 мая 2019 г. в 12:15, Joel Savitz :
> > >
> > > 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
> > > Signed-off-by: Joel Savitz
> > > ---
> > > 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, _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
Re: [PATCH v2 0/2] sys/prctl: expose TASK_SIZE value to userspace
On Thu, May 02, 2019 at 04:52:20PM -0400, Joel Savitz wrote: > In the mainline kernel, there is no quick mechanism to get the virtual > memory size of the current process from userspace. > > Despite the current state of affairs, this information is available to the > user through several means, one being a linear search of the entire address > space. This is an inefficient use of cpu cycles. > > A component of the libhugetlb kernel test does exactly this, and as > systems' address spaces increase beyond 32-bits, this method becomes > exceedingly tedious. > > For example, on a ppc64le system with a 47-bit address space, the linear > search causes the test to hang for some unknown amount of time. I > couldn't give you an exact number because I just ran it for about 10-20 > minutes and went to go do something else, probably to get coffee or > something, and when I came back, I just killed the test and patched it > to use this new mechanism. I re-ran my new version of the test using a > kernel with this patch, and of course it passed through the previously > bottlenecking codepath nearly instantaneously. > > As such, I propose that the prctl syscall be extended to include the > option to retrieve TASK_SIZE from the kernel. > > This patch will allow us to upgrade an O(n) codepath to O(1) in an > architecture-independent manner, and provide a mechanism for others > to do the same. > > Joel Savitz(2): > sys/prctl: add PR_GET_TASK_SIZE option to prctl(2) > prctl.2: Document the new PR_GET_TASK_SIZE option > > include/uapi/linux/prctl.h | 3 +++ > kernel/sys.c | 10 ++ > 2 files changed, 13 insertions(+) > > man2/prctl.2 | 9 + > 1 file changed, 9 insertions(+) > > -- > 2.18.1 > If you decide to repost patch 1/2 to sort out the minor nit I pointed out, you can keep my ack. Acked-by: Rafael Aquini
Re: [PATCH v2 0/2] sys/prctl: expose TASK_SIZE value to userspace
Yes, this the change, thanks to the suggestion of Yury Norov. 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 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.
Re: [PATCH v2 0/2] sys/prctl: expose TASK_SIZE value to userspace
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.
Re: [PATCH v2 0/2] sys/prctl: expose TASK_SIZE value to userspace
On 5/2/19 4:52 PM, Joel Savitz wrote: > In the mainline kernel, there is no quick mechanism to get the virtual > memory size of the current process from userspace. > > Despite the current state of affairs, this information is available to the > user through several means, one being a linear search of the entire address > space. This is an inefficient use of cpu cycles. > > A component of the libhugetlb kernel test does exactly this, and as > systems' address spaces increase beyond 32-bits, this method becomes > exceedingly tedious. > > For example, on a ppc64le system with a 47-bit address space, the linear > search causes the test to hang for some unknown amount of time. I > couldn't give you an exact number because I just ran it for about 10-20 > minutes and went to go do something else, probably to get coffee or > something, and when I came back, I just killed the test and patched it > to use this new mechanism. I re-ran my new version of the test using a > kernel with this patch, and of course it passed through the previously > bottlenecking codepath nearly instantaneously. > > As such, I propose that the prctl syscall be extended to include the > option to retrieve TASK_SIZE from the kernel. > > This patch will allow us to upgrade an O(n) codepath to O(1) in an > architecture-independent manner, and provide a mechanism for others > to do the same. > > Joel Savitz(2): > sys/prctl: add PR_GET_TASK_SIZE option to prctl(2) > prctl.2: Document the new PR_GET_TASK_SIZE option > > include/uapi/linux/prctl.h | 3 +++ > kernel/sys.c | 10 ++ > 2 files changed, 13 insertions(+) > > man2/prctl.2 | 9 + > 1 file changed, 9 insertions(+) > > -- > 2.18.1 What did you change in v2 versus v1? Cheers, Longman
[PATCH v2 0/2] sys/prctl: expose TASK_SIZE value to userspace
In the mainline kernel, there is no quick mechanism to get the virtual memory size of the current process from userspace. Despite the current state of affairs, this information is available to the user through several means, one being a linear search of the entire address space. This is an inefficient use of cpu cycles. A component of the libhugetlb kernel test does exactly this, and as systems' address spaces increase beyond 32-bits, this method becomes exceedingly tedious. For example, on a ppc64le system with a 47-bit address space, the linear search causes the test to hang for some unknown amount of time. I couldn't give you an exact number because I just ran it for about 10-20 minutes and went to go do something else, probably to get coffee or something, and when I came back, I just killed the test and patched it to use this new mechanism. I re-ran my new version of the test using a kernel with this patch, and of course it passed through the previously bottlenecking codepath nearly instantaneously. As such, I propose that the prctl syscall be extended to include the option to retrieve TASK_SIZE from the kernel. This patch will allow us to upgrade an O(n) codepath to O(1) in an architecture-independent manner, and provide a mechanism for others to do the same. Joel Savitz(2): sys/prctl: add PR_GET_TASK_SIZE option to prctl(2) prctl.2: Document the new PR_GET_TASK_SIZE option include/uapi/linux/prctl.h | 3 +++ kernel/sys.c | 10 ++ 2 files changed, 13 insertions(+) man2/prctl.2 | 9 + 1 file changed, 9 insertions(+) -- 2.18.1
