[Uboot-stm32] [PATCH 0/7] arm: cache: cp15: don't map reserved region with no-map property

[Etienne Carriere]

On Wed, 7 Oct 2020 at 17:08, Ard Biesheuvel <ardb at kernel.org> wrote:
>
> On Wed, 7 Oct 2020 at 16:55, Etienne Carriere
> <etienne.carriere at linaro.org> wrote:
> >
> > Hello all,
> >
> > On Wed, 7 Oct 2020 at 15:16, Ard Biesheuvel <ardb at kernel.org> wrote:
> > >
> > > On Wed, 7 Oct 2020 at 13:53, Ahmad Fatoum <a.fatoum at pengutronix.de> wrote:
> > > >
> > > > Hello,
> > > >
> > > > On 10/7/20 1:23 PM, Ahmad Fatoum wrote:
> > > > > My findings[1] back then were that U-Boot did set the eXecute Never bit only on
> > > > > OMAP, but not for other platforms.  So I could imagine this being the root cause
> > > > > of Patrick's issues as well:
> > > >
> > > > Rereading my own link, my memory is a little less fuzzy: eXecute Never was being
> > > > set, but was without effect due Manager mode being set in the DACR:
> > > >
> > > > > The ARM Architecture Reference Manual notes[1]:
> > > > > > When using the Short-descriptor translation table format, the XN
> > > > > > attribute is not checked for domains marked as Manager.
> > > > > > Therefore, the system must not include read-sensitive memory in
> > > > > > domains marked as Manager, because the XN bit does not prevent
> > > > > > speculative fetches from a Manager domain.
> > > >
> > > > > To avoid speculative access to read-sensitive memory-mapped peripherals
> > > > > on ARMv7, we'll need U-Boot to use client domain permissions, so the XN
> > > > > bit can function.
> > > >
> > > > > This issue has come up before and was fixed in de63ac278
> > > > > ("ARM: mmu: Set domain permissions to client access") for OMAP2 only.
> > > > > It's equally applicable to all ARMv7-A platforms where caches are
> > > > > enabled.
> > > > > [1]: B3.7.2 - Execute-never restrictions on instruction fetching
> > > >
> > > > Hope this helps,
> > > > Ahmad
> > > >
> > >
> > > It most definitely does, thanks a lot.
> > >
> > > U-boot's mmu_setup() currently sets DACR to manager for all domains,
> > > so this is broken for all non-LPAE configurations running on v7 CPUs
> > > (except OMAP and perhaps others that fixed it individually). This
> > > affects all device mappings: not just secure DRAM for OP-TEE, but any
> > > MMIO register for any peripheral that is mapped into the CPU's address
> > > space.
> >
> > Despite the change proposed below seems to fix permissions bypass,
> > I think that not mapping the memory address ranges that are explicitly
> > expected to be not mapped (as in Patrick proposal) seems a consistent approach.
> >
>
> Agreed.
>
> But the issue Patrick is addressing uncovered a real bug that affects
> many platforms, so let's please take advantage of this, and get it
> fixed for everyone.
>
> Then, we can decide whether unmapping the secure DRAM is worth it or
> not, on its own merit, and not based on the justification that it
> fixes a bug that should be fixed in a different way in the first
> place.

Fair.

Etienne