[PATCH] arm64: reject prctl(PR_PAC_RESET_KEYS) on compat tasks
Dave Martin
Dave.Martin at arm.com
Wed Oct 14 14:00:13 EDT 2020
On Wed, Oct 14, 2020 at 10:45:48AM -0700, Peter Collingbourne wrote:
> On Wed, Oct 14, 2020 at 2:54 AM Dave Martin <Dave.Martin at arm.com> wrote:
> >
> > On Wed, Oct 14, 2020 at 06:24:30AM +0100, Peter Collingbourne wrote:
> > > It doesn't make sense to issue prctl(PR_PAC_RESET_KEYS) on a
> > > compat task because the 32-bit instruction set does not offer PAuth
> > > instructions. For consistency with other 64-bit only prctls such as
> > > {SET,GET}_TAGGED_ADDR_CTRL, reject the prctl on compat tasks.
> > >
> > > Although this is a userspace-visible change, maybe it isn't too late
> > > to make this change given that the hardware isn't available yet and
> > > it's very unlikely that anyone has 32-bit software that actually
> > > depends on this succeeding.
> > >
> > > Link: https://linux-review.googlesource.com/id/Ie885a1ff84ab498cc9f62d6451e9f2cfd4b1d06a
> > > Signed-off-by: Peter Collingbourne <pcc at google.com>
> >
> > This does seem an anomaly, but it's not an isolated case. I suspect
> > that some other prctls are also missing a compat check -- PR_SVE_SET_VL
> > doesn't have it, for example.
> >
> > So, I'm not sure it's worth fixing this one case in isolation. Fixing
> > all affected cases may have greater risk, and it won't stay fixed, since
> > the compat check will likely often get forgotten when a new prctl is
> > added.
>
> The only other affected cases involve SVE and that doesn't have
> hardware available yet either, right? I'm going by the binutils CPU
> list, which is the closest thing that I'm aware of to an official list
> of all microarchitectures and their supported ISA features:
>
> https://github.com/bminor/binutils-gdb/blob/6248f5e4fc4ad1e433156520e44ac3217c39a621/gas/config/tc-aarch64.c#L8888
>
> (and I know that Neoverse V1/N2 isn't available yet)
Yes, that's probably a reasonable list.
SVE is supported by Fujitsu's A64FX part, but that's in few pockets. In
any case, it doesn't feel legitimate for a compat process to be using
this. The chance of extra compat checks there breaking something seems
pretty low.
It's not like we ever promised it would work on compat.
> > So, is this anomaly in any way harmful?
>
> Not as far as I can tell, at least for this specific prctl.
Ack
> > Can the code be refactored in such a way as to make it hard to forget
> > the check in future?
>
> I've never been a fan of the arch-specific prctls being listed in
> kernel/sys.c. It seems better to me to have that handling be moved
> into a new arch hook and that should let us remove some boilerplate as
> well. We can make the default case in the prctl syscall handler look
> like this:
>
> default:
> return arch_handle_prctrl(option, arg2, arg3, arg4, arg5);
>
> And move the arch-specific prctls into a switch in
> arch_handle_prctl(). Now, since (as far as I can tell) all of the
> arm64-specific prctls do not make sense on compat tasks, we can add
> an:
>
> if (is_compat_task())
> return -EINVAL;
>
> to the top of arch_handle_prctl() and any new arm64-specific prctls
> will get the compat check by default. Of course, if we add a new
> compat-compatible prctl in the future, we may add it to a new switch
> before the if statement.
>
> Peter
Something like that could work.
I did have a try at cleaning some of this up some time ago [1], but
other stuff happened and I never finished it -- not sure how relevant it
still is, but might be worth a look.
Cheers
---Dave
[1] [RFC PATCH 00/11] prctl: Modernise wiring for optional prctl() calls
https://lore.kernel.org/lkml/1526318067-4964-1-git-send-email-Dave.Martin@arm.com/
More information about the linux-arm-kernel
mailing list