lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAJu-Uz4rnMCR5-zHj=Ub1xZD=mmw3z1-mEg+M=qWXV3yO7OJkg@mail.gmail.com> Date: Thu, 2 May 2019 14:34:49 -0700 From: Yury Norov <norov.maillist@...il.com> To: Joel Savitz <jsavitz@...hat.com> Cc: Cyrill Gorcunov <gorcunov@...il.com>, Waiman Long <longman@...hat.com>, linux-kernel@...r.kernel.org, Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...nel.org>, Masami Hiramatsu <mhiramat@...nel.org>, Mauro Carvalho Chehab <mchehab+samsung@...nel.org>, Kristina Martsenko <kristina.martsenko@....com>, Andrew Morton <akpm@...ux-foundation.org>, Kees Cook <keescook@...omium.org>, "Gustavo A. R. Silva" <gustavo@...eddedor.com>, YueHaibing <yuehaibing@...wei.com>, Micah Morton <mortonm@...omium.org>, Yang Shi <yang.shi@...ux.alibaba.com>, Jann Horn <jannh@...gle.com>, Alexey Dobriyan <adobriyan@...il.com>, Rafael Aquini <aquini@...hat.com>, Michael Kerrisk <mtk.manpages@...il.com> Subject: Re: [PATCH v2 0/2] sys/prctl: expose TASK_SIZE value to userspace чт, 2 мая 2019 г. в 14:23, Joel Savitz <jsavitz@...hat.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 <gorcunov@...il.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 <jsavitz@...hat.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.maillist@...il.com> wrote: > > > > чт, 2 мая 2019 г. в 12:15, Joel Savitz <jsavitz@...hat.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 <adobriyan@...il.com> > > > Signed-off-by: Joel Savitz <jsavitz@...hat.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
Powered by blists - more mailing lists