Commit 22f3a4f6 authored by Kevin Brodsky's avatar Kevin Brodsky Committed by Will Deacon
Browse files

arm64: poe: Handle spurious Overlay faults

We do not currently issue an ISB after updating POR_EL0 when
context-switching it, for instance. The rationale is that if the old
value of POR_EL0 is more restrictive and causes a fault during
uaccess, the access will be retried [1]. In other words, we are
trading an ISB on every context-switching for the (unlikely)
possibility of a spurious fault. We may also miss faults if the new
value of POR_EL0 is more restrictive, but that's considered
acceptable.

However, as things stand, a spurious Overlay fault results in
uaccess failing right away since it causes fault_from_pkey() to
return true. If an Overlay fault is reported, we therefore need to
double check POR_EL0 against vma_pkey(vma) - this is what
arch_vma_access_permitted() already does.

As it turns out, we already perform that explicit check if no
Overlay fault is reported, and we need to keep that check (see
comment added in fault_from_pkey()). Net result: the Overlay ISS2
bit isn't of much help to decide whether a pkey fault occurred.

Remove the check for the Overlay bit from fault_from_pkey() and
add a comment to try and explain the situation. While at it, also
add a comment to permission_overlay_switch() in case anyone gets
surprised by the lack of ISB.

[1] https://lore.kernel.org/linux-arm-kernel/ZtYNGBrcE-j35fpw@arm.com/



Fixes: 160a8e13 ("arm64: context switch POR_EL0 register")
Signed-off-by: default avatarKevin Brodsky <kevin.brodsky@arm.com>
Link: https://lore.kernel.org/r/20250619160042.2499290-2-kevin.brodsky@arm.com


Signed-off-by: default avatarWill Deacon <will@kernel.org>
parent a75ad2fc
Loading
Loading
Loading
Loading
+5 −0
Original line number Diff line number Diff line
@@ -673,6 +673,11 @@ static void permission_overlay_switch(struct task_struct *next)
	current->thread.por_el0 = read_sysreg_s(SYS_POR_EL0);
	if (current->thread.por_el0 != next->thread.por_el0) {
		write_sysreg_s(next->thread.por_el0, SYS_POR_EL0);
		/*
		 * No ISB required as we can tolerate spurious Overlay faults -
		 * the fault handler will check again based on the new value
		 * of POR_EL0.
		 */
	}
}

+21 −9
Original line number Diff line number Diff line
@@ -487,17 +487,29 @@ static void do_bad_area(unsigned long far, unsigned long esr,
	}
}

static bool fault_from_pkey(unsigned long esr, struct vm_area_struct *vma,
			unsigned int mm_flags)
static bool fault_from_pkey(struct vm_area_struct *vma, unsigned int mm_flags)
{
	unsigned long iss2 = ESR_ELx_ISS2(esr);

	if (!system_supports_poe())
		return false;

	if (esr_fsc_is_permission_fault(esr) && (iss2 & ESR_ELx_Overlay))
		return true;

	/*
	 * We do not check whether an Overlay fault has occurred because we
	 * cannot make a decision based solely on its value:
	 *
	 * - If Overlay is set, a fault did occur due to POE, but it may be
	 *   spurious in those cases where we update POR_EL0 without ISB (e.g.
	 *   on context-switch). We would then need to manually check POR_EL0
	 *   against vma_pkey(vma), which is exactly what
	 *   arch_vma_access_permitted() does.
	 *
	 * - If Overlay is not set, we may still need to report a pkey fault.
	 *   This is the case if an access was made within a mapping but with no
	 *   page mapped, and POR_EL0 forbids the access (according to
	 *   vma_pkey()). Such access will result in a SIGSEGV regardless
	 *   because core code checks arch_vma_access_permitted(), but in order
	 *   to report the correct error code - SEGV_PKUERR - we must handle
	 *   that case here.
	 */
	return !arch_vma_access_permitted(vma,
			mm_flags & FAULT_FLAG_WRITE,
			mm_flags & FAULT_FLAG_INSTRUCTION,
@@ -635,7 +647,7 @@ static int __kprobes do_page_fault(unsigned long far, unsigned long esr,
		goto bad_area;
	}

	if (fault_from_pkey(esr, vma, mm_flags)) {
	if (fault_from_pkey(vma, mm_flags)) {
		pkey = vma_pkey(vma);
		vma_end_read(vma);
		fault = 0;
@@ -679,7 +691,7 @@ static int __kprobes do_page_fault(unsigned long far, unsigned long esr,
		goto bad_area;
	}

	if (fault_from_pkey(esr, vma, mm_flags)) {
	if (fault_from_pkey(vma, mm_flags)) {
		pkey = vma_pkey(vma);
		mmap_read_unlock(mm);
		fault = 0;