From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751541AbbEEAgj (ORCPT ); Mon, 4 May 2015 20:36:39 -0400 Received: from mail-by2on0065.outbound.protection.outlook.com ([207.46.100.65]:61130 "EHLO na01-by2-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751102AbbEEAgb convert rfc822-to-8bit (ORCPT ); Mon, 4 May 2015 20:36:31 -0400 X-Greylist: delayed 1105 seconds by postgrey-1.27 at vger.kernel.org; Mon, 04 May 2015 20:36:31 EDT From: "Pinski, Andrew" To: Philipp Tomsich CC: "linux-arm-kernel@lists.infradead.org" , "linux-kernel@vger.kernel.org" , Andrew Pinski , Christoph Muellner , "Benedikt Huber" , Andreas Kraschitzer , Kumar Sankaran , Catalin Marinas Subject: Re: [PATCH v4 22/24] arm64:ilp32: use compat for stack_t Thread-Topic: [PATCH v4 22/24] arm64:ilp32: use compat for stack_t Thread-Index: AQHQdib0Y4eMGVcvuEGD9jHkGUwopZ1soYi1 Date: Tue, 5 May 2015 00:03:30 +0000 Message-ID: <133D58DB-61A4-4BA8-A683-B20C70753E69@caviumnetworks.com> References: ,<09081413bd99e0a04e9783dc44fcd52eaeab1af1.1428953303.git.philipp.tomsich@theobroma-systems.com> In-Reply-To: <09081413bd99e0a04e9783dc44fcd52eaeab1af1.1428953303.git.philipp.tomsich@theobroma-systems.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: lists.infradead.org; dkim=none (message not signed) header.d=none; x-originating-ip: [64.2.3.194] x-microsoft-antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:CY1PR0701MB1243; x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:; x-exchange-antispam-report-cfa-test: BCL:0;PCL:0;RULEID:(601004)(5005006)(3002001);SRVR:CY1PR0701MB1243;BCL:0;PCL:0;RULEID:;SRVR:CY1PR0701MB1243; x-forefront-prvs: 0567A15835 x-forefront-antispam-report: SFV:NSPM;SFS:(10009020)(6009001)(51704005)(164054003)(24454002)(377454003)(83716003)(2900100001)(2656002)(2950100001)(86362001)(46102003)(110136002)(106116001)(5001960100002)(19580405001)(82746002)(92566002)(33656002)(19580395003)(77156002)(62966003)(102836002)(54356999)(122556002)(87936001)(50986999)(76176999)(99286002)(66066001)(36756003)(104396002);DIR:OUT;SFP:1101;SCL:1;SRVR:CY1PR0701MB1243;H:CY1PR0701MB1244.namprd07.prod.outlook.com;FPR:;SPF:None;MLV:sfv;LANG:en; Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 8BIT MIME-Version: 1.0 X-OriginatorOrg: caviumnetworks.com X-MS-Exchange-CrossTenant-originalarrivaltime: 05 May 2015 00:03:30.4726 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 711e4ccf-2e9b-4bcf-a551-4094005b6194 X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY1PR0701MB1243 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org > On Apr 13, 2015, at 1:18 PM, Philipp Tomsich wrote: > > We use a 'natively sized' stack_t in glibc (i.e. having a 32bit pointer for > ss_sp), which requires the invocation of the compat layer for the following > functionality: > * sigaltstack > * saving and restoring uc_stack during signal setup and returns Can you explain why you want to use a natively sized stack_t? My patches for glibc included changing stack_t too. I would rather keep the same size stack_t between lp64 and ilp32 due easier gdb support. Thanks, Andrew > > As the userspace stack_t is natively sized, we avoid code duplication in the > syscall table and can use the compat-functions to zero-extend the pointers > involved. > > Signed-off-by: Philipp Tomsich > Signed-off-by: Christoph Muellner > --- > arch/arm64/kernel/signal.c | 19 +++++++++++++++++++ > arch/arm64/kernel/sys_ilp32.c | 44 +------------------------------------------ > 2 files changed, 20 insertions(+), 43 deletions(-) > > diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c > index 99e36be..b3f6e52 100644 > --- a/arch/arm64/kernel/signal.c > +++ b/arch/arm64/kernel/signal.c > @@ -34,6 +34,7 @@ > #include > #include > #include > +#include > > /* > * Do a signal return; undo the signal stack. These are aligned to 128-bit. > @@ -148,9 +149,22 @@ asmlinkage long sys_rt_sigreturn(struct pt_regs *regs) > if (restore_sigframe(regs, frame)) > goto badframe; > > + > +#if defined(CONFIG_ARM64_ILP32) > + if (is_ilp32_compat_task()) { > + /* For ILP32, we have a different stack_t (the ss_sp > + field will be only 32bit sized), which fits into > + the memory area reserved for the (larger) LP64 > + stack_t and which we place into uc_stack: this > + implies padding after the ILP32 stack_t. */ > + if (compat_restore_altstack((compat_stack_t*)&frame->uc.uc_stack)) > + goto badframe; > + } else > +#endif > if (restore_altstack(&frame->uc.uc_stack)) > goto badframe; > > + > return regs->regs[0]; > > badframe: > @@ -264,6 +278,11 @@ static int setup_rt_frame(int usig, struct ksignal *ksig, sigset_t *set, > __put_user_error(0, &frame->uc.uc_flags, err); > __put_user_error(NULL, &frame->uc.uc_link, err); > > +#if defined(CONFIG_ARM64_ILP32) > + if (is_ilp32_compat_task()) > + err |= __compat_save_altstack((compat_stack_t*)&frame->uc.uc_stack, regs->sp); > + else > +#endif > err |= __save_altstack(&frame->uc.uc_stack, regs->sp); > err |= setup_sigframe(frame, regs, set); > if (err == 0) { > diff --git a/arch/arm64/kernel/sys_ilp32.c b/arch/arm64/kernel/sys_ilp32.c > index 3471f27..31f82ca 100644 > --- a/arch/arm64/kernel/sys_ilp32.c > +++ b/arch/arm64/kernel/sys_ilp32.c > @@ -77,6 +77,7 @@ > > /* Pointer in struct */ > #define sys_mount compat_sys_mount > +#define sys_sigaltstack compat_sys_sigaltstack > > /* NUMA */ > /* unsigned long bitmaps */ > @@ -122,49 +123,6 @@ asmlinkage long ilp32_sys_mq_notify(mqd_t mqdes, const struct sigevent __user *u > but need special handling due to padding for SIGEV_THREAD. */ > #define sys_mq_notify ilp32_sys_mq_notify > > - > -/* sigaltstack needs some special handling as the > - padding for stack_t might not be non-zero. */ > -long ilp32_sys_sigaltstack(const stack_t __user *uss_ptr, > - stack_t __user *uoss_ptr) > -{ > - stack_t uss, uoss; > - int ret; > - mm_segment_t seg; > - > - if (uss_ptr) { > - if (!access_ok(VERIFY_READ, uss_ptr, sizeof(*uss_ptr))) > - return -EFAULT; > - if (__get_user(uss.ss_sp, &uss_ptr->ss_sp) | > - __get_user(uss.ss_flags, &uss_ptr->ss_flags) | > - __get_user(uss.ss_size, &uss_ptr->ss_size)) > - return -EFAULT; > - /* Zero extend the sp address and the size. */ > - uss.ss_sp = (void *)(uintptr_t)(unsigned int)(uintptr_t)uss.ss_sp; > - uss.ss_size = (size_t)(unsigned int)uss.ss_size; > - } > - seg = get_fs(); > - set_fs(KERNEL_DS); > - /* Note we need to use uoss as we have changed the segment to the > - kernel one so passing an user one around is wrong. */ > - ret = sys_sigaltstack((stack_t __force __user *) (uss_ptr ? &uss : NULL), > - (stack_t __force __user *) &uoss); > - set_fs(seg); > - if (ret >= 0 && uoss_ptr) { > - if (!access_ok(VERIFY_WRITE, uoss_ptr, sizeof(stack_t)) || > - __put_user(uoss.ss_sp, &uoss_ptr->ss_sp) || > - __put_user(uoss.ss_flags, &uoss_ptr->ss_flags) || > - __put_user(uoss.ss_size, &uoss_ptr->ss_size)) > - ret = -EFAULT; > - } > - return ret; > -} > - > -/* sigaltstack needs some special handling as the padding > - for stack_t might not be non-zero. */ > -#define sys_sigaltstack ilp32_sys_sigaltstack > - > - > #include > > #undef __SYSCALL > -- > 1.9.1 > From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew.Pinski@caviumnetworks.com (Pinski, Andrew) Date: Tue, 5 May 2015 00:03:30 +0000 Subject: [PATCH v4 22/24] arm64:ilp32: use compat for stack_t In-Reply-To: <09081413bd99e0a04e9783dc44fcd52eaeab1af1.1428953303.git.philipp.tomsich@theobroma-systems.com> References: , <09081413bd99e0a04e9783dc44fcd52eaeab1af1.1428953303.git.philipp.tomsich@theobroma-systems.com> Message-ID: <133D58DB-61A4-4BA8-A683-B20C70753E69@caviumnetworks.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org > On Apr 13, 2015, at 1:18 PM, Philipp Tomsich wrote: > > We use a 'natively sized' stack_t in glibc (i.e. having a 32bit pointer for > ss_sp), which requires the invocation of the compat layer for the following > functionality: > * sigaltstack > * saving and restoring uc_stack during signal setup and returns Can you explain why you want to use a natively sized stack_t? My patches for glibc included changing stack_t too. I would rather keep the same size stack_t between lp64 and ilp32 due easier gdb support. Thanks, Andrew > > As the userspace stack_t is natively sized, we avoid code duplication in the > syscall table and can use the compat-functions to zero-extend the pointers > involved. > > Signed-off-by: Philipp Tomsich > Signed-off-by: Christoph Muellner > --- > arch/arm64/kernel/signal.c | 19 +++++++++++++++++++ > arch/arm64/kernel/sys_ilp32.c | 44 +------------------------------------------ > 2 files changed, 20 insertions(+), 43 deletions(-) > > diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c > index 99e36be..b3f6e52 100644 > --- a/arch/arm64/kernel/signal.c > +++ b/arch/arm64/kernel/signal.c > @@ -34,6 +34,7 @@ > #include > #include > #include > +#include > > /* > * Do a signal return; undo the signal stack. These are aligned to 128-bit. > @@ -148,9 +149,22 @@ asmlinkage long sys_rt_sigreturn(struct pt_regs *regs) > if (restore_sigframe(regs, frame)) > goto badframe; > > + > +#if defined(CONFIG_ARM64_ILP32) > + if (is_ilp32_compat_task()) { > + /* For ILP32, we have a different stack_t (the ss_sp > + field will be only 32bit sized), which fits into > + the memory area reserved for the (larger) LP64 > + stack_t and which we place into uc_stack: this > + implies padding after the ILP32 stack_t. */ > + if (compat_restore_altstack((compat_stack_t*)&frame->uc.uc_stack)) > + goto badframe; > + } else > +#endif > if (restore_altstack(&frame->uc.uc_stack)) > goto badframe; > > + > return regs->regs[0]; > > badframe: > @@ -264,6 +278,11 @@ static int setup_rt_frame(int usig, struct ksignal *ksig, sigset_t *set, > __put_user_error(0, &frame->uc.uc_flags, err); > __put_user_error(NULL, &frame->uc.uc_link, err); > > +#if defined(CONFIG_ARM64_ILP32) > + if (is_ilp32_compat_task()) > + err |= __compat_save_altstack((compat_stack_t*)&frame->uc.uc_stack, regs->sp); > + else > +#endif > err |= __save_altstack(&frame->uc.uc_stack, regs->sp); > err |= setup_sigframe(frame, regs, set); > if (err == 0) { > diff --git a/arch/arm64/kernel/sys_ilp32.c b/arch/arm64/kernel/sys_ilp32.c > index 3471f27..31f82ca 100644 > --- a/arch/arm64/kernel/sys_ilp32.c > +++ b/arch/arm64/kernel/sys_ilp32.c > @@ -77,6 +77,7 @@ > > /* Pointer in struct */ > #define sys_mount compat_sys_mount > +#define sys_sigaltstack compat_sys_sigaltstack > > /* NUMA */ > /* unsigned long bitmaps */ > @@ -122,49 +123,6 @@ asmlinkage long ilp32_sys_mq_notify(mqd_t mqdes, const struct sigevent __user *u > but need special handling due to padding for SIGEV_THREAD. */ > #define sys_mq_notify ilp32_sys_mq_notify > > - > -/* sigaltstack needs some special handling as the > - padding for stack_t might not be non-zero. */ > -long ilp32_sys_sigaltstack(const stack_t __user *uss_ptr, > - stack_t __user *uoss_ptr) > -{ > - stack_t uss, uoss; > - int ret; > - mm_segment_t seg; > - > - if (uss_ptr) { > - if (!access_ok(VERIFY_READ, uss_ptr, sizeof(*uss_ptr))) > - return -EFAULT; > - if (__get_user(uss.ss_sp, &uss_ptr->ss_sp) | > - __get_user(uss.ss_flags, &uss_ptr->ss_flags) | > - __get_user(uss.ss_size, &uss_ptr->ss_size)) > - return -EFAULT; > - /* Zero extend the sp address and the size. */ > - uss.ss_sp = (void *)(uintptr_t)(unsigned int)(uintptr_t)uss.ss_sp; > - uss.ss_size = (size_t)(unsigned int)uss.ss_size; > - } > - seg = get_fs(); > - set_fs(KERNEL_DS); > - /* Note we need to use uoss as we have changed the segment to the > - kernel one so passing an user one around is wrong. */ > - ret = sys_sigaltstack((stack_t __force __user *) (uss_ptr ? &uss : NULL), > - (stack_t __force __user *) &uoss); > - set_fs(seg); > - if (ret >= 0 && uoss_ptr) { > - if (!access_ok(VERIFY_WRITE, uoss_ptr, sizeof(stack_t)) || > - __put_user(uoss.ss_sp, &uoss_ptr->ss_sp) || > - __put_user(uoss.ss_flags, &uoss_ptr->ss_flags) || > - __put_user(uoss.ss_size, &uoss_ptr->ss_size)) > - ret = -EFAULT; > - } > - return ret; > -} > - > -/* sigaltstack needs some special handling as the padding > - for stack_t might not be non-zero. */ > -#define sys_sigaltstack ilp32_sys_sigaltstack > - > - > #include > > #undef __SYSCALL > -- > 1.9.1 >