| 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
| ||
|
Date: Thu, 13 Jun 2019 12:16:59 +0100
From: Dave Martin <Dave.Martin@....com>
To: Andrey Konovalov <andreyknvl@...gle.com>
Cc: linux-arm-kernel@...ts.infradead.org, linux-mm@...ck.org,
linux-kernel@...r.kernel.org, amd-gfx@...ts.freedesktop.org,
dri-devel@...ts.freedesktop.org, linux-rdma@...r.kernel.org,
linux-media@...r.kernel.org, kvm@...r.kernel.org,
linux-kselftest@...r.kernel.org,
Mark Rutland <mark.rutland@....com>,
Szabolcs Nagy <Szabolcs.Nagy@....com>,
Catalin Marinas <catalin.marinas@....com>,
Will Deacon <will.deacon@....com>,
Kostya Serebryany <kcc@...gle.com>,
Khalid Aziz <khalid.aziz@...cle.com>,
Felix Kuehling <Felix.Kuehling@....com>,
Vincenzo Frascino <vincenzo.frascino@....com>,
Jacob Bramley <Jacob.Bramley@....com>,
Leon Romanovsky <leon@...nel.org>,
Christoph Hellwig <hch@...radead.org>,
Jason Gunthorpe <jgg@...pe.ca>,
Evgeniy Stepanov <eugenis@...gle.com>,
Kevin Brodsky <kevin.brodsky@....com>,
Kees Cook <keescook@...omium.org>,
Ruben Ayrapetyan <Ruben.Ayrapetyan@....com>,
Ramana Radhakrishnan <Ramana.Radhakrishnan@....com>,
Alex Williamson <alex.williamson@...hat.com>,
Mauro Carvalho Chehab <mchehab@...nel.org>,
Dmitry Vyukov <dvyukov@...gle.com>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Yishai Hadas <yishaih@...lanox.com>,
Jens Wiklander <jens.wiklander@...aro.org>,
Lee Smith <Lee.Smith@....com>,
Alexander Deucher <Alexander.Deucher@....com>,
Andrew Morton <akpm@...ux-foundation.org>,
enh <enh@...gle.com>, Robin Murphy <robin.murphy@....com>,
Christian Koenig <Christian.Koenig@....com>,
Luc Van Oostenryck <luc.vanoostenryck@...il.com>
Subject: Re: [PATCH v17 03/15] arm64: Introduce prctl() options to control
the tagged user addresses ABI
On Wed, Jun 12, 2019 at 01:43:20PM +0200, Andrey Konovalov wrote:
> From: Catalin Marinas <catalin.marinas@....com>
>
> 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 <catalin.marinas@....com>
> ---
> 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 */
(A couple of comments I missed in my last reply:)
Name mismatch?
> +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()
> +
[...]
> 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 <linux/kernel.h>
> #include <linux/mm.h>
> #include <linux/stddef.h>
> +#include <linux/sysctl.h>
> #include <linux/unistd.h>
> #include <linux/user.h>
> #include <linux/delay.h>
> @@ -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;
So, tagging can actually be locked on by having a process enable it and
then some possibly unrelated process clearing tagged_addr_prctl_allowed.
That feels a bit weird.
Do we want to allow a process that has tagging on to be able to turn
it off at all? Possibly things like CRIU might want to do that.
> + if (is_compat_task())
> + return -EINVAL;
> + if (arg & ~PR_TAGGED_ADDR_ENABLE)
> + return -EINVAL;
How do we expect this argument to be extended in the future?
I'm wondering whether this is really a bitmask or an enum, or a mixture
of the two. Maybe it doesn't matter.
> +
> + if (arg & PR_TAGGED_ADDR_ENABLE)
> + set_thread_flag(TIF_TAGGED_ADDR);
> + else
> + clear_thread_flag(TIF_TAGGED_ADDR);
I think update_thread_flag() could be used here.
[...]
Cheers
---Dave
Powered by blists - more mailing lists