On Mon, Jun 10, 2019 at 06:53:27PM +0100, Catalin Marinas wrote:
On Mon, Jun 03, 2019 at 06:55:04PM +0200, Andrey Konovalov wrote:
diff --git a/arch/arm64/include/asm/uaccess.h b/arch/arm64/include/asm/uaccess.h index e5d5f31c6d36..9164ecb5feca 100644 --- a/arch/arm64/include/asm/uaccess.h +++ b/arch/arm64/include/asm/uaccess.h @@ -94,7 +94,7 @@ static inline unsigned long __range_ok(const void __user *addr, unsigned long si return ret; } -#define access_ok(addr, size) __range_ok(addr, size) +#define access_ok(addr, size) __range_ok(untagged_addr(addr), size)
I'm going to propose an opt-in method here (RFC for now). We can't have a check in untagged_addr() since this is already used throughout the kernel for both user and kernel addresses (khwasan) but we can add one in __range_ok(). The same prctl() option will be used for controlling the precise/imprecise mode of MTE later on. We can use a TIF_ flag here assuming that this will be called early on and any cloned thread will inherit this.
Anyway, it's easier to paste some diff than explain but Vincenzo can fold them into his ABI patches that should really go together with these. I added a couple of MTE definitions for prctl() as an example, not used currently:
------------------8<--------------------------------------------- diff --git a/arch/arm64/include/asm/processor.h b/arch/arm64/include/asm/processor.h index fcd0e691b1ea..2d4cb7e4edab 100644 --- a/arch/arm64/include/asm/processor.h +++ b/arch/arm64/include/asm/processor.h @@ -307,6 +307,10 @@ extern void __init minsigstksz_setup(void); /* PR_PAC_RESET_KEYS prctl */ #define PAC_RESET_KEYS(tsk, arg) ptrauth_prctl_reset_keys(tsk, arg) +/* PR_UNTAGGED_UADDR prctl */ +int untagged_uaddr_set_mode(unsigned long arg); +#define SET_UNTAGGED_UADDR_MODE(arg) untagged_uaddr_set_mode(arg)
/*
- For CONFIG_GCC_PLUGIN_STACKLEAK
diff --git a/arch/arm64/include/asm/thread_info.h b/arch/arm64/include/asm/thread_info.h index c285d1ce7186..89ce77773c49 100644 --- a/arch/arm64/include/asm/thread_info.h +++ b/arch/arm64/include/asm/thread_info.h @@ -101,6 +101,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_UNTAGGED_UADDR 26 #define _TIF_SIGPENDING (1 << TIF_SIGPENDING) #define _TIF_NEED_RESCHED (1 << TIF_NEED_RESCHED) @@ -116,6 +117,7 @@ void arch_release_task_struct(struct task_struct *tsk); #define _TIF_FSCHECK (1 << TIF_FSCHECK) #define _TIF_32BIT (1 << TIF_32BIT) #define _TIF_SVE (1 << TIF_SVE) +#define _TIF_UNTAGGED_UADDR (1 << TIF_UNTAGGED_UADDR) #define _TIF_WORK_MASK (_TIF_NEED_RESCHED | _TIF_SIGPENDING | \ _TIF_NOTIFY_RESUME | _TIF_FOREIGN_FPSTATE | \ diff --git a/arch/arm64/include/asm/uaccess.h b/arch/arm64/include/asm/uaccess.h index 9164ecb5feca..54f5bbaebbc4 100644 --- a/arch/arm64/include/asm/uaccess.h +++ b/arch/arm64/include/asm/uaccess.h @@ -73,6 +73,9 @@ static inline unsigned long __range_ok(const void __user *addr, unsigned long si { unsigned long ret, limit = current_thread_info()->addr_limit;
- if (test_thread_flag(TIF_UNTAGGED_UADDR))
addr = untagged_addr(addr);- __chk_user_ptr(addr); asm volatile( // A + B <= C + 1 for all A,B,C, in four easy steps:
@@ -94,7 +97,7 @@ static inline unsigned long __range_ok(const void __user *addr, unsigned long si return ret; } -#define access_ok(addr, size) __range_ok(untagged_addr(addr), size) +#define access_ok(addr, size) __range_ok(addr, size) #define user_addr_max get_fs #define _ASM_EXTABLE(from, to) \ diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c index 3767fb21a5b8..fd191c5b92aa 100644 --- a/arch/arm64/kernel/process.c +++ b/arch/arm64/kernel/process.c @@ -552,3 +552,18 @@ void arch_setup_new_exec(void) ptrauth_thread_init_user(current); }
+/*
- Enable the relaxed ABI allowing tagged user addresses into the kernel.
- */
+int untagged_uaddr_set_mode(unsigned long arg) +{
- if (is_compat_task())
return -ENOTSUPP;- if (arg)
return -EINVAL;- set_thread_flag(TIF_UNTAGGED_UADDR);
- return 0;
+}
I think this should be paired with a flag clearing in copy_thread(), yes? (i.e. each binary needs to opt in)