Re: [PATCH] arm64: reject prctl(PR_PAC_RESET_KEYS) on compat tasks

From: Dave Martin <Dave.Martin@arm.com>
To: Peter Collingbourne <pcc@google.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>,
	Vincenzo Frascino <Vincenzo.Frascino@arm.com>,
	Will Deacon <will@kernel.org>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>,
	Kristina Martsenko <kristina.martsenko@arm.com>
Subject: Re: [PATCH] arm64: reject prctl(PR_PAC_RESET_KEYS) on compat tasks
Date: Wed, 14 Oct 2020 10:53:59 +0100	[thread overview]
Message-ID: <20201014095356.GK32292@arm.com> (raw)
In-Reply-To: <20201014052430.11630-1-pcc@google.com>

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@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.

So, is this anomaly in any way harmful?

Can the code be refactored in such a way as to make it hard to forget
the check in future?

Cheers
---Dave

> ---
>  arch/arm64/kernel/pointer_auth.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/arch/arm64/kernel/pointer_auth.c b/arch/arm64/kernel/pointer_auth.c
> index 1e77736a4f66..245c3ee97ed8 100644
> --- a/arch/arm64/kernel/pointer_auth.c
> +++ b/arch/arm64/kernel/pointer_auth.c
> @@ -1,5 +1,6 @@
>  // SPDX-License-Identifier: GPL-2.0
>  
> +#include <linux/compat.h>
>  #include <linux/errno.h>
>  #include <linux/prctl.h>
>  #include <linux/random.h>
> @@ -17,6 +18,9 @@ int ptrauth_prctl_reset_keys(struct task_struct *tsk, unsigned long arg)
>  	if (!system_supports_address_auth() && !system_supports_generic_auth())
>  		return -EINVAL;
>  
> +	if (is_compat_task())
> +		return -EINVAL;
> +
>  	if (!arg) {
>  		ptrauth_keys_init_user(keys);
>  		return 0;
> -- 
> 2.28.0.1011.ga647a8990f-goog
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel