Hi Dave,
On Thu, Jun 13, 2019 at 12:02:35PM +0100, Dave P Martin wrote:
On Wed, Jun 12, 2019 at 01:43:20PM +0200, Andrey Konovalov wrote:
+/*
- 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.)
I had the same reaction. Maybe for another patch sanitising this pattern across the kernel.
--- 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?
I kept it generic, at least the tagged address part. The MTE bits later on would be arm64-specific.
@@ -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)<bikeshed>
How do you anticipate these arguments being used in the future?
I don't expect them to be used at all. But since I'm not sure, I'd force them as zero for now rather than ignored. The GET is supposed to return the SET arg2, hence I'd rather not used the other arguments.
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.
It's fine by me to ignore the other arguments. I just followed the pattern of some existing prctl options. I don't have a strong opinion either way.
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?
Not sure, maybe some other library (e.g. a JIT compiler) would like to check whether tagged addresses have been enabled during application start and decide to generate tagged pointers for itself. It seemed pretty harmless, unless we add more complex things to the prctl() that cannot be returned in one request).