On Tue, Oct 26, 2021 at 12:56:08PM +0200, Ard Biesheuvel wrote:

On Tue, 26 Oct 2021 at 12:55, Russell King (Oracle)
[off-list ref] wrote:

quoted

On Tue, Oct 26, 2021 at 06:38:16PM +0800, Quanyang Wang wrote:

quoted

Hi Ard,

On 10/26/21 6:12 PM, Ard Biesheuvel wrote:

quoted

On Tue, 26 Oct 2021 at 11:53, Quanyang Wang [off-list ref] wrote:

quoted

Hi,
Sorry for the inconvenience.

On 10/26/21 4:59 PM, Russell King (Oracle) wrote:

quoted

On Sun, Oct 24, 2021 at 11:44:31PM +0200, Linus Walleij wrote:

quoted

On Wed, Oct 20, 2021 at 7:50 AM [off-list ref] wrote:

quoted

From: Quanyang Wang <redacted>

Not only the early fixmap range, but also the fixmap range should be
checked if it spans multiple pmds. When enabling CONFIG_DEBUG_HIGHMEM,
some systems which contain up to 16 CPUs will crash.

Signed-off-by: Quanyang Wang <redacted>

Looks reasonable to me.
Reviewed-by: Linus Walleij <redacted>

Please submit this patch into Russell's patch tracker.

... and has totally broken what looks like _all_ ARM kernel builds.

This patch is intended to trigger build error when it check the value of
__end_of_fixmap_region is equal or larger than 256.

Why? The fixmap region is larger than one PMD, so why do we need to cap it?

In __kmap_local_pfn_prot, arch_kmap_local_set_pte(&init_mm, vaddr, kmap_pte
- idx, pteval) is used to set pteval.
But the ptep is calculated by "kmap_pte - idx", which means all ptes must be
placed next to each other and no gaps. But for ARM, the ptes for the range
"0xffe00000~0xfff00000" is not next to the ptes for the range
"0xffc80000~0xffdfffff".

When the idx is larger than 256, virtual address is in 0xffdxxxxx, access
this address will crash since its pteval isn't set correctly.

Thanks for the explanation.

Sadly, this does seem to be correct. Even if the PTE tables are
located next to each other in memory, they _still_ won't be a
contiguous array of entries due to being interleaved with the Linux
PTE table and the hardware PTE table.

Since the address range 0xffe00000-0xfff00000 is already half of one
PTE table containing 512 contiguous entries, we are limited to 256
fixmap PTEs maximum. If we have more than that we will start trampling
over memory below the PTE table _and_ we will start corrupting Linux
PTE entries in the 0xfff00000-0xffffffff range.

I suspect this hasn't been seen because of a general lack of ARM
systems with more than 4 CPUs.

But doesn't that make it a kmap_local regression? Or do you think this
issue existed before that as well?

It definitely is a bug in tglx's kmap_local code, which assumes all
PTEs in the fixmap region are contiguously arranged.

Looking back further, when local kmaps were handled in arch code, this
bug did /not/ exist. We used to get the PTE entry to update via:

	unsigned long vaddr = __fix_to_virt(idx);
	pte_t *ptep = pte_offset_kernel(pmd_off_k(vaddr), vaddr);

which later became:

	pte_t *ptep = virt_to_kpte(vaddr);

Both of which walk the page tables.

So in summary a regression caused by converting ARM to kmap_local.

I think we could fix it by providing our own arch_kmap_local_set_pte()
which ignores the ptep argument, and instead walks the page tables
using the vaddr argument.

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

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