On Wed, 7 Oct 2020 at 16:55, Etienne Carriere etienne.carriere@linaro.org wrote:
Hello all,
On Wed, 7 Oct 2020 at 15:16, Ard Biesheuvel ardb@kernel.org wrote:
On Wed, 7 Oct 2020 at 13:53, Ahmad Fatoum a.fatoum@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.