linux-kernel - Re: [PATCH] arm64: KVM: marking pages as XN in Stage-2 does not care about CTR

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <47f80f46b9bac66846871b2db32a3f92@kernel.org>
Date:   Mon, 12 Oct 2020 08:25:26 +0100
From:   Marc Zyngier <maz@...nel.org>
To:     l00484210 <limingwang@...wei.com>
Cc:     catalin.marinas@....com, will@...nel.org, broonie@...nel.org,
        suzuki.poulose@....com, linux-arm-kernel@...ts.infradead.org,
        linux-kernel@...r.kernel.org, fanhenglong@...wei.com,
        wanghaibin.wang@...wei.com, tangnianyao@...wei.com,
        jiangyifei@...wei.com, dengkai1@...wei.com,
        zhang.zhanghailiang@...wei.com, victor.zhangxiaofeng@...wei.com
Subject: Re: [PATCH] arm64: KVM: marking pages as XN in Stage-2 does not care
 about CTR_EL0.DIC

Hi Li,

On 2020-10-12 02:08, l00484210 wrote:
> From: MingWang Li <limingwang@...wei.com>
> 
> When testing the ARMv8.2-TTS2UXN feature, setting bits of XN is 
> unavailable.
> Because the control bit CTR_EL0.DIC is set by default on system.
> 
> But when CTR_EL0.DIC is set, software does not need to flush icache 
> actively,
> instead of clearing XN bits.The patch, the commit id of which
> is 6ae4b6e0578886eb36cedbf99f04031d93f9e315, has implemented the 
> function
> of CTR_EL0.DIC.
> 
> Signed-off-by: MingWang Li <limingwang@...wei.com>
> Signed-off-by: Henglong Fan <fanhenglong@...wei.com>
> ---
>  arch/arm64/include/asm/pgtable-prot.h | 12 +-----------
>  1 file changed, 1 insertion(+), 11 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/pgtable-prot.h
> b/arch/arm64/include/asm/pgtable-prot.h
> index 4d867c6446c4..5feb94882bf7 100644
> --- a/arch/arm64/include/asm/pgtable-prot.h
> +++ b/arch/arm64/include/asm/pgtable-prot.h
> @@ -79,17 +79,7 @@ extern bool arm64_use_ng_mappings;
>  		__val;							\
>  	 })
> 
> -#define PAGE_S2_XN							\
> -	({								\
> -		u64 __val;						\
> -		if (cpus_have_const_cap(ARM64_HAS_CACHE_DIC))		\
> -			__val = 0;					\
> -		else							\
> -			__val = PTE_S2_XN;				\
> -		__val;							\
> -	})
> -
> -#define PAGE_S2			__pgprot(_PROT_DEFAULT | PAGE_S2_MEMATTR(NORMAL) |
> PTE_S2_RDONLY | PAGE_S2_XN)
> +#define PAGE_S2			__pgprot(_PROT_DEFAULT | PAGE_S2_MEMATTR(NORMAL) |
> PTE_S2_RDONLY | PTE_S2_XN)
>  #define PAGE_S2_DEVICE		__pgprot(_PROT_DEFAULT |
> PAGE_S2_MEMATTR(DEVICE_nGnRE) | PTE_S2_RDONLY | PTE_S2_XN)
> 
>  #define PAGE_NONE		__pgprot(((_PAGE_DEFAULT) & ~PTE_VALID) |
> PTE_PROT_NONE | PTE_RDONLY | PTE_NG | PTE_PXN | PTE_UXN)

I don't understand what you are trying to achieve here.

This whole point of not setting XN in the page tables when DIC is 
present
is to avoid a pointless permission fault at run time. At you noticed
above, no icache invalidation is necessary. So why would you ever want
to take a fault on exec the first place?

         M.
-- 
Jazz is not dead. It just smells funny...