[PATCH 0/3] ARM: fix hash_name() and branch predictor issues

[PATCH 0/3] ARM: fix hash_name() and branch predictor issues

[Russell King (Oracle)]

This series, which I'm intending to be part of my upcoming pull request
for this merge window (note - will be sent before Thursday, otherwise
it won't be until maybe the middle of January) fixes the recently
reported hash_name() issue, and also the long standing branch predictor
hardening smp_processor_id() warning which occurs due to interrupts
being enabled.

Fixing this isn't trivial because there are many paths through
do_page_fault() which are non-obvious.

For example, we detect page faults by the kernel attempting to execute
memory that it shouldn't and print such a warning. If we introduce an
early address check, we avoid that detection, making it become a
plain kernel oops without the informative error message.

The first patch adds an additional check in __do_kernel_fault() to
detect this condition. This patch is a non-obvious dependency for the
next patch.

The second patch handles faults above the top of userspace entirely
separately, meaning we have a simpler and more obvious fault path,
which avoids any possibility of taking any MM mutexes, which is the
cause of the hash_name() problem.

The third patch moves harden_branch_predictor() out of
__do_user_fault() and to appropriate places in the parent functions.
The reason this has been avoided thus far is because do_page_fault()
can be a hot path (since it's used for page aging as well) but with
kernel address faults being handled by an entirely separate path,
this avoids adding to that overhead.

I would like to get some attributations for this - specifically
tested-by.

Thanks.

 arch/arm/mm/alignment.c |  6 +++-
 arch/arm/mm/fault.c     | 74 +++++++++++++++++++++++++++++++++++++++++--------
 2 files changed, 67 insertions(+), 13 deletions(-)

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

[PATCH 1/3] ARM: allow __do_kernel_fault() to report execution of memory faults

[Russell King (Oracle)]

Allow __do_kernel_fault() to detect the execution of memory, so we can
provide the same fault message as do_page_fault() would do. This is
required when we split the kernel address fault handling from the
main do_page_fault() code path.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 arch/arm/mm/fault.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c
index 2bc828a1940c..f0884bf91dfa 100644
--- a/arch/arm/mm/fault.c
+++ b/arch/arm/mm/fault.c
@@ -162,6 +162,8 @@ __do_kernel_fault(struct mm_struct *mm, unsigned long addr, unsigned int fsr,
 	 */
 	if (addr < PAGE_SIZE) {
 		msg = "NULL pointer dereference";
+	} else if (is_permission_fault(fsr) && fsr & FSR_LNX_PF) {
+		msg = "execution of memory";
 	} else {
 		if (is_translation_fault(fsr) &&
 		    kfence_handle_page_fault(addr, is_write_fault(fsr), regs))
-- 
2.47.3

Re: [PATCH 1/3] ARM: allow __do_kernel_fault() to report execution of memory faults

[Xie Yuanbin]

On Mon, 08 Dec 2025 12:34:54 +0000, Russell King wrote:
> diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c
> index 2bc828a1940c..f0884bf91dfa 100644
> --- a/arch/arm/mm/fault.c
> +++ b/arch/arm/mm/fault.c
> @@ -162,6 +162,8 @@ __do_kernel_fault(struct mm_struct *mm, unsigned long addr, unsigned int fsr,
>  	 */
>  	if (addr < PAGE_SIZE) {
>  		msg = "NULL pointer dereference";
> +	} else if (is_permission_fault(fsr) && fsr & FSR_LNX_PF) {
> +		msg = "execution of memory";
>  	} else {
>  		if (is_translation_fault(fsr) &&
>  		    kfence_handle_page_fault(addr, is_write_fault(fsr), regs))

This patch caused a compilation error, is_permission_fault() is not
defined. Fixed with:
```patch
diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c
index 6ba5a11cc..066408523 100644
--- a/arch/arm/mm/fault.c
+++ b/arch/arm/mm/fault.c
@@ -192,6 +192,21 @@ static void die_kernel_fault(const char *msg, struct mm_struct *mm,
 	make_task_dead(SIGKILL);
 }

+static inline bool is_permission_fault(unsigned int fsr)
+{
+#ifdef CONFIG_MMU
+	int fs = fsr_fs(fsr);
+#ifdef CONFIG_ARM_LPAE
+	if ((fs & FS_MMU_NOLL_MASK) == FS_PERM_NOLL)
+		return true;
+#else
+	if (fs == FS_L1_PERM || fs == FS_L2_PERM)
+		return true;
+#endif
+#endif
+	return false;
+}
+
 /*
  * Oops.  The kernel tried to access some page that wasn't present.
  */
@@ -305,19 +320,6 @@ void do_bad_area_user_fixup(unsigned long addr, unsigned int fsr,
 #define VM_FAULT_BADMAP		0x010000
 #define VM_FAULT_BADACCESS	0x020000

-static inline bool is_permission_fault(unsigned int fsr)
-{
-	int fs = fsr_fs(fsr);
-#ifdef CONFIG_ARM_LPAE
-	if ((fs & FS_MMU_NOLL_MASK) == FS_PERM_NOLL)
-		return true;
-#else
-	if (fs == FS_L1_PERM || fs == FS_L2_PERM)
-		return true;
-#endif
-	return false;
-}
-
 static vm_fault_t __kprobes
 __do_page_fault(struct mm_struct *mm, unsigned long addr, unsigned int flags,
 		unsigned long vma_flags, struct pt_regs *regs)
```

Re: [PATCH 1/3] ARM: allow __do_kernel_fault() to report execution of memory faults

[Russell King (Oracle)]

On Tue, Dec 09, 2025 at 12:02:23PM +0800, Xie Yuanbin wrote:
> On Mon, 08 Dec 2025 12:34:54 +0000, Russell King wrote:
> > diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c
> > index 2bc828a1940c..f0884bf91dfa 100644
> > --- a/arch/arm/mm/fault.c
> > +++ b/arch/arm/mm/fault.c
> > @@ -162,6 +162,8 @@ __do_kernel_fault(struct mm_struct *mm, unsigned long addr, unsigned int fsr,
> >  	 */
> >  	if (addr < PAGE_SIZE) {
> >  		msg = "NULL pointer dereference";
> > +	} else if (is_permission_fault(fsr) && fsr & FSR_LNX_PF) {
> > +		msg = "execution of memory";
> >  	} else {
> >  		if (is_translation_fault(fsr) &&
> >  		    kfence_handle_page_fault(addr, is_write_fault(fsr), regs))
> 
> This patch caused a compilation error, is_permission_fault() is not
> defined. Fixed with:

Thanks - as you've guessed I have some other patches on top of this
which move the is_xxx_fault() into fault.h.

However, for the sake of this series, my preferred fix is to group
all the is_xxx_fault() inline functions together as the first patch
of this series:

8<===
From: "Russell King (Oracle)" <rmk+kernel@armlinux.org.uk>
Subject: [PATCH] ARM: group is_permission_fault() with is_translation_fault()

Group is_permission_fault() with is_translation_fault(), which is
needed to use is_permission_fault() in __do_kernel_fault(). As
this is static inline, there is no need for this to be under
CONFIG_MMU.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 arch/arm/mm/fault.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c
index 2bc828a1940c..f87f353e5a8b 100644
--- a/arch/arm/mm/fault.c
+++ b/arch/arm/mm/fault.c
@@ -128,6 +128,19 @@ static inline bool is_translation_fault(unsigned int fsr)
 	return false;
 }
 
+static inline bool is_permission_fault(unsigned int fsr)
+{
+	int fs = fsr_fs(fsr);
+#ifdef CONFIG_ARM_LPAE
+	if ((fs & FS_MMU_NOLL_MASK) == FS_PERM_NOLL)
+		return true;
+#else
+	if (fs == FS_L1_PERM || fs == FS_L2_PERM)
+		return true;
+#endif
+	return false;
+}
+
 static void die_kernel_fault(const char *msg, struct mm_struct *mm,
 			     unsigned long addr, unsigned int fsr,
 			     struct pt_regs *regs)
@@ -225,19 +238,6 @@ void do_bad_area(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
 }
 
 #ifdef CONFIG_MMU
-static inline bool is_permission_fault(unsigned int fsr)
-{
-	int fs = fsr_fs(fsr);
-#ifdef CONFIG_ARM_LPAE
-	if ((fs & FS_MMU_NOLL_MASK) == FS_PERM_NOLL)
-		return true;
-#else
-	if (fs == FS_L1_PERM || fs == FS_L2_PERM)
-		return true;
-#endif
-	return false;
-}
-
 #ifdef CONFIG_CPU_TTBR0_PAN
 static inline bool ttbr0_usermode_access_allowed(struct pt_regs *regs)
 {
-- 
2.47.3

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

[PATCH 2/3] ARM: fix hash_name() fault

[Russell King (Oracle)]

Zizhi Wo reports:

"During the execution of hash_name()->load_unaligned_zeropad(), a
 potential memory access beyond the PAGE boundary may occur. For
 example, when the filename length is near the PAGE_SIZE boundary.
 This triggers a page fault, which leads to a call to
 do_page_fault()->mmap_read_trylock(). If we can't acquire the lock,
 we have to fall back to the mmap_read_lock() path, which calls
 might_sleep(). This breaks RCU semantics because path lookup occurs
 under an RCU read-side critical section."

This is seen with CONFIG_DEBUG_ATOMIC_SLEEP=y and CONFIG_KFENCE=y.

Kernel addresses (with the exception of the vectors/kuser helper
page) do not have VMAs associated with them. If the vectors/kuser
helper page faults, then there are two possibilities:

1. if the fault happened while in kernel mode, then we're basically
   dead, because the CPU won't be able to vector through this page
   to handle the fault.
2. if the fault happened while in user mode, that means the page was
   protected from user access, and we want to fault anyway.

Thus, we can handle kernel addresses from any context entirely
separately without going anywhere near the mmap lock. This gives us
an entirely non-sleeping path for all kernel mode kernel address
faults.

As we handle the kernel address faults before interrupts are enabled,
this change has the side effect of improving the branch predictor
hardening, but does not completely solve the issue.

Reported-by: Zizhi Wo <wozizhi@huaweicloud.com>
Reported-by: Xie Yuanbin <xieyuanbin1@huawei.com>
Link: https://lore.kernel.org/r/20251126090505.3057219-1-wozizhi@huaweicloud.com
Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 arch/arm/mm/fault.c | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c
index f0884bf91dfa..92fc70b56d72 100644
--- a/arch/arm/mm/fault.c
+++ b/arch/arm/mm/fault.c
@@ -261,6 +261,35 @@ static inline bool ttbr0_usermode_access_allowed(struct pt_regs *regs)
 }
 #endif
 
+static int __kprobes
+do_kernel_address_page_fault(struct mm_struct *mm, unsigned long addr,
+			     unsigned int fsr, struct pt_regs *regs)
+{
+	if (user_mode(regs)) {
+		/*
+		 * Fault from user mode for a kernel space address. User mode
+		 * should not be faulting in kernel space, which includes the
+		 * vector/khelper page. Send a SIGSEGV.
+		 */
+		__do_user_fault(addr, fsr, SIGSEGV, SEGV_MAPERR, regs);
+	} else {
+		/*
+		 * Fault from kernel mode. Enable interrupts if they were
+		 * enabled in the parent context. Section (upper page table)
+		 * translation faults are handled via do_translation_fault(),
+		 * so we will only get here for a non-present kernel space
+		 * PTE or PTE permission fault. This may happen in exceptional
+		 * circumstances and need the fixup tables to be walked.
+		 */
+		if (interrupts_enabled(regs))
+			local_irq_enable();
+
+		__do_kernel_fault(mm, addr, fsr, regs);
+	}
+
+	return 0;
+}
+
 static int __kprobes
 do_page_fault(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
 {
@@ -274,6 +303,12 @@ do_page_fault(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
 	if (kprobe_page_fault(regs, fsr))
 		return 0;
 
+	/*
+	 * Handle kernel addresses faults separately, which avoids touching
+	 * the mmap lock from contexts that are not able to sleep.
+	 */
+	if (addr >= TASK_SIZE)
+		return do_kernel_address_page_fault(mm, addr, fsr, regs);
 
 	/* Enable interrupts if they were enabled in the parent context. */
 	if (interrupts_enabled(regs))
-- 
2.47.3

[PATCH 3/3] ARM: fix branch predictor hardening

[Russell King (Oracle)]

__do_user_fault() may be called with indeterminent interrupt enable
state, which means we may be preemptive at this point. This causes
problems when calling harden_branch_predictor(). For example, when
called from a data abort, do_alignment_fault()->do_bad_area().

Move harden_branch_predictor() out of __do_user_fault() and into the
calling contexts.

Moving it into do_kernel_address_page_fault(), we can be sure that
interrupts will be disabled here.

Converting do_translation_fault() to use do_kernel_address_page_fault()
rather than do_bad_area() means that we keep branch predictor handling
for translation faults. Interrupts will also be disabled at this call
site.

do_sect_fault() needs special handling, so detect user mode accesses
to kernel-addresses, and add an explicit call to branch predictor
hardening.

Finally, add branch predictor hardening to do_alignment() for the
faulting case (user mode accessing kernel addresses) before interrupts
are enabled.

This should cover all cases where harden_branch_predictor() is called,
ensuring that it is always has interrupts disabled, also ensuring that
it is called early in each call path.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 arch/arm/mm/alignment.c |  6 +++++-
 arch/arm/mm/fault.c     | 39 ++++++++++++++++++++++++++-------------
 2 files changed, 31 insertions(+), 14 deletions(-)

diff --git a/arch/arm/mm/alignment.c b/arch/arm/mm/alignment.c
index 3c6ddb1afdc4..812380f30ae3 100644
--- a/arch/arm/mm/alignment.c
+++ b/arch/arm/mm/alignment.c
@@ -19,10 +19,11 @@
 #include <linux/init.h>
 #include <linux/sched/signal.h>
 #include <linux/uaccess.h>
+#include <linux/unaligned.h>
 
 #include <asm/cp15.h>
 #include <asm/system_info.h>
-#include <linux/unaligned.h>
+#include <asm/system_misc.h>
 #include <asm/opcodes.h>
 
 #include "fault.h"
@@ -809,6 +810,9 @@ do_alignment(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
 	int thumb2_32b = 0;
 	int fault;
 
+	if (addr >= TASK_SIZE && user_mode(regs))
+		harden_branch_predictor();
+
 	if (interrupts_enabled(regs))
 		local_irq_enable();
 
diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c
index 92fc70b56d72..6d7a7e639321 100644
--- a/arch/arm/mm/fault.c
+++ b/arch/arm/mm/fault.c
@@ -185,9 +185,6 @@ __do_user_fault(unsigned long addr, unsigned int fsr, unsigned int sig,
 {
 	struct task_struct *tsk = current;
 
-	if (addr > TASK_SIZE)
-		harden_branch_predictor();
-
 #ifdef CONFIG_DEBUG_USER
 	if (((user_debug & UDBG_SEGV) && (sig == SIGSEGV)) ||
 	    ((user_debug & UDBG_BUS)  && (sig == SIGBUS))) {
@@ -269,8 +266,10 @@ do_kernel_address_page_fault(struct mm_struct *mm, unsigned long addr,
 		/*
 		 * Fault from user mode for a kernel space address. User mode
 		 * should not be faulting in kernel space, which includes the
-		 * vector/khelper page. Send a SIGSEGV.
+		 * vector/khelper page. Handle the branch predictor hardening
+		 * while interrupts are still disabled, then send a SIGSEGV.
 		 */
+		harden_branch_predictor();
 		__do_user_fault(addr, fsr, SIGSEGV, SEGV_MAPERR, regs);
 	} else {
 		/*
@@ -485,16 +484,20 @@ do_page_fault(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
  * We enter here because the first level page table doesn't contain
  * a valid entry for the address.
  *
- * If the address is in kernel space (>= TASK_SIZE), then we are
- * probably faulting in the vmalloc() area.
+ * If this is a user address (addr < TASK_SIZE), we handle this as a
+ * normal page fault. This leaves the remainder of the function to handle
+ * kernel address translation faults.
  *
- * If the init_task's first level page tables contains the relevant
- * entry, we copy the it to this task.  If not, we send the process
- * a signal, fixup the exception, or oops the kernel.
+ * Since user mode is not permitted to access kernel addresses, pass these
+ * directly to do_kernel_address_page_fault() to handle.
  *
- * NOTE! We MUST NOT take any locks for this case. We may be in an
- * interrupt or a critical region, and should only copy the information
- * from the master page table, nothing more.
+ * Otherwise, we're probably faulting in the vmalloc() area, so try to fix
+ * that up. Note that we must not take any locks or enable interrupts in
+ * this case.
+ *
+ * If vmalloc() fixup fails, that means the non-leaf page tables did not
+ * contain an entry for this address, so handle this via
+ * do_kernel_address_page_fault().
  */
 #ifdef CONFIG_MMU
 static int __kprobes
@@ -560,7 +563,8 @@ do_translation_fault(unsigned long addr, unsigned int fsr,
 	return 0;
 
 bad_area:
-	do_bad_area(addr, fsr, regs);
+	do_kernel_address_page_fault(current->mm, addr, fsr, regs);
+
 	return 0;
 }
 #else					/* CONFIG_MMU */
@@ -580,7 +584,16 @@ do_translation_fault(unsigned long addr, unsigned int fsr,
 static int
 do_sect_fault(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
 {
+	/*
+	 * If this is a kernel address, but from user mode, then userspace
+	 * is trying bad stuff. Invoke the branch predictor handling.
+	 * Interrupts are disabled here.
+	 */
+	if (addr >= TASK_SIZE && user_mode(regs))
+		harden_branch_predictor();
+
 	do_bad_area(addr, fsr, regs);
+
 	return 0;
 }
 #endif /* CONFIG_ARM_LPAE */
-- 
2.47.3

Re: [PATCH 0/3] ARM: fix hash_name() and branch predictor issues

[Xie Yuanbin]

On Mon, 8 Dec 2025 12:34:27 +0000, Russell King wrote:
> The first patch adds an additional check in __do_kernel_fault() to
> detect this condition. This patch is a non-obvious dependency for the
> next patch.
>
> The second patch handles faults above the top of userspace entirely
> separately, meaning we have a simpler and more obvious fault path,
> which avoids any possibility of taking any MM mutexes, which is the
> cause of the hash_name() problem.
>
> The third patch moves harden_branch_predictor() out of
> __do_user_fault() and to appropriate places in the parent functions.
> The reason this has been avoided thus far is because do_page_fault()
> can be a hot path (since it's used for page aging as well) but with
> kernel address faults being handled by an entirely separate path,
> this avoids adding to that overhead.

I don't have any objections to the first and second patches. However,
for the third patch, I feel it complicates some things. It removes
harden_branch_predictor() from __do_user_fault() and adds it to various
previous calling points.

This seems to be solely for adapting to the scenario of do_alignment() &&
user_mode(regs) && addr >= TASK_SIZE. Does this scenario really exist?
Again, in your last email, you described it as follows:
On Fri, 5 Dec 2025 12:08:14 +0000, Russell King wrote:
> Also tested usermode access to kernel space
> which fails with SEGV:
> - read from 0xc0000000 (section permission fault, do_sect_fault)
> - read from 0xffff2000 (page translation fault, do_page_fault)
> - read from 0xffff0000 (vectors page - read possible as expected)
> - write to 0xffff0000 (page permission fault, do_page_fault)

In other words, if there is a BUG_ON():
```patch
diff --git a/arch/arm/mm/alignment.c b/arch/arm/mm/alignment.c
index 3c6ddb1afdc4..d61c5adc7f96 100644
--- a/arch/arm/mm/alignment.c
+++ b/arch/arm/mm/alignment.c
@@ -833,10 +833,12 @@ do_alignment(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
 		}
 	} else {
 		fault = alignment_get_arm(regs, (void *)instrptr, &instr);
 	}
 
+	BUG_ON(user_mode(regs) && !fault && addr >= TASK_SIZE);
+
 	if (fault) {
 		type = TYPE_FAULT;
 		goto bad_or_fault;
 	}
```
Could this BUG_ON() be triggered?

If the answer is "no," then I think that something this may be enough:
```patch
diff --git a/arch/arm/mm/alignment.c b/arch/arm/mm/alignment.c
index 3c6ddb1afdc4..03aa2cff8c16 100644
--- a/arch/arm/mm/alignment.c
+++ b/arch/arm/mm/alignment.c
@@ -809,6 +809,9 @@ do_alignment(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
 	int thumb2_32b = 0;
 	int fault;

+	if (unlikely(addr >= TASK_SIZE && user_mode(regs)))
+		return do_kernel_address_page_fault(current->mm, addr, fsr, regs);
+
 	if (interrupts_enabled(regs))
 		local_irq_enable();

```
And just keep harden_branch_predictor() stay in __do_user_fault().

If I have misunderstood anything, please point it out. Thanks very much.

Re: [PATCH 0/3] ARM: fix hash_name() and branch predictor issues

[Russell King (Oracle)]

On Mon, Dec 08, 2025 at 11:08:40PM +0800, Xie Yuanbin wrote:
> On Mon, 8 Dec 2025 12:34:27 +0000, Russell King wrote:
> > The first patch adds an additional check in __do_kernel_fault() to
> > detect this condition. This patch is a non-obvious dependency for the
> > next patch.
> >
> > The second patch handles faults above the top of userspace entirely
> > separately, meaning we have a simpler and more obvious fault path,
> > which avoids any possibility of taking any MM mutexes, which is the
> > cause of the hash_name() problem.
> >
> > The third patch moves harden_branch_predictor() out of
> > __do_user_fault() and to appropriate places in the parent functions.
> > The reason this has been avoided thus far is because do_page_fault()
> > can be a hot path (since it's used for page aging as well) but with
> > kernel address faults being handled by an entirely separate path,
> > this avoids adding to that overhead.
> 
> I don't have any objections to the first and second patches. However,
> for the third patch, I feel it complicates some things. It removes
> harden_branch_predictor() from __do_user_fault() and adds it to various
> previous calling points.
> 
> This seems to be solely for adapting to the scenario of do_alignment() &&
> user_mode(regs) && addr >= TASK_SIZE. Does this scenario really exist?

As explained in the other thread, alignment faults are the first thing
that are checked when enabled, before permission fauls, so we can end
up entering do_alignment() from userspace with an address in the
kernel address space.

However, there is a user accessible page above TASK_SIZE, that is the
vectors page which can have userspace helpers in. Userspace can read
from this page, and thus can perform unaligned loads to this page.

The system behaviour of unaligned loads has been configurable, ARMv4,
for example, had the ability to check for alignment faults and raise
this exception. If that was disabled, then unaligned loads would
load the equivalent 32-bit aligned address and rotate the data in
the register. Compilers knew about this, and would code for it,
which made it ABI. Later compilers stopped using that which cut down
on the unaligned loads that userspace issued. Modern CPUs (ARMv7)
tend to behave "sanely" in that unaligned loads are generally
supported in hardware.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH 0/3] ARM: fix hash_name() and branch predictor issues

[Russell King (Oracle)]

On Mon, Dec 08, 2025 at 03:59:49PM +0000, Russell King (Oracle) wrote:
> On Mon, Dec 08, 2025 at 11:08:40PM +0800, Xie Yuanbin wrote:
> > On Mon, 8 Dec 2025 12:34:27 +0000, Russell King wrote:
> > > The first patch adds an additional check in __do_kernel_fault() to
> > > detect this condition. This patch is a non-obvious dependency for the
> > > next patch.
> > >
> > > The second patch handles faults above the top of userspace entirely
> > > separately, meaning we have a simpler and more obvious fault path,
> > > which avoids any possibility of taking any MM mutexes, which is the
> > > cause of the hash_name() problem.
> > >
> > > The third patch moves harden_branch_predictor() out of
> > > __do_user_fault() and to appropriate places in the parent functions.
> > > The reason this has been avoided thus far is because do_page_fault()
> > > can be a hot path (since it's used for page aging as well) but with
> > > kernel address faults being handled by an entirely separate path,
> > > this avoids adding to that overhead.
> > 
> > I don't have any objections to the first and second patches. However,
> > for the third patch, I feel it complicates some things. It removes
> > harden_branch_predictor() from __do_user_fault() and adds it to various
> > previous calling points.
> > 
> > This seems to be solely for adapting to the scenario of do_alignment() &&
> > user_mode(regs) && addr >= TASK_SIZE. Does this scenario really exist?
> 
> As explained in the other thread, alignment faults are the first thing
> that are checked when enabled, before permission fauls, so we can end
> up entering do_alignment() from userspace with an address in the
> kernel address space.
> 
> However, there is a user accessible page above TASK_SIZE, that is the
> vectors page which can have userspace helpers in. Userspace can read
> from this page, and thus can perform unaligned loads to this page.
> 
> The system behaviour of unaligned loads has been configurable, ARMv4,
> for example, had the ability to check for alignment faults and raise
> this exception. If that was disabled, then unaligned loads would
> load the equivalent 32-bit aligned address and rotate the data in
> the register. Compilers knew about this, and would code for it,
> which made it ABI. Later compilers stopped using that which cut down
> on the unaligned loads that userspace issued. Modern CPUs (ARMv7)
> tend to behave "sanely" in that unaligned loads are generally
> supported in hardware.

Also, would you mind giving ASAP some tested-by/reviewed-by for these
patches so they can be pushed out to linux-next for a bit of testing
there pelase? I'm on vacation from Thursday, so time is very short
to get these out - if we're not ready by Wednesday, then it'll be the
new year, possibly after the 11th January, before I can do anything
further (medical stuff.)

Thanks.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH 0/3] ARM: fix hash_name() and branch predictor issues

[Xie Yuanbin]

On Mon, 8 Dec 2025 16:47:24 +0000, Russell King wrote:
> Also, would you mind giving ASAP some tested-by/reviewed-by for these
> patches so they can be pushed out to linux-next for a bit of testing
> there pelase? I'm on vacation from Thursday, so time is very short
> to get these out - if we're not ready by Wednesday, then it'll be the
> new year, possibly after the 11th January, before I can do anything
> further (medical stuff.)
>
> Thanks.

As mentioned in the previous email, this(these patches) should be the
best solution, I think. And I have carefully read the three patches, so:

Reviewed-by: Xie Yuanbin <xieyuanbin1@huawei.com>

As for testing, I can perform some simple tests myself, and I am trying
to let the testing team to help with some further testing, like
syzkaller.

Re: [PATCH 0/3] ARM: fix hash_name() and branch predictor issues

[Xie Yuanbin]

Hi, Russell!

TL; DR:
patch1 and patch2 with compilation error fixed:

Tested-by: Xie Yuanbin <xieyuanbin1@huawei.com>

patch3:
The simple test case passed, but the syzkaller test (the original
scenario that triggered the branch predictor issue) is still running.
The syzkaller test is better to run for a day.

All patches with compilation error fixed:

Reviewed-by: Xie Yuanbin <xieyuanbin1@huawei.com>


Detailed Description:

On Tue, 9 Dec 2025 10:52:16 +0800, Xie Yuanbin wrote:
> On Mon, 8 Dec 2025 16:47:24 +0000, Russell King wrote:
>> Also, would you mind giving ASAP some tested-by/reviewed-by for these
>> patches so they can be pushed out to linux-next for a bit of testing
>> there pelase? I'm on vacation from Thursday, so time is very short
>> to get these out - if we're not ready by Wednesday, then it'll be the
>> new year, possibly after the 11th January, before I can do anything
>> further (medical stuff.)
>>
>> Thanks.
>
> As mentioned in the previous email, this(these patches) should be the
> best solution, I think. And I have carefully read the three patches, so:
>
> Reviewed-by: Xie Yuanbin <xieyuanbin1@huawei.com>
>
> As for testing, I can perform some simple tests myself, and I am trying
> to let the testing team to help with some further testing, like
> syzkaller.

Your first patch caused a compilation error: is_permission_fault() is not
defined. I fixed it with this patch:
```patch
diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c
index 2bc828a1940c..7082ba87e886 100644
--- a/arch/arm/mm/fault.c
+++ b/arch/arm/mm/fault.c
@@ -143,6 +143,21 @@ static void die_kernel_fault(const char *msg, struct mm_struct *mm,
 	make_task_dead(SIGKILL);
 }

+static inline bool is_permission_fault(unsigned int fsr)
+{
+#ifdef CONFIG_MMU
+	int fs = fsr_fs(fsr);
+#ifdef CONFIG_ARM_LPAE
+	if ((fs & FS_MMU_NOLL_MASK) == FS_PERM_NOLL)
+		return true;
+#else
+	if (fs == FS_L1_PERM || fs == FS_L2_PERM)
+		return true;
+#endif
+#endif
+	return false;
+}
+
 /*
  * Oops.  The kernel tried to access some page that wasn't present.
  */
@@ -225,19 +240,6 @@ void do_bad_area(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
 }

 #ifdef CONFIG_MMU
-static inline bool is_permission_fault(unsigned int fsr)
-{
-	int fs = fsr_fs(fsr);
-#ifdef CONFIG_ARM_LPAE
-	if ((fs & FS_MMU_NOLL_MASK) == FS_PERM_NOLL)
-		return true;
-#else
-	if (fs == FS_L1_PERM || fs == FS_L2_PERM)
-		return true;
-#endif
-	return false;
-}
-
 #ifdef CONFIG_CPU_TTBR0_PAN
 static inline bool ttbr0_usermode_access_allowed(struct pt_regs *regs)
 {
```
With the compilation error fixing patch first, and then your three
patches, and then Liyuan's patch:
Link: https://lore.kernel.org/20251127025848.363992-1-pangliyuan1@huawei.com
The current testing situation is as follows:

1. About the hash_name() fault issue:
Simple test case:
Link: https://lore.kernel.org/20251127140109.191657-1-xieyuanbin1@huawei.com
that can trigger this issue, as well as the original scenario which
trigger this issue, have passed the tests.

So patch1 and patch2:

Tested-by: Xie Yuanbin <xieyuanbin1@huawei.com>

2. About the branch predictor issue:
Simple test cases (Link same as above) that can trigger this issue, has
passed the tests. However, the original scenario which trigger the issue
is the syzkaller test, which might take some time, possibly around a day.

Re: [PATCH 0/3] ARM: fix hash_name() and branch predictor issues

[Russell King (Oracle)]

On Tue, Dec 09, 2025 at 04:56:27PM +0800, Xie Yuanbin wrote:
> Hi, Russell!
> 
> TL; DR:
> patch1 and patch2 with compilation error fixed:
> 
> Tested-by: Xie Yuanbin <xieyuanbin1@huawei.com>
> 
> patch3:
> The simple test case passed, but the syzkaller test (the original
> scenario that triggered the branch predictor issue) is still running.
> The syzkaller test is better to run for a day.
> 
> All patches with compilation error fixed:
> 
> Reviewed-by: Xie Yuanbin <xieyuanbin1@huawei.com>

To avoid any confusion, does this include my preferred fix that I sent
or your change?

Thanks.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH 0/3] ARM: fix hash_name() and branch predictor issues

[Xie Yuanbin]

On Tue, 9 Dec 2025 08:59:24 +0000, Russell King wrote:
> On Tue, Dec 09, 2025 at 04:56:27PM +0800, Xie Yuanbin wrote:
>> Hi, Russell!
>>
>> TL; DR:
>> patch1 and patch2 with compilation error fixed:
>>
>> Tested-by: Xie Yuanbin <xieyuanbin1@huawei.com>
>>
>> patch3:
>> The simple test case passed, but the syzkaller test (the original
>> scenario that triggered the branch predictor issue) is still running.
>> The syzkaller test is better to run for a day.
>>
>> All patches with compilation error fixed:
>>
>> Reviewed-by: Xie Yuanbin <xieyuanbin1@huawei.com>
>
> To avoid any confusion, does this include my preferred fix that I sent
> or your change?
>
> Thanks.

I am using my change, which is detailed in the last email, because you
send your preferred fix just 10 mins ago.

Also, I do the test with Liyuan's patch as "suffix" patch, as mentioned
in the previous email.

Re: [PATCH 0/3] ARM: fix hash_name() and branch predictor issues

[Xie Yuanbin]

Hi, Russell!

The syzkaller test has been running for 19+ hours now, and no issues
have been found so far. So to patch3:

Tested-by: Xie Yuanbin <xieyuanbin1@huawei.com>

This test will continue to run. If any issues are found, I will email
you. If I don't send any emails, then there are no issues. :)

On Tue, 9 Dec 2025 16:56:27 +0800, Xie Yuanbin wrote:
> patch1 and patch2 with compilation error fixed:
>
> Tested-by: Xie Yuanbin <xieyuanbin1@huawei.com>
>
> patch3:
> The simple test case passed, but the syzkaller test (the original
> scenario that triggered the branch predictor issue) is still running.
> The syzkaller test is better to run for a day.
>
> ...
>
> 2. About the branch predictor issue:
> Simple test cases (Link same as above) that can trigger this issue, has
> passed the tests. However, the original scenario which trigger the issue
> is the syzkaller test, which might take some time, possibly around a day.

Re: [PATCH 0/3] ARM: fix hash_name() and branch predictor issues

[Russell King (Oracle)]

On Wed, Dec 10, 2025 at 05:08:36PM +0800, Xie Yuanbin wrote:
> Hi, Russell!
> 
> The syzkaller test has been running for 19+ hours now, and no issues
> have been found so far. So to patch3:
> 
> Tested-by: Xie Yuanbin <xieyuanbin1@huawei.com>
> 
> This test will continue to run. If any issues are found, I will email
> you. If I don't send any emails, then there are no issues. :)

Thanks. I've updated the three patches, and pushed out the updated
tree. See the top four commits:

https://git.kernel.org/pub/scm/linux/kernel/git/rmk/linux.git/log/?h=fixes

Given the timings, I doubt that linux-next will pick this up before
I need to send Linus a pull request before going on vacation for the
remainder of this year:

https://git.kernel.org/pub/scm/linux/kernel/git/rmk/linux.git/log/?h=for-linus

However, I also can't guarantee that I'll be able to look at mainline
at the start of next year due to other pressures - and I don't yet
know for how long that's likely to be at the moment.

So, it's either a pull request for the for-linus tag later today,
at some point next year likely not before mid-January and splitting
it up into fixes (for -rc) and the two PREEMPT_RT non-fix patches (for
the next merge window.)

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH 0/3] ARM: fix hash_name() and branch predictor issues

[Xie Yuanbin]

On Wed, 10 Dec 2025 15:11:40 +0000, Russell King wrote:
> Thanks. I've updated the three patches, and pushed out the updated
> tree. See the top four commits:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/rmk/linux.git/log/?h=fixes
>
> Given the timings, I doubt that linux-next will pick this up before
> I need to send Linus a pull request before going on vacation for the
> remainder of this year:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/rmk/linux.git/log/?h=for-linus
>
> However, I also can't guarantee that I'll be able to look at mainline
> at the start of next year due to other pressures - and I don't yet
> know for how long that's likely to be at the moment.
>
> So, it's either a pull request for the for-linus tag later today,
> at some point next year likely not before mid-January and splitting
> it up into fixes (for -rc) and the two PREEMPT_RT non-fix patches (for
> the next merge window.)

Thanks! These patches seem to have been merged into the linux-next branch.

By the way, before the PREEMPT_RT enabled patch are finally merged,
something like this should be applied first:
Link: https://lore.kernel.org/20251127140109.191657-2-xieyuanbin1@huawei.com

I believe you won't forget this, but I currently don't see it in any patch
queue/list/system. Therefore, out of caution, I still sent this email.

Re: [PATCH 0/3] ARM: fix hash_name() and branch predictor issues

[Xie Yuanbin]

On Mon, 8 Dec 2025 15:59:49 +0000, Russell King wrote:
> However, there is a user accessible page above TASK_SIZE, that is the
> vectors page which can have userspace helpers in. Userspace can read
> from this page, and thus can perform unaligned loads to this page.

Thanks for your reply.

This is a key point, I think. This means that triggering an kernel
address's do_alignment() from user-mode is possible, and it's a normal
situation, not even a fault. And I also found that alignment_get_thumb()
and alignment_get_arm() might cause a fault, which will make things even
more complicated if we still keep harden_branch_predictor() inside
__do_user_fault().

In conclusion, this(these patches) should be the best solution; I can't
think of a better way.

> The system behaviour of unaligned loads has been configurable, ARMv4,
> for example, had the ability to check for alignment faults and raise
> this exception. If that was disabled, then unaligned loads would
> load the equivalent 32-bit aligned address and rotate the data in
> the register. Compilers knew about this, and would code for it,
> which made it ABI. Later compilers stopped using that which cut down
> on the unaligned loads that userspace issued. Modern CPUs (ARMv7)
> tend to behave "sanely" in that unaligned loads are generally
> supported in hardware.

Yes, I have a general understanding of this. Anyway, thanks for your
reply.