Re: [PATCH v19 02/15] arm64: Introduce prctl() options to control the tagged user addresses ABI
On Tue, Jul 23, 2019 at 07:58:39PM +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(). A Kconfig
> option allows the overall disabling of the relaxed ABI.
>
> The PR_SET_TAGGED_ADDR_CTRL will be expanded in the future to handle
> MTE-specific settings like imprecise vs precise exceptions.
>
> Reviewed-by: Kees Cook
> Signed-off-by: Catalin Marinas
> Signed-off-by: Andrey Konovalov
Following several discussions on the list and in private, I'm proposing
the update below. I can send it as a patch on top of the current series
since Will has already queued this.
---8<-
>From 1b3f57ab0c2c51f8b31c19fb34d270e1f3ee57fe Mon Sep 17 00:00:00 2001
From: Catalin Marinas
Date: Fri, 9 Aug 2019 15:09:15 +0100
Subject: [PATCH] fixup! arm64: Introduce prctl() options to control the
tagged user addresses ABI
Rename abi.tagged_addr sysctl control to abi.tagged_addr_disabled,
defaulting to 0. Only prevent prctl(PR_TAGGED_ADDR_ENABLE)from being
called when abi.tagged_addr_disabled==1.
Force unused arg* of the new prctl() to 0.
Signed-off-by: Catalin Marinas
---
arch/arm64/kernel/process.c | 17 ++---
kernel/sys.c| 4
2 files changed, 14 insertions(+), 7 deletions(-)
diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
index 76b7c55026aa..03689c0beb34 100644
--- a/arch/arm64/kernel/process.c
+++ b/arch/arm64/kernel/process.c
@@ -579,17 +579,22 @@ void arch_setup_new_exec(void)
/*
* Control the relaxed ABI allowing tagged user addresses into the kernel.
*/
-static unsigned int tagged_addr_prctl_allowed = 1;
+static unsigned int tagged_addr_disabled;
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;
+ /*
+* Do not allow the enabling of the tagged address ABI if globally
+* disabled via sysctl abi.tagged_addr_disabled.
+*/
+ if (arg & PR_TAGGED_ADDR_ENABLE && tagged_addr_disabled)
+ return -EINVAL;
+
update_thread_flag(TIF_TAGGED_ADDR, arg & PR_TAGGED_ADDR_ENABLE);
return 0;
@@ -597,8 +602,6 @@ long set_tagged_addr_ctrl(unsigned long arg)
long get_tagged_addr_ctrl(void)
{
- if (!tagged_addr_prctl_allowed)
- return -EINVAL;
if (is_compat_task())
return -EINVAL;
@@ -618,9 +621,9 @@ static int one = 1;
static struct ctl_table tagged_addr_sysctl_table[] = {
{
- .procname = "tagged_addr",
+ .procname = "tagged_addr_disabled",
.mode = 0644,
- .data = &tagged_addr_prctl_allowed,
+ .data = &tagged_addr_disabled,
.maxlen = sizeof(int),
.proc_handler = proc_dointvec_minmax,
.extra1 = &zero,
diff --git a/kernel/sys.c b/kernel/sys.c
index c6c4d5358bd3..ec48396b4943 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -2499,9 +2499,13 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2,
unsigned long, arg3,
error = PAC_RESET_KEYS(me, arg2);
break;
case PR_SET_TAGGED_ADDR_CTRL:
+ if (arg3 || arg4 || arg5)
+ 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();
break;
default:
Re: [PATCH v19 02/15] arm64: Introduce prctl() options to control the tagged user addresses ABI
On Thu, Aug 01, 2019 at 09:45:05AM -0700, Dave Hansen wrote: > On 8/1/19 5:38 AM, Kevin Brodsky wrote: > > This patch series only changes what is allowed or not at the syscall > > interface. It does not change the address space size. On arm64, TBI (Top > > Byte Ignore) has always been enabled for userspace, so it has never been > > possible to use the upper 8 bits of user pointers for addressing. > > Oh, so does the address space that's available already chop that out? Yes. Currently the hardware only supports 52-bit virtual addresses. It could be expanded (though it needs a 5th page table level) to 56-bit VA but it's not currently on our (hardware) plans. Beyond 56-bit, it cannot be done without breaking the software expectations (and hopefully I'll retire before we need this ;)). > > If other architectures were to support a similar functionality, then I > > agree that a common and more generic interface (if needed) would be > > helpful, but as it stands this is an arm64-specific prctl, and on arm64 > > the address tag is defined by the architecture as bits [63:56]. > > It should then be an arch_prctl(), no? I guess you just want renaming SET_TAGGED_ADDR_CTRL() to arch_prctl_tagged_addr_ctrl_set()? (similarly for 'get') -- Catalin
Re: [PATCH v19 02/15] arm64: Introduce prctl() options to control the tagged user addresses ABI
On 8/1/19 5:38 AM, Kevin Brodsky wrote: > This patch series only changes what is allowed or not at the syscall > interface. It does not change the address space size. On arm64, TBI (Top > Byte Ignore) has always been enabled for userspace, so it has never been > possible to use the upper 8 bits of user pointers for addressing. Oh, so does the address space that's available already chop that out? > If other architectures were to support a similar functionality, then I > agree that a common and more generic interface (if needed) would be > helpful, but as it stands this is an arm64-specific prctl, and on arm64 > the address tag is defined by the architecture as bits [63:56]. It should then be an arch_prctl(), no?
Re: [PATCH v19 02/15] arm64: Introduce prctl() options to control the tagged user addresses ABI
On 31/07/2019 18:05, Dave Hansen wrote:
On 7/23/19 10:58 AM, Andrey Konovalov wrote:
+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;
+
+ update_thread_flag(TIF_TAGGED_ADDR, arg & PR_TAGGED_ADDR_ENABLE);
+
+ return 0;
+}
Instead of a plain enable/disable, a more flexible ABI would be to have
the tag mask be passed in. That way, an implementation that has a
flexible tag size can select it. It also ensures that userspace
actually knows what the tag size is and isn't surprised if a hardware
implementation changes the tag size or position.
Also, this whole set deals with tagging/untagging, but there's an
effective loss of address space when you do this. Is that dealt with
anywhere? How do we ensure that allocations don't get placed at a
tagged address before this gets turned on? Where's that checking?
This patch series only changes what is allowed or not at the syscall interface. It
does not change the address space size. On arm64, TBI (Top Byte Ignore) has always
been enabled for userspace, so it has never been possible to use the upper 8 bits of
user pointers for addressing.
If other architectures were to support a similar functionality, then I agree that a
common and more generic interface (if needed) would be helpful, but as it stands this
is an arm64-specific prctl, and on arm64 the address tag is defined by the
architecture as bits [63:56].
Kevin
Re: [PATCH v19 02/15] arm64: Introduce prctl() options to control the tagged user addresses ABI
On 7/23/19 10:58 AM, Andrey Konovalov wrote:
> +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;
> +
> + update_thread_flag(TIF_TAGGED_ADDR, arg & PR_TAGGED_ADDR_ENABLE);
> +
> + return 0;
> +}
Instead of a plain enable/disable, a more flexible ABI would be to have
the tag mask be passed in. That way, an implementation that has a
flexible tag size can select it. It also ensures that userspace
actually knows what the tag size is and isn't surprised if a hardware
implementation changes the tag size or position.
Also, this whole set deals with tagging/untagging, but there's an
effective loss of address space when you do this. Is that dealt with
anywhere? How do we ensure that allocations don't get placed at a
tagged address before this gets turned on? Where's that checking?
[PATCH v19 02/15] arm64: Introduce prctl() options to control the tagged user addresses ABI
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(). A Kconfig
option allows the overall disabling of the relaxed ABI.
The PR_SET_TAGGED_ADDR_CTRL will be expanded in the future to handle
MTE-specific settings like imprecise vs precise exceptions.
Reviewed-by: Kees Cook
Signed-off-by: Catalin Marinas
Signed-off-by: Andrey Konovalov
---
arch/arm64/Kconfig | 9
arch/arm64/include/asm/processor.h | 8 +++
arch/arm64/include/asm/thread_info.h | 1 +
arch/arm64/include/asm/uaccess.h | 4 +-
arch/arm64/kernel/process.c | 73
include/uapi/linux/prctl.h | 5 ++
kernel/sys.c | 12 +
7 files changed, 111 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 3adcec05b1f6..5d254178b9ca 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -1110,6 +1110,15 @@ config ARM64_SW_TTBR0_PAN
zeroed area and reserved ASID. The user access routines
restore the valid TTBR0_EL1 temporarily.
+config ARM64_TAGGED_ADDR_ABI
+ bool "Enable the tagged user addresses syscall ABI"
+ default y
+ help
+ When this option is enabled, user applications can opt in to a
+ relaxed ABI via prctl() allowing tagged addresses to be passed
+ to system calls as pointer arguments. For details, see
+ Documentation/arm64/tagged-address-abi.txt.
+
menuconfig COMPAT
bool "Kernel support for 32-bit EL0"
depends on ARM64_4K_PAGES || EXPERT
diff --git a/arch/arm64/include/asm/processor.h
b/arch/arm64/include/asm/processor.h
index fd5b1a4efc70..ee86070a28d4 100644
--- a/arch/arm64/include/asm/processor.h
+++ b/arch/arm64/include/asm/processor.h
@@ -296,6 +296,14 @@ extern void __init minsigstksz_setup(void);
/* PR_PAC_RESET_KEYS prctl */
#define PAC_RESET_KEYS(tsk, arg) ptrauth_prctl_reset_keys(tsk, arg)
+#ifdef CONFIG_ARM64_TAGGED_ADDR_ABI
+/* PR_{SET,GET}_TAGGED_ADDR_CTRL 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()
+#endif
+
/*
* For CONFIG_GCC_PLUGIN_STACKLEAK
*
diff --git a/arch/arm64/include/asm/thread_info.h
b/arch/arm64/include/asm/thread_info.h
index 180b34ec5965..012238d8e58d 100644
--- a/arch/arm64/include/asm/thread_info.h
+++ b/arch/arm64/include/asm/thread_info.h
@@ -90,6 +90,7 @@ void arch_release_task_struct(struct task_struct *tsk);
#define TIF_SVE23 /* 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_ADDR26 /* 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 a138e3b4f717..097d6bfac0b7 100644
--- a/arch/arm64/include/asm/uaccess.h
+++ b/arch/arm64/include/asm/uaccess.h
@@ -62,7 +62,9 @@ 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 (IS_ENABLED(CONFIG_ARM64_TAGGED_ADDR_ABI) &&
+ 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 6a869d9f304f..ef06a303bda0 100644
--- a/arch/arm64/kernel/process.c
+++ b/arch/arm64/kernel/process.c
@@ -19,6 +19,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -38,6 +39,7 @@
#include
#include
#include
+#include
#include
#include
@@ -307,11 +309,18 @@ static void tls_thread_flush(void)
}
}
+static void flush_tagged_addr_state(void)
+{
+ if (IS_ENABLED(CONFIG_ARM64_TAGGED_ADDR_ABI))
+ clear_thread_flag(TIF_TAGGED_ADDR);
+}
+
void flush_thread(void)
{
fpsimd_flush_thread();
tls_thread_flush();
flush_ptrace_hw_breakpoint(current);
+ flush_tagged_addr_state();
}
void release_thread(struct task_struct *dead_task)
@@ -541,3 +550,67 @@ void arch_setup_new_exec(void)
ptrauth_thread_init_user(current);
}
+
+#ifdef CON
