Re: [PATCH 1/2] arm64: Support execute-only permissions with Enhanced PAN

From: Dave Martin <Dave.Martin@arm.com>
To: Vladimir Murzin <vladimir.murzin@arm.com>
Cc: catalin.marinas@arm.com, keescook@chromium.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 1/2] arm64: Support execute-only permissions with Enhanced PAN
Date: Thu, 19 Nov 2020 18:52:05 +0000	[thread overview]
Message-ID: <20201119185205.GO6882@arm.com> (raw)
In-Reply-To: <20201119133953.15585-2-vladimir.murzin@arm.com>

On Thu, Nov 19, 2020 at 01:39:52PM +0000, Vladimir Murzin wrote:
> Enhanced Privileged Access Never (EPAN) allows Privileged Access Never
> to be used with Execute-only mappings.
> 
> Absence of such support was a reason for 24cecc377463 ("arm64: Revert
> support for execute-only user mappings"). Thus now it can be revisited
> and re-enabled.
> 
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>
> ---
>  arch/arm64/Kconfig                    | 17 +++++++++++++++++
>  arch/arm64/include/asm/cpucaps.h      |  5 +++--
>  arch/arm64/include/asm/pgtable-prot.h |  5 +++--
>  arch/arm64/include/asm/pgtable.h      | 14 +++++++++++++-
>  arch/arm64/include/asm/sysreg.h       |  1 +
>  arch/arm64/kernel/cpufeature.c        | 21 +++++++++++++++++++++
>  arch/arm64/mm/fault.c                 |  3 +++
>  7 files changed, 61 insertions(+), 5 deletions(-)
> 

[...]

> diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
> index 1ee9400..b93222e 100644
> --- a/arch/arm64/mm/fault.c
> +++ b/arch/arm64/mm/fault.c
> @@ -467,6 +467,9 @@ static int __kprobes do_page_fault(unsigned long addr, unsigned int esr,
>  	if (faulthandler_disabled() || !mm)
>  		goto no_context;
>  
> +	if (cpus_have_const_cap(ARM64_HAS_EPAN))
> +		vm_flags &= ~VM_EXEC;
> +

IIUC, this would be telling __do_page_fault() that the access would have
succeeded with any kind of permissions except for write access, which
doesn't seem right.

Also, isn't vm_flags just overwritten by the code after the hunk?

The logic in __do_page_fault() looks like might not have been written
with the assumption that there might be more than a single set bit in
vm_flags.

But I'm not familiar with this code, and might be totally
misunderstanding what's going on here...

>  	if (user_mode(regs))
>  		mm_flags |= FAULT_FLAG_USER;

[...]

Cheers
---Dave

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