From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.5 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_PASS,USER_AGENT_MUTT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 489FAC43219 for ; Fri, 3 May 2019 22:15:09 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 23677206E0 for ; Fri, 3 May 2019 22:15:09 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727105AbfECWPI (ORCPT ); Fri, 3 May 2019 18:15:08 -0400 Received: from mx1.redhat.com ([209.132.183.28]:48614 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726042AbfECWPH (ORCPT ); Fri, 3 May 2019 18:15:07 -0400 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 527C4308425B; Fri, 3 May 2019 22:15:06 +0000 (UTC) Received: from x230.aquini.net (ovpn-120-150.rdu2.redhat.com [10.10.120.150]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 10FD65C582; Fri, 3 May 2019 22:15:00 +0000 (UTC) Date: Fri, 3 May 2019 18:14:59 -0400 From: Rafael Aquini To: Yury Norov Cc: Joel Savitz , linux-kernel@vger.kernel.org, Thomas Gleixner , Ingo Molnar , Masami Hiramatsu , Waiman Long , Mauro Carvalho Chehab , Kristina Martsenko , Andrew Morton , Cyrill Gorcunov , Kees Cook , "Gustavo A. R. Silva" , YueHaibing , Micah Morton , Yang Shi , Jann Horn , Alexey Dobriyan , Michael Kerrisk , David Laight Subject: Re: [PATCH v3 1/2] kernel/sys: add PR_GET_TASK_SIZE option to prctl(2) Message-ID: <20190503221458.GC10302@x230.aquini.net> References: <1556907021-29730-1-git-send-email-jsavitz@redhat.com> <1556907021-29730-2-git-send-email-jsavitz@redhat.com> <20190503210831.GB5887@yury-thinkpad> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20190503210831.GB5887@yury-thinkpad> User-Agent: Mutt/1.11.3 (2019-02-01) X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.40]); Fri, 03 May 2019 22:15:06 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 > > 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 > > Signed-off-by: Joel Savitz > > --- > > 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. Or we could get TIF_32BIT also defined for x86 (same value of TIF_ADDR32) and check for it instead. i.e. ... #if defined(CONFIG_64BIT) && defined(TIF_32BIT) if (test_thread_flag(TIF_32BIT)) ... which is also uglier and keeps adding unecessary complexity to a very simple task. At this point, I think we just should give up on trying this via prctl(2) and do it via /proc//status instead. > > 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