On 26/02/2020 18:05, Catalin Marinas wrote:
> Once user space is given access to tagged memory, the kernel must be
> able to clear/save/restore tags visible to the user. This is done via
> the linear mapping, therefore map it as such. The new MT_NORMAL_TAGGED
> index for MAIR_EL1 is initially mapped as Normal memory and later
> changed to Normal Tagged via the cpufeature infrastructure. From a
> mismatched attribute aliases perspective, the Tagged memory is
> considered a permission and it won't lead to undefined behaviour.
>
> The empty_zero_page is cleared to ensure that the tags it contains are
> already zeroed. The actual tags-aware clear_page() implementation is
> part of a subsequent patch.
>
> Signed-off-by: Catalin Marinas <catalin.marinas@xxxxxxx>
> ---
> arch/arm64/include/asm/memory.h | 1 +
> arch/arm64/include/asm/pgtable-prot.h | 2 ++
> arch/arm64/kernel/cpufeature.c | 30 +++++++++++++++++++++++++++
> arch/arm64/mm/dump.c | 4 ++++
> arch/arm64/mm/mmu.c | 22 ++++++++++++++++++--
> arch/arm64/mm/proc.S | 8 +++++--
> 6 files changed, 63 insertions(+), 4 deletions(-)
>
[...]
> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index 128f70852bf3..a2c206444e47 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -120,7 +120,7 @@ static bool pgattr_change_is_safe(u64 old, u64 new)
> * The following mapping attributes may be updated in live
> * kernel mappings without the need for break-before-make.
> */
> - static const pteval_t mask = PTE_PXN | PTE_RDONLY | PTE_WRITE | PTE_NG;
> + pteval_t mask = PTE_PXN | PTE_RDONLY | PTE_WRITE | PTE_NG;
>
> /* creating or taking down mappings is always safe */
> if (old == 0 || new == 0)
> @@ -134,6 +134,19 @@ static bool pgattr_change_is_safe(u64 old, u64 new)
> if (old & ~new & PTE_NG)
> return false;
>
> + if (system_supports_mte()) {
> + /*
> + * Changing the memory type between Normal and Normal-Tagged
> + * is safe since Tagged is considered a permission attribute
> + * from the mismatched attribute aliases perspective.
> + */
> + if ((old & PTE_ATTRINDX_MASK) == PTE_ATTRINDX(MT_NORMAL) ||
> + (old & PTE_ATTRINDX_MASK) == PTE_ATTRINDX(MT_NORMAL_TAGGED) ||
> + (new & PTE_ATTRINDX_MASK) == PTE_ATTRINDX(MT_NORMAL) ||
> + (new & PTE_ATTRINDX_MASK) == PTE_ATTRINDX(MT_NORMAL_TAGGED))
> + mask |= PTE_ATTRINDX_MASK;
> + }
> +
> return ((old ^ new) & ~mask) == 0;
> }
This is much more permissive than I would expect. If either the old or
new memory type is NORMAL (or NORMAL_TAGGED) then the memory type is
ignored altogether.
Should this check be:
if (((old & PTE_ATTRINDX_MASK) == PTE_ATTRINDX(MT_NORMAL) ||
(old & PTE_ATTRINDX_MASK) == PTE_ATTRINDX(MT_NORMAL_TAGGED)) &&
((new & PTE_ATTRINDX_MASK) == PTE_ATTRINDX(MT_NORMAL) ||
(new & PTE_ATTRINDX_MASK) == PTE_ATTRINDX(MT_NORMAL_TAGGED)))
Steve
