Hi Catalin,
On 12/06/2019 10:32, Catalin Marinas wrote:
Hi Vincenzo,
On Tue, Jun 11, 2019 at 06:09:10PM +0100, Vincenzo Frascino wrote:
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);
Nit: in line we the other functions in thread_flush we could have something like "tagged_addr_thread_flush", maybe inlined.
The other functions do a lot more than clearing a TIF flag, so they deserved their own place. We could do this when adding MTE support. I think we also need to check what other TIF flags we may inadvertently pass on execve(), maybe have a mask clearing.
Agreed. All the comments I provided are meant to simplify the addition of MTE support.
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)
#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)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;Why do we need two prctl here? We could have only one and use arg2 as set/get and arg3 as a parameter. What do you think?
This follows the other PR_* options, e.g. PR_SET_VL/GET_VL, PR_*_FP_MODE. We will use other bits in arg2, for example to set the precise vs imprecise MTE trapping.
Indeed. I was not questioning the pre-existing interface definition, but trying more to reduce the changes to the ABI to the minimum since: - prctl does not mandate how to use the arg[2-5] - prctl interface is flexible enough for the problem to be solved with only one PR_ command.
I agree on reusing the interface for MTE for the purposes you specified.