[PATCH 0/3] ARM: fix hash_name() and branch predictor issues
Xie Yuanbin
xieyuanbin1 at huawei.com
Mon Dec 8 07:08:40 PST 2025
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.
More information about the linux-arm-kernel
mailing list