Received: by 2002:a25:ab43:0:0:0:0:0 with SMTP id u61csp7039912ybi; Thu, 13 Jun 2019 08:33:55 -0700 (PDT) X-Google-Smtp-Source: APXvYqw4pmIrlTJBFOzk2krpESWJ1qk7Qbz42F+4Lhw/XMGtXvws3cgTcfD1aExxcaKcNy3K8iAu X-Received: by 2002:a63:8dc4:: with SMTP id z187mr31594668pgd.337.1560440035127; Thu, 13 Jun 2019 08:33:55 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1560440035; cv=none; d=google.com; s=arc-20160816; b=eBQbVzy8o4DibXupxPtewV4n1fJU/4ocTYyBah/MD2v6DbmrKJ3Yh2MH0QX87NzOvs ojhusnVMqDP8P1AUGXDuCGdXMUSoDsXMvsZgopxXBjWY2X5Dwu41oITrSkaJLzA0r7ns kpAbErf+P6wae2zsjKoztsaJOUETJzNGCrEC9LWbAfjOzBaRtIcq76I33D1M/tXUQVpK bHi8set1I6Oc+ys56R9+pvkTfGx65UiugiQiIBgdt3wS2UyHcUD4PtiWnmW+9zW2b4Aw W9PQmXVNpHMFS/fUp4SngiNVC+iS7TSvMwvKGMnsDaZb34rGlM0Vaha1K7HtCYBBizdk DI8g== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:user-agent:in-reply-to :content-disposition:mime-version:references:message-id:subject:cc :to:from:date; bh=hYVaXjUB756wshctJHd6Fc9t2YTSplLyLBl41C/qXj4=; b=JuC2DzD7i4vzc9uzt9XDFSJPT4ltkxyhH1ZVkyeIH5lyDsnqOE21A/DHeUMxTavhr9 4mZZjs5f0BRUun4vLgTDq5Acipa3UoVy9SogVdM105R2PZrDtC7OGZHJrtLEO/Uspbr0 q2LV8WnZB5/6jn70MbX0p7odMUlgqdboUt49HDQ3M1WybrwDeSp/+iIoiZgcbn7YZFgf /tl/3kOhnbzDsi8+FltKTj3xa5AlNu9eQsqDtNUa5tcHhcQpaIMvDedkl9wG+PgfjX9N uYFG4H8rBTSJAB3qMkDKHfsoSolS6Cn9k1BO7sHihSyUIvrKKL8+4KAScCymwTntxUQo Unlw== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id e19si3413258pfi.91.2019.06.13.08.33.39; Thu, 13 Jun 2019 08:33:55 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728775AbfFMPbZ (ORCPT + 99 others); Thu, 13 Jun 2019 11:31:25 -0400 Received: from foss.arm.com ([217.140.110.172]:37966 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728621AbfFMLCn (ORCPT ); Thu, 13 Jun 2019 07:02:43 -0400 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 5543A3EF; Thu, 13 Jun 2019 04:02:42 -0700 (PDT) Received: from e103592.cambridge.arm.com (usa-sjc-imap-foss1.foss.arm.com [10.121.207.14]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 03BCD3F694; Thu, 13 Jun 2019 04:04:20 -0700 (PDT) Date: Thu, 13 Jun 2019 12:02:35 +0100 From: Dave Martin To: Andrey Konovalov Cc: linux-arm-kernel@lists.infradead.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, amd-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org, linux-rdma@vger.kernel.org, linux-media@vger.kernel.org, kvm@vger.kernel.org, linux-kselftest@vger.kernel.org, Mark Rutland , Szabolcs Nagy , Catalin Marinas , Will Deacon , Kostya Serebryany , Khalid Aziz , Felix Kuehling , Vincenzo Frascino , Jacob Bramley , Leon Romanovsky , Christoph Hellwig , Jason Gunthorpe , Evgeniy Stepanov , Kevin Brodsky , Kees Cook , Ruben Ayrapetyan , Ramana Radhakrishnan , Alex Williamson , Mauro Carvalho Chehab , Dmitry Vyukov , Greg Kroah-Hartman , Yishai Hadas , Jens Wiklander , Lee Smith , Alexander Deucher , Andrew Morton , enh , Robin Murphy , Christian Koenig , Luc Van Oostenryck Subject: Re: [PATCH v17 03/15] arm64: Introduce prctl() options to control the tagged user addresses ABI Message-ID: <20190613110235.GW28398@e103592.cambridge.arm.com> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Jun 12, 2019 at 01:43:20PM +0200, Andrey Konovalov wrote: > From: Catalin Marinas > > It is not desirable to relax the ABI to allow tagged user addresses into > the kernel indiscriminately. This patch introduces a prctl() interface > for enabling or disabling the tagged ABI with a global sysctl control > for preventing applications from enabling the relaxed ABI (meant for > testing user-space prctl() return error checking without reconfiguring > the kernel). The ABI properties are inherited by threads of the same > application and fork()'ed children but cleared on execve(). > > The PR_SET_TAGGED_ADDR_CTRL will be expanded in the future to handle > MTE-specific settings like imprecise vs precise exceptions. > > Signed-off-by: Catalin Marinas > --- > arch/arm64/include/asm/processor.h | 6 +++ > arch/arm64/include/asm/thread_info.h | 1 + > arch/arm64/include/asm/uaccess.h | 3 +- > arch/arm64/kernel/process.c | 67 ++++++++++++++++++++++++++++ > include/uapi/linux/prctl.h | 5 +++ > kernel/sys.c | 16 +++++++ > 6 files changed, 97 insertions(+), 1 deletion(-) > > diff --git a/arch/arm64/include/asm/processor.h b/arch/arm64/include/asm/processor.h > index fcd0e691b1ea..fee457456aa8 100644 > --- a/arch/arm64/include/asm/processor.h > +++ b/arch/arm64/include/asm/processor.h > @@ -307,6 +307,12 @@ extern void __init minsigstksz_setup(void); > /* PR_PAC_RESET_KEYS prctl */ > #define PAC_RESET_KEYS(tsk, arg) ptrauth_prctl_reset_keys(tsk, arg) > > +/* PR_TAGGED_ADDR prctl */ > +long set_tagged_addr_ctrl(unsigned long arg); > +long get_tagged_addr_ctrl(void); > +#define SET_TAGGED_ADDR_CTRL(arg) set_tagged_addr_ctrl(arg) > +#define GET_TAGGED_ADDR_CTRL() get_tagged_addr_ctrl() > + > /* > * For CONFIG_GCC_PLUGIN_STACKLEAK > * > diff --git a/arch/arm64/include/asm/thread_info.h b/arch/arm64/include/asm/thread_info.h > index f1d032be628a..354a31d2b737 100644 > --- a/arch/arm64/include/asm/thread_info.h > +++ b/arch/arm64/include/asm/thread_info.h > @@ -99,6 +99,7 @@ void arch_release_task_struct(struct task_struct *tsk); > #define TIF_SVE 23 /* Scalable Vector Extension in use */ > #define TIF_SVE_VL_INHERIT 24 /* Inherit sve_vl_onexec across exec */ > #define TIF_SSBD 25 /* Wants SSB mitigation */ > +#define TIF_TAGGED_ADDR 26 /* Allow tagged user addresses */ > > #define _TIF_SIGPENDING (1 << TIF_SIGPENDING) > #define _TIF_NEED_RESCHED (1 << TIF_NEED_RESCHED) > diff --git a/arch/arm64/include/asm/uaccess.h b/arch/arm64/include/asm/uaccess.h > index df729afca0ba..995b9ea11a89 100644 > --- a/arch/arm64/include/asm/uaccess.h > +++ b/arch/arm64/include/asm/uaccess.h > @@ -73,7 +73,8 @@ static inline unsigned long __range_ok(const void __user *addr, unsigned long si > { > unsigned long ret, limit = current_thread_info()->addr_limit; > > - addr = untagged_addr(addr); > + if (test_thread_flag(TIF_TAGGED_ADDR)) > + addr = untagged_addr(addr); > > __chk_user_ptr(addr); > asm volatile( > diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c > index 3767fb21a5b8..69d0be1fc708 100644 > --- a/arch/arm64/kernel/process.c > +++ b/arch/arm64/kernel/process.c > @@ -30,6 +30,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -323,6 +324,7 @@ void flush_thread(void) > fpsimd_flush_thread(); > tls_thread_flush(); > flush_ptrace_hw_breakpoint(current); > + clear_thread_flag(TIF_TAGGED_ADDR); > } > > void release_thread(struct task_struct *dead_task) > @@ -552,3 +554,68 @@ void arch_setup_new_exec(void) > > ptrauth_thread_init_user(current); > } > + > +/* > + * Control the relaxed ABI allowing tagged user addresses into the kernel. > + */ > +static unsigned int tagged_addr_prctl_allowed = 1; > + > +long set_tagged_addr_ctrl(unsigned long arg) > +{ > + if (!tagged_addr_prctl_allowed) > + return -EINVAL; > + if (is_compat_task()) > + return -EINVAL; > + if (arg & ~PR_TAGGED_ADDR_ENABLE) > + return -EINVAL; > + > + if (arg & PR_TAGGED_ADDR_ENABLE) > + set_thread_flag(TIF_TAGGED_ADDR); > + else > + clear_thread_flag(TIF_TAGGED_ADDR); > + > + return 0; > +} > + > +long get_tagged_addr_ctrl(void) > +{ > + if (!tagged_addr_prctl_allowed) > + return -EINVAL; > + if (is_compat_task()) > + return -EINVAL; > + > + if (test_thread_flag(TIF_TAGGED_ADDR)) > + return PR_TAGGED_ADDR_ENABLE; > + > + return 0; > +} > + > +/* > + * Global sysctl to disable the tagged user addresses support. This control > + * only prevents the tagged address ABI enabling via prctl() and does not > + * disable it for tasks that already opted in to the relaxed ABI. > + */ > +static int zero; > +static int one = 1; !!! And these can't even be const without a cast. Yuk. (Not your fault though, but it would be nice to have a proc_dobool() to avoid this.) > + > +static struct ctl_table tagged_addr_sysctl_table[] = { > + { > + .procname = "tagged_addr", > + .mode = 0644, > + .data = &tagged_addr_prctl_allowed, > + .maxlen = sizeof(int), > + .proc_handler = proc_dointvec_minmax, > + .extra1 = &zero, > + .extra2 = &one, > + }, > + { } > +}; > + > +static int __init tagged_addr_init(void) > +{ > + if (!register_sysctl("abi", tagged_addr_sysctl_table)) > + return -EINVAL; > + return 0; > +} > + > +core_initcall(tagged_addr_init); > diff --git a/include/uapi/linux/prctl.h b/include/uapi/linux/prctl.h > index 094bb03b9cc2..2e927b3e9d6c 100644 > --- a/include/uapi/linux/prctl.h > +++ b/include/uapi/linux/prctl.h > @@ -229,4 +229,9 @@ struct prctl_mm_map { > # define PR_PAC_APDBKEY (1UL << 3) > # define PR_PAC_APGAKEY (1UL << 4) > > +/* Tagged user address controls for arm64 */ > +#define PR_SET_TAGGED_ADDR_CTRL 55 > +#define PR_GET_TAGGED_ADDR_CTRL 56 > +# define PR_TAGGED_ADDR_ENABLE (1UL << 0) > + Do we expect this prctl to be applicable to other arches, or is it strictly arm64-specific? > #endif /* _LINUX_PRCTL_H */ > diff --git a/kernel/sys.c b/kernel/sys.c > index 2969304c29fe..ec48396b4943 100644 > --- a/kernel/sys.c > +++ b/kernel/sys.c > @@ -124,6 +124,12 @@ > #ifndef PAC_RESET_KEYS > # define PAC_RESET_KEYS(a, b) (-EINVAL) > #endif > +#ifndef SET_TAGGED_ADDR_CTRL > +# define SET_TAGGED_ADDR_CTRL(a) (-EINVAL) > +#endif > +#ifndef GET_TAGGED_ADDR_CTRL > +# define GET_TAGGED_ADDR_CTRL() (-EINVAL) > +#endif > > /* > * this is where the system-wide overflow UID and GID are defined, for > @@ -2492,6 +2498,16 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3, > return -EINVAL; > error = PAC_RESET_KEYS(me, arg2); > break; > + case PR_SET_TAGGED_ADDR_CTRL: > + if (arg3 || arg4 || arg5) How do you anticipate these arguments being used in the future? For the SVE prctls I took the view that "get" could only ever mean one thing, and "put" already had a flags argument with spare bits for future expansion anyway, so forcing the extra arguments to zero would be unnecessary. Opinions seem to differ on whether requiring surplus arguments to be 0 is beneficial for hygiene, but the glibc prototype for prctl() is int prctl (int __option, ...); so it seemed annoying to have to pass extra arguments to it just for the sake of it. IMHO this also makes the code at the call site less readable, since it's not immediately apparent that all those 0s are meaningless. (OTOH, the extra arguments are harmless and prctl is far from being a general-purpose syscall.) > + return -EINVAL; > + error = SET_TAGGED_ADDR_CTRL(arg2); > + break; > + case PR_GET_TAGGED_ADDR_CTRL: > + if (arg2 || arg3 || arg4 || arg5) > + return -EINVAL; > + error = GET_TAGGED_ADDR_CTRL(); Having a "get" prctl is probably a good idea, but is there a clear usecase for it? (The usecase for PR_SVE_GET_VL was always a bit dubious, since the VL can also be read via an SVE insn or a compiler intrinsic, which is less portable but much cheaper. As for the PR_SVE_SET_VL_INHERIT flag that can be read via PR_SVE_GET_VL, I've never been sure how useful it is to be able to read that...) [...] Cheers ---Dave