diff mbox series

[v9,30/44] arm64: kasan: Allow enabling in-kernel MTE

Message ID 5ce2fc45920e59623a4a9d8d39b6c96792f1e055.1605046192.git.andreyknvl@google.com (mailing list archive)
State New, archived
Headers show
Series [v9,01/44] kasan: drop unnecessary GPL text from comment headers | expand

Commit Message

Andrey Konovalov Nov. 10, 2020, 10:10 p.m. UTC 
From: Vincenzo Frascino <vincenzo.frascino@arm.com>

Hardware tag-based KASAN relies on Memory Tagging Extension (MTE)
feature and requires it to be enabled. MTE supports

This patch adds a new mte_init_tags() helper, that enables MTE in
Synchronous mode in EL1 and is intended to be called from KASAN runtime
during initialization.

The Tag Checking operation causes a synchronous data abort as
a consequence of a tag check fault when MTE is configured in
synchronous mode.

As part of this change enable match-all tag for EL1 to allow the
kernel to access user pages without faulting. This is required because
the kernel does not have knowledge of the tags set by the user in a
page.

Note: For MTE, the TCF bit field in SCTLR_EL1 affects only EL1 in a
similar way as TCF0 affects EL0.

MTE that is built on top of the Top Byte Ignore (TBI) feature hence we
enable it as part of this patch as well.

Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
Co-developed-by: Andrey Konovalov <andreyknvl@google.com>
Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
Change-Id: I4d67497268bb7f0c2fc5dcacefa1e273df4af71d
---
 arch/arm64/include/asm/mte-kasan.h |  6 ++++++
 arch/arm64/kernel/mte.c            |  7 +++++++
 arch/arm64/mm/proc.S               | 23 ++++++++++++++++++++---
 3 files changed, 33 insertions(+), 3 deletions(-)

Comments

Catalin Marinas Nov. 12, 2020, 9:43 a.m. UTC | #1 
On Tue, Nov 10, 2020 at 11:10:27PM +0100, Andrey Konovalov wrote:
> From: Vincenzo Frascino <vincenzo.frascino@arm.com>
> 
> Hardware tag-based KASAN relies on Memory Tagging Extension (MTE)
> feature and requires it to be enabled. MTE supports
> 
> This patch adds a new mte_init_tags() helper, that enables MTE in
> Synchronous mode in EL1 and is intended to be called from KASAN runtime
> during initialization.

There's no mte_init_tags() in this function.

> diff --git a/arch/arm64/kernel/mte.c b/arch/arm64/kernel/mte.c
> index 600b26d65b41..7f477991a6cf 100644
> --- a/arch/arm64/kernel/mte.c
> +++ b/arch/arm64/kernel/mte.c
> @@ -129,6 +129,13 @@ void *mte_set_mem_tag_range(void *addr, size_t size, u8 tag)
>  	return ptr;
>  }
>  
> +void mte_enable(void)
> +{
> +	/* Enable MTE Sync Mode for EL1. */
> +	sysreg_clear_set(sctlr_el1, SCTLR_ELx_TCF_MASK, SCTLR_ELx_TCF_SYNC);
> +	isb();
> +}

Nitpick: maybe rename this to mte_enable_kernel() since MTE is already
enabled for user apps.
Vincenzo Frascino Nov. 12, 2020, 4:46 p.m. UTC | #2 
Hi Catalin,

missed this one.

On 11/12/20 9:43 AM, Catalin Marinas wrote:
> On Tue, Nov 10, 2020 at 11:10:27PM +0100, Andrey Konovalov wrote:
>> From: Vincenzo Frascino <vincenzo.frascino@arm.com>
>>
>> Hardware tag-based KASAN relies on Memory Tagging Extension (MTE)
>> feature and requires it to be enabled. MTE supports
>>
>> This patch adds a new mte_init_tags() helper, that enables MTE in
>> Synchronous mode in EL1 and is intended to be called from KASAN runtime
>> during initialization.
> 
> There's no mte_init_tags() in this function.
> 
>> diff --git a/arch/arm64/kernel/mte.c b/arch/arm64/kernel/mte.c
>> index 600b26d65b41..7f477991a6cf 100644
>> --- a/arch/arm64/kernel/mte.c
>> +++ b/arch/arm64/kernel/mte.c
>> @@ -129,6 +129,13 @@ void *mte_set_mem_tag_range(void *addr, size_t size, u8 tag)
>>  	return ptr;
>>  }
>>  
>> +void mte_enable(void)
>> +{
>> +	/* Enable MTE Sync Mode for EL1. */
>> +	sysreg_clear_set(sctlr_el1, SCTLR_ELx_TCF_MASK, SCTLR_ELx_TCF_SYNC);
>> +	isb();
>> +}
> 
> Nitpick: maybe rename this to mte_enable_kernel() since MTE is already
> enabled for user apps.
> 

I will fix this in the next iteration.
Vincenzo Frascino Nov. 13, 2020, 11:17 a.m. UTC | #3 
Hi Catalin,

On 11/12/20 9:43 AM, Catalin Marinas wrote:
> On Tue, Nov 10, 2020 at 11:10:27PM +0100, Andrey Konovalov wrote:
>> From: Vincenzo Frascino <vincenzo.frascino@arm.com>
>>
>> Hardware tag-based KASAN relies on Memory Tagging Extension (MTE)
>> feature and requires it to be enabled. MTE supports
>>
>> This patch adds a new mte_init_tags() helper, that enables MTE in
>> Synchronous mode in EL1 and is intended to be called from KASAN runtime
>> during initialization.
> 
> There's no mte_init_tags() in this function.
> 

During the rework, I realized that the description of mte_init_tags() in this
patch refers to mte_enable_kernel(). In fact the only thing that mte_init_tags()
does is to configure the GCR_EL1 register, hence my preference would be to keep
all the code that deals with such a register in one patch.

What is your preference?
Catalin Marinas Nov. 13, 2020, noon UTC | #4 
On Fri, Nov 13, 2020 at 11:17:15AM +0000, Vincenzo Frascino wrote:
> On 11/12/20 9:43 AM, Catalin Marinas wrote:
> > On Tue, Nov 10, 2020 at 11:10:27PM +0100, Andrey Konovalov wrote:
> >> From: Vincenzo Frascino <vincenzo.frascino@arm.com>
> >>
> >> Hardware tag-based KASAN relies on Memory Tagging Extension (MTE)
> >> feature and requires it to be enabled. MTE supports
> >>
> >> This patch adds a new mte_init_tags() helper, that enables MTE in
> >> Synchronous mode in EL1 and is intended to be called from KASAN runtime
> >> during initialization.
> > 
> > There's no mte_init_tags() in this function.
> 
> During the rework, I realized that the description of mte_init_tags() in this
> patch refers to mte_enable_kernel(). In fact the only thing that mte_init_tags()
> does is to configure the GCR_EL1 register, hence my preference would be to keep
> all the code that deals with such a register in one patch.

Fine by me as long as the commit text is consistent with the diff.
Vincenzo Frascino Nov. 13, 2020, 12:04 p.m. UTC | #5 
On 11/13/20 12:00 PM, Catalin Marinas wrote:
> On Fri, Nov 13, 2020 at 11:17:15AM +0000, Vincenzo Frascino wrote:
>> On 11/12/20 9:43 AM, Catalin Marinas wrote:
>>> On Tue, Nov 10, 2020 at 11:10:27PM +0100, Andrey Konovalov wrote:
>>>> From: Vincenzo Frascino <vincenzo.frascino@arm.com>
>>>>
>>>> Hardware tag-based KASAN relies on Memory Tagging Extension (MTE)
>>>> feature and requires it to be enabled. MTE supports
>>>>
>>>> This patch adds a new mte_init_tags() helper, that enables MTE in
>>>> Synchronous mode in EL1 and is intended to be called from KASAN runtime
>>>> during initialization.
>>>
>>> There's no mte_init_tags() in this function.
>>
>> During the rework, I realized that the description of mte_init_tags() in this
>> patch refers to mte_enable_kernel(). In fact the only thing that mte_init_tags()
>> does is to configure the GCR_EL1 register, hence my preference would be to keep
>> all the code that deals with such a register in one patch.
> 
> Fine by me as long as the commit text is consistent with the diff.
> 

Done already, it will be in the next series. Thank you for the quick turnaround.
diff mbox series

Patch

diff --git a/arch/arm64/include/asm/mte-kasan.h b/arch/arm64/include/asm/mte-kasan.h
index 3a70fb1807fd..aa3ea2e0b3a8 100644
--- a/arch/arm64/include/asm/mte-kasan.h
+++ b/arch/arm64/include/asm/mte-kasan.h
@@ -29,6 +29,8 @@  u8 mte_get_mem_tag(void *addr);
 u8 mte_get_random_tag(void);
 void *mte_set_mem_tag_range(void *addr, size_t size, u8 tag);
 
+void mte_enable(void);
+
 #else /* CONFIG_ARM64_MTE */
 
 static inline u8 mte_get_ptr_tag(void *ptr)
@@ -49,6 +51,10 @@  static inline void *mte_set_mem_tag_range(void *addr, size_t size, u8 tag)
 	return addr;
 }
 
+static inline void mte_enable(void)
+{
+}
+
 #endif /* CONFIG_ARM64_MTE */
 
 #endif /* __ASSEMBLY__ */
diff --git a/arch/arm64/kernel/mte.c b/arch/arm64/kernel/mte.c
index 600b26d65b41..7f477991a6cf 100644
--- a/arch/arm64/kernel/mte.c
+++ b/arch/arm64/kernel/mte.c
@@ -129,6 +129,13 @@  void *mte_set_mem_tag_range(void *addr, size_t size, u8 tag)
 	return ptr;
 }
 
+void mte_enable(void)
+{
+	/* Enable MTE Sync Mode for EL1. */
+	sysreg_clear_set(sctlr_el1, SCTLR_ELx_TCF_MASK, SCTLR_ELx_TCF_SYNC);
+	isb();
+}
+
 static void update_sctlr_el1_tcf0(u64 tcf0)
 {
 	/* ISB required for the kernel uaccess routines */
diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S
index 23c326a06b2d..7c3304fb15d9 100644
--- a/arch/arm64/mm/proc.S
+++ b/arch/arm64/mm/proc.S
@@ -40,9 +40,15 @@ 
 #define TCR_CACHE_FLAGS	TCR_IRGN_WBWA | TCR_ORGN_WBWA
 
 #ifdef CONFIG_KASAN_SW_TAGS
-#define TCR_KASAN_FLAGS TCR_TBI1
+#define TCR_KASAN_SW_FLAGS TCR_TBI1
 #else
-#define TCR_KASAN_FLAGS 0
+#define TCR_KASAN_SW_FLAGS 0
+#endif
+
+#ifdef CONFIG_KASAN_HW_TAGS
+#define TCR_KASAN_HW_FLAGS SYS_TCR_EL1_TCMA1 | TCR_TBI1
+#else
+#define TCR_KASAN_HW_FLAGS 0
 #endif
 
 /*
@@ -427,6 +433,10 @@  SYM_FUNC_START(__cpu_setup)
 	 */
 	mov_q	x5, MAIR_EL1_SET
 #ifdef CONFIG_ARM64_MTE
+	mte_tcr	.req	x20
+
+	mov	mte_tcr, #0
+
 	/*
 	 * Update MAIR_EL1, GCR_EL1 and TFSR*_EL1 if MTE is supported
 	 * (ID_AA64PFR1_EL1[11:8] > 1).
@@ -447,6 +457,9 @@  SYM_FUNC_START(__cpu_setup)
 	/* clear any pending tag check faults in TFSR*_EL1 */
 	msr_s	SYS_TFSR_EL1, xzr
 	msr_s	SYS_TFSRE0_EL1, xzr
+
+	/* set the TCR_EL1 bits */
+	mov_q	mte_tcr, TCR_KASAN_HW_FLAGS
 1:
 #endif
 	msr	mair_el1, x5
@@ -456,7 +469,11 @@  SYM_FUNC_START(__cpu_setup)
 	 */
 	mov_q	x10, TCR_TxSZ(VA_BITS) | TCR_CACHE_FLAGS | TCR_SMP_FLAGS | \
 			TCR_TG_FLAGS | TCR_KASLR_FLAGS | TCR_ASID16 | \
-			TCR_TBI0 | TCR_A1 | TCR_KASAN_FLAGS
+			TCR_TBI0 | TCR_A1 | TCR_KASAN_SW_FLAGS
+#ifdef CONFIG_ARM64_MTE
+	orr	x10, x10, mte_tcr
+	.unreq	mte_tcr
+#endif
 	tcr_clear_errata_bits x10, x9, x5
 
 #ifdef CONFIG_ARM64_VA_BITS_52