Re: [PATCH] ARM: add BUILD_BUG_ON to check if fixmap range spans multiple pmds

[Russell King (Oracle)]

On Tue, Oct 26, 2021 at 06:38:16PM +0800, Quanyang Wang wrote:
> Hi Ard,
> 
> On 10/26/21 6:12 PM, Ard Biesheuvel wrote:
> > On Tue, 26 Oct 2021 at 11:53, Quanyang Wang <quanyang.wang@xxxxxxxxxxxxx> wrote:
> > > 
> > > Hi,
> > > Sorry for the inconvenience.
> > > 
> > > On 10/26/21 4:59 PM, Russell King (Oracle) wrote:
> > > > On Sun, Oct 24, 2021 at 11:44:31PM +0200, Linus Walleij wrote:
> > > > > On Wed, Oct 20, 2021 at 7:50 AM <quanyang.wang@xxxxxxxxxxxxx> wrote:
> > > > > 
> > > > > > From: Quanyang Wang <quanyang.wang@xxxxxxxxxxxxx>
> > > > > > 
> > > > > > 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 <quanyang.wang@xxxxxxxxxxxxx>
> > > > > 
> > > > > Looks reasonable to me.
> > > > > Reviewed-by: Linus Walleij <linus.walleij@xxxxxxxxxx>
> > > > > 
> > > > > 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.

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