[26/35] kasan, arm64: Enable TBI EL1

Message ID	518da1e5169a4e343caa3c37feed5ad551b77a34.1597425745.git.andreyknvl@google.com (mailing list archive)
State	New, archived
Headers	show Return-Path: <SRS0=uP9R=BY=lists.infradead.org=linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@kernel.org> DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org BC95D20791 Date: Fri, 14 Aug 2020 19:27:08 +0200 In-Reply-To: <cover.1597425745.git.andreyknvl@google.com> Message-Id: <518da1e5169a4e343caa3c37feed5ad551b77a34.1597425745.git.andreyknvl@google.com> Mime-Version: 1.0 References: <cover.1597425745.git.andreyknvl@google.com> Subject: [PATCH 26/35] kasan, arm64: Enable TBI EL1 From: Andrey Konovalov <andreyknvl@google.com> To: Dmitry Vyukov <dvyukov@google.com>, Vincenzo Frascino <vincenzo.frascino@arm.com>, Catalin Marinas <catalin.marinas@arm.com>, kasan-dev@googlegroups.com summary: Content analysis details: (-7.7 points) pts rule name description ---- ---------------------- -------------------------------------------------- -0.0 RCVD_IN_DNSWL_NONE RBL: Sender listed at https://www.dnswl.org/, no trust [2607:f8b0:4864:20:0:0:0:84a listed in] [list.dnswl.org] -0.0 SPF_PASS SPF: sender matches SPF record -7.5 USER_IN_DEF_DKIM_WL From: address is in the default DKIM white-list 0.0 SPF_HELO_NONE SPF: HELO does not publish an SPF Record 0.1 DKIM_SIGNED Message has a DKIM or DK signature, not necessarily valid -0.1 DKIM_VALID_AU Message has a valid DKIM or DK signature from author's domain -0.1 DKIM_VALID_EF Message has a valid DKIM or DK signature from envelope-from domain -0.1 DKIM_VALID Message has at least one valid DKIM or DK signature -0.0 DKIMWL_WL_MED DKIMwl.org - Medium sender Precedence: list Cc: Marco Elver <elver@google.com>, Elena Petrova <lenaptr@google.com>, Kevin Brodsky <kevin.brodsky@arm.com>, Will Deacon <will.deacon@arm.com>, Branislav Rankov <Branislav.Rankov@arm.com>, linux-kernel@vger.kernel.org, linux-mm@kvack.org, Alexander Potapenko <glider@google.com>, linux-arm-kernel@lists.infradead.org, Andrey Ryabinin <aryabinin@virtuozzo.com>, Andrew Morton <akpm@linux-foundation.org>, Evgenii Stepanov <eugenis@google.com> Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" <linux-arm-kernel-bounces@lists.infradead.org> Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org
Series	kasan: add hardware tag-based mode for arm64 \| expand [00/35] kasan: add hardware tag-based mode for arm64 [01/35] kasan: KASAN_VMALLOC depends on KASAN_GENERIC [02/35] kasan: group vmalloc code [03/35] kasan: shadow declarations only for software modes [04/35] kasan: rename (un)poison_shadow to (un)poison_memory [05/35] kasan: rename KASAN_SHADOW_* to KASAN_GRANULE_* [06/35] kasan: only build init.c for software modes [07/35] kasan: split out shadow.c from common.c [08/35] kasan: rename generic/tags_report.c files [09/35] kasan: don't duplicate config dependencies [10/35] kasan: hide invalid free check implementation [11/35] kasan: decode stack frame only with KASAN_STACK_ENABLE [12/35] kasan, arm64: only init shadow for software modes [13/35] kasan, arm64: only use kasan_depth for software modes [14/35] kasan: rename addr_has_shadow to addr_has_metadata [15/35] kasan: rename print_shadow_for_address to print_memory_metadata [16/35] kasan: kasan_non_canonical_hook only for software modes [17/35] kasan: rename SHADOW layout macros to META [18/35] kasan: separate metadata_fetch_row for each mode [19/35] kasan: don't allow SW_TAGS with ARM64_MTE [20/35] arm64: mte: Add in-kernel MTE helpers [21/35] arm64: mte: Add in-kernel tag fault handler [22/35] arm64: mte: Enable in-kernel MTE [23/35] arm64: mte: Convert gcr_user into an exclude mask [24/35] arm64: mte: Switch GCR_EL1 in kernel entry and exit [25/35] kasan: introduce CONFIG_KASAN_HW_TAGS [26/35] kasan, arm64: Enable TBI EL1 [27/35] kasan, arm64: align allocations for HW_TAGS [28/35] kasan: define KASAN_GRANULE_SIZE for HW_TAGS [29/35] kasan, x86, s390: update undef CONFIG_KASAN [30/35] kasan, arm64: expand CONFIG_KASAN checks [31/35] kasan, arm64: implement HW_TAGS runtime [32/35] kasan, arm64: print report from tag fault handler [33/35] kasan, slub: reset tags when accessing metadata [34/35] kasan, arm64: enable CONFIG_KASAN_HW_TAGS [35/35] kasan: add documentation for hardware tag-based mode

Andrey Konovalov Aug. 14, 2020, 5:27 p.m. UTC

From: Vincenzo Frascino <vincenzo.frascino@arm.com>

Hardware tag-based KASAN relies on Memory Tagging Extension (MTE) that is
built on top of the Top Byte Ignore (TBI) feature.

Enable in-kernel TBI when CONFIG_KASAN_HW_TAGS is turned on by enabling
the TCR_TBI1 bit in proc.S.

Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
---
 arch/arm64/mm/proc.S | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Catalin Marinas Aug. 27, 2020, 10:40 a.m. UTC | #1

On Fri, Aug 14, 2020 at 07:27:08PM +0200, Andrey Konovalov wrote:
> diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S
> index 152d74f2cc9c..6880ddaa5144 100644
> --- a/arch/arm64/mm/proc.S
> +++ b/arch/arm64/mm/proc.S
> @@ -38,7 +38,7 @@
>  /* PTWs cacheable, inner/outer WBWA */
>  #define TCR_CACHE_FLAGS	TCR_IRGN_WBWA | TCR_ORGN_WBWA
>  
> -#ifdef CONFIG_KASAN_SW_TAGS
> +#if defined(CONFIG_KASAN_SW_TAGS) || defined(CONFIG_KASAN_HW_TAGS)
>  #define TCR_KASAN_FLAGS TCR_TBI1
>  #else
>  #define TCR_KASAN_FLAGS 0

I prefer to turn TBI1 on only if MTE is present. So on top of the v8
user series, just do this in __cpu_setup.

Vincenzo Frascino Aug. 27, 2020, 11:05 a.m. UTC | #2

On 8/27/20 11:40 AM, Catalin Marinas wrote:
> On Fri, Aug 14, 2020 at 07:27:08PM +0200, Andrey Konovalov wrote:
>> diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S
>> index 152d74f2cc9c..6880ddaa5144 100644
>> --- a/arch/arm64/mm/proc.S
>> +++ b/arch/arm64/mm/proc.S
>> @@ -38,7 +38,7 @@
>>  /* PTWs cacheable, inner/outer WBWA */
>>  #define TCR_CACHE_FLAGS	TCR_IRGN_WBWA | TCR_ORGN_WBWA
>>  
>> -#ifdef CONFIG_KASAN_SW_TAGS
>> +#if defined(CONFIG_KASAN_SW_TAGS) || defined(CONFIG_KASAN_HW_TAGS)
>>  #define TCR_KASAN_FLAGS TCR_TBI1
>>  #else
>>  #define TCR_KASAN_FLAGS 0
> 
> I prefer to turn TBI1 on only if MTE is present. So on top of the v8
> user series, just do this in __cpu_setup.
> 

Not sure I understand... Enabling TBI1 only if MTE is present would break
KASAN_SW_TAGS which is based on TBI1 but not on MTE.

Catalin Marinas Aug. 27, 2020, 11:13 a.m. UTC | #3

On Thu, Aug 27, 2020 at 12:05:55PM +0100, Vincenzo Frascino wrote:
> On 8/27/20 11:40 AM, Catalin Marinas wrote:
> > On Fri, Aug 14, 2020 at 07:27:08PM +0200, Andrey Konovalov wrote:
> >> diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S
> >> index 152d74f2cc9c..6880ddaa5144 100644
> >> --- a/arch/arm64/mm/proc.S
> >> +++ b/arch/arm64/mm/proc.S
> >> @@ -38,7 +38,7 @@
> >>  /* PTWs cacheable, inner/outer WBWA */
> >>  #define TCR_CACHE_FLAGS	TCR_IRGN_WBWA | TCR_ORGN_WBWA
> >>  
> >> -#ifdef CONFIG_KASAN_SW_TAGS
> >> +#if defined(CONFIG_KASAN_SW_TAGS) || defined(CONFIG_KASAN_HW_TAGS)
> >>  #define TCR_KASAN_FLAGS TCR_TBI1
> >>  #else
> >>  #define TCR_KASAN_FLAGS 0
> > 
> > I prefer to turn TBI1 on only if MTE is present. So on top of the v8
> > user series, just do this in __cpu_setup.
> 
> Not sure I understand... Enabling TBI1 only if MTE is present would break
> KASAN_SW_TAGS which is based on TBI1 but not on MTE.

You keep the KASAN_SW_TAGS as above but for HW_TAGS, only set TBI1 later
in __cpu_setup().

Vincenzo Frascino Aug. 27, 2020, 11:17 a.m. UTC | #4

On 8/27/20 12:13 PM, Catalin Marinas wrote:
> On Thu, Aug 27, 2020 at 12:05:55PM +0100, Vincenzo Frascino wrote:
>> On 8/27/20 11:40 AM, Catalin Marinas wrote:
>>> On Fri, Aug 14, 2020 at 07:27:08PM +0200, Andrey Konovalov wrote:
>>>> diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S
>>>> index 152d74f2cc9c..6880ddaa5144 100644
>>>> --- a/arch/arm64/mm/proc.S
>>>> +++ b/arch/arm64/mm/proc.S
>>>> @@ -38,7 +38,7 @@
>>>>  /* PTWs cacheable, inner/outer WBWA */
>>>>  #define TCR_CACHE_FLAGS	TCR_IRGN_WBWA | TCR_ORGN_WBWA
>>>>  
>>>> -#ifdef CONFIG_KASAN_SW_TAGS
>>>> +#if defined(CONFIG_KASAN_SW_TAGS) || defined(CONFIG_KASAN_HW_TAGS)
>>>>  #define TCR_KASAN_FLAGS TCR_TBI1
>>>>  #else
>>>>  #define TCR_KASAN_FLAGS 0
>>>
>>> I prefer to turn TBI1 on only if MTE is present. So on top of the v8
>>> user series, just do this in __cpu_setup.
>>
>> Not sure I understand... Enabling TBI1 only if MTE is present would break
>> KASAN_SW_TAGS which is based on TBI1 but not on MTE.
> 
> You keep the KASAN_SW_TAGS as above but for HW_TAGS, only set TBI1 later
> in __cpu_setup().
> 

Ok, sounds good.

Andrey Konovalov Aug. 27, 2020, 12:43 p.m. UTC | #5

On Thu, Aug 27, 2020 at 1:15 PM Vincenzo Frascino
<vincenzo.frascino@arm.com> wrote:
>
>
>
> On 8/27/20 12:13 PM, Catalin Marinas wrote:
> > On Thu, Aug 27, 2020 at 12:05:55PM +0100, Vincenzo Frascino wrote:
> >> On 8/27/20 11:40 AM, Catalin Marinas wrote:
> >>> On Fri, Aug 14, 2020 at 07:27:08PM +0200, Andrey Konovalov wrote:
> >>>> diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S
> >>>> index 152d74f2cc9c..6880ddaa5144 100644
> >>>> --- a/arch/arm64/mm/proc.S
> >>>> +++ b/arch/arm64/mm/proc.S
> >>>> @@ -38,7 +38,7 @@
> >>>>  /* PTWs cacheable, inner/outer WBWA */
> >>>>  #define TCR_CACHE_FLAGS   TCR_IRGN_WBWA | TCR_ORGN_WBWA
> >>>>
> >>>> -#ifdef CONFIG_KASAN_SW_TAGS
> >>>> +#if defined(CONFIG_KASAN_SW_TAGS) || defined(CONFIG_KASAN_HW_TAGS)
> >>>>  #define TCR_KASAN_FLAGS TCR_TBI1
> >>>>  #else
> >>>>  #define TCR_KASAN_FLAGS 0
> >>>
> >>> I prefer to turn TBI1 on only if MTE is present. So on top of the v8
> >>> user series, just do this in __cpu_setup.
> >>
> >> Not sure I understand... Enabling TBI1 only if MTE is present would break
> >> KASAN_SW_TAGS which is based on TBI1 but not on MTE.
> >
> > You keep the KASAN_SW_TAGS as above but for HW_TAGS, only set TBI1 later
> > in __cpu_setup().
> >
>
> Ok, sounds good.

Sounds good to me too.

Vincenzo, could you take care of Catalin's comments on your (arm64)
patches, do the rebase onto user mte v8, and share it with me? I'll
work on KASAN changes in the meantime, and then integrate everything
together for v2.

Perhaps the best way to test only the arm64 part is writing a simple
module that causes an MTE fault. (At least that's what I did when I
was testing core in-kernel MTE patches separately.) Or reuse this
series, all KASAN patches should rebase cleanly on top of the latest
mainline.

Vincenzo Frascino Aug. 27, 2020, 1:45 p.m. UTC | #6

Hi Andrey,

On 8/27/20 1:43 PM, Andrey Konovalov wrote:
> On Thu, Aug 27, 2020 at 1:15 PM Vincenzo Frascino
> <vincenzo.frascino@arm.com> wrote:
>>
>>
>>
>> On 8/27/20 12:13 PM, Catalin Marinas wrote:
>>> On Thu, Aug 27, 2020 at 12:05:55PM +0100, Vincenzo Frascino wrote:
>>>> On 8/27/20 11:40 AM, Catalin Marinas wrote:
>>>>> On Fri, Aug 14, 2020 at 07:27:08PM +0200, Andrey Konovalov wrote:
>>>>>> diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S
>>>>>> index 152d74f2cc9c..6880ddaa5144 100644
>>>>>> --- a/arch/arm64/mm/proc.S
>>>>>> +++ b/arch/arm64/mm/proc.S
>>>>>> @@ -38,7 +38,7 @@
>>>>>>  /* PTWs cacheable, inner/outer WBWA */
>>>>>>  #define TCR_CACHE_FLAGS   TCR_IRGN_WBWA | TCR_ORGN_WBWA
>>>>>>
>>>>>> -#ifdef CONFIG_KASAN_SW_TAGS
>>>>>> +#if defined(CONFIG_KASAN_SW_TAGS) || defined(CONFIG_KASAN_HW_TAGS)
>>>>>>  #define TCR_KASAN_FLAGS TCR_TBI1
>>>>>>  #else
>>>>>>  #define TCR_KASAN_FLAGS 0
>>>>>
>>>>> I prefer to turn TBI1 on only if MTE is present. So on top of the v8
>>>>> user series, just do this in __cpu_setup.
>>>>
>>>> Not sure I understand... Enabling TBI1 only if MTE is present would break
>>>> KASAN_SW_TAGS which is based on TBI1 but not on MTE.
>>>
>>> You keep the KASAN_SW_TAGS as above but for HW_TAGS, only set TBI1 later
>>> in __cpu_setup().
>>>
>>
>> Ok, sounds good.
> 
> Sounds good to me too.
> 
> Vincenzo, could you take care of Catalin's comments on your (arm64)
> patches, do the rebase onto user mte v8, and share it with me? I'll
> work on KASAN changes in the meantime, and then integrate everything
> together for v2.
> 

I am happy to do that. I will be on holiday though from this Saturday till the
September, 9. After that I will start the rebasing.

> Perhaps the best way to test only the arm64 part is writing a simple
> module that causes an MTE fault. (At least that's what I did when I
> was testing core in-kernel MTE patches separately.) Or reuse this
> series, all KASAN patches should rebase cleanly on top of the latest
> mainline.
> 

I can reuse the patches as they are, unless they require changes when I start
rebasing. In such a case to not duplicate the work I will scale back to use a
simple module.

Andrey Konovalov Aug. 27, 2020, 2:36 p.m. UTC | #7

On Thu, Aug 27, 2020 at 3:42 PM Vincenzo Frascino
<vincenzo.frascino@arm.com> wrote:
>
> Hi Andrey,
>
> On 8/27/20 1:43 PM, Andrey Konovalov wrote:
> > On Thu, Aug 27, 2020 at 1:15 PM Vincenzo Frascino
> > <vincenzo.frascino@arm.com> wrote:
> >>
> >>
> >>
> >> On 8/27/20 12:13 PM, Catalin Marinas wrote:
> >>> On Thu, Aug 27, 2020 at 12:05:55PM +0100, Vincenzo Frascino wrote:
> >>>> On 8/27/20 11:40 AM, Catalin Marinas wrote:
> >>>>> On Fri, Aug 14, 2020 at 07:27:08PM +0200, Andrey Konovalov wrote:
> >>>>>> diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S
> >>>>>> index 152d74f2cc9c..6880ddaa5144 100644
> >>>>>> --- a/arch/arm64/mm/proc.S
> >>>>>> +++ b/arch/arm64/mm/proc.S
> >>>>>> @@ -38,7 +38,7 @@
> >>>>>>  /* PTWs cacheable, inner/outer WBWA */
> >>>>>>  #define TCR_CACHE_FLAGS   TCR_IRGN_WBWA | TCR_ORGN_WBWA
> >>>>>>
> >>>>>> -#ifdef CONFIG_KASAN_SW_TAGS
> >>>>>> +#if defined(CONFIG_KASAN_SW_TAGS) || defined(CONFIG_KASAN_HW_TAGS)
> >>>>>>  #define TCR_KASAN_FLAGS TCR_TBI1
> >>>>>>  #else
> >>>>>>  #define TCR_KASAN_FLAGS 0
> >>>>>
> >>>>> I prefer to turn TBI1 on only if MTE is present. So on top of the v8
> >>>>> user series, just do this in __cpu_setup.
> >>>>
> >>>> Not sure I understand... Enabling TBI1 only if MTE is present would break
> >>>> KASAN_SW_TAGS which is based on TBI1 but not on MTE.
> >>>
> >>> You keep the KASAN_SW_TAGS as above but for HW_TAGS, only set TBI1 later
> >>> in __cpu_setup().
> >>>
> >>
> >> Ok, sounds good.
> >
> > Sounds good to me too.
> >
> > Vincenzo, could you take care of Catalin's comments on your (arm64)
> > patches, do the rebase onto user mte v8, and share it with me? I'll
> > work on KASAN changes in the meantime, and then integrate everything
> > together for v2.
> >
>
> I am happy to do that. I will be on holiday though from this Saturday till the
> September, 9. After that I will start the rebasing.

Ah, OK. I'll see if I can do the rebase and fix some of Catalin's
comments myself then. I'll let you know the current status once you're
back.

>
> > Perhaps the best way to test only the arm64 part is writing a simple
> > module that causes an MTE fault. (At least that's what I did when I
> > was testing core in-kernel MTE patches separately.) Or reuse this
> > series, all KASAN patches should rebase cleanly on top of the latest
> > mainline.
> >
>
> I can reuse the patches as they are, unless they require changes when I start
> rebasing. In such a case to not duplicate the work I will scale back to use a
> simple module.
>
> --
> Regards,
> Vincenzo

Andrey Konovalov Sept. 8, 2020, 1:18 p.m. UTC | #8

On Thu, Aug 27, 2020 at 12:40 PM Catalin Marinas
<catalin.marinas@arm.com> wrote:
>
> On Fri, Aug 14, 2020 at 07:27:08PM +0200, Andrey Konovalov wrote:
> > diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S
> > index 152d74f2cc9c..6880ddaa5144 100644
> > --- a/arch/arm64/mm/proc.S
> > +++ b/arch/arm64/mm/proc.S
> > @@ -38,7 +38,7 @@
> >  /* PTWs cacheable, inner/outer WBWA */
> >  #define TCR_CACHE_FLAGS      TCR_IRGN_WBWA | TCR_ORGN_WBWA
> >
> > -#ifdef CONFIG_KASAN_SW_TAGS
> > +#if defined(CONFIG_KASAN_SW_TAGS) || defined(CONFIG_KASAN_HW_TAGS)
> >  #define TCR_KASAN_FLAGS TCR_TBI1
> >  #else
> >  #define TCR_KASAN_FLAGS 0
>
> I prefer to turn TBI1 on only if MTE is present. So on top of the v8
> user series, just do this in __cpu_setup.

Started working on this, but realized that I don't understand what
exactly is suggested here. TCR_KASAN_FLAGS are used in __cpu_setup(),
so this already happens in __cpu_setup().

Do you mean that TBI1 should be enabled when CONFIG_ARM64_MTE is
enabled, but CONFIG_KASAN_HW_TAGS is disabled?

Catalin Marinas Sept. 8, 2020, 2:06 p.m. UTC | #9

On Tue, Sep 08, 2020 at 03:18:04PM +0200, Andrey Konovalov wrote:
> On Thu, Aug 27, 2020 at 12:40 PM Catalin Marinas
> <catalin.marinas@arm.com> wrote:
> >
> > On Fri, Aug 14, 2020 at 07:27:08PM +0200, Andrey Konovalov wrote:
> > > diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S
> > > index 152d74f2cc9c..6880ddaa5144 100644
> > > --- a/arch/arm64/mm/proc.S
> > > +++ b/arch/arm64/mm/proc.S
> > > @@ -38,7 +38,7 @@
> > >  /* PTWs cacheable, inner/outer WBWA */
> > >  #define TCR_CACHE_FLAGS      TCR_IRGN_WBWA | TCR_ORGN_WBWA
> > >
> > > -#ifdef CONFIG_KASAN_SW_TAGS
> > > +#if defined(CONFIG_KASAN_SW_TAGS) || defined(CONFIG_KASAN_HW_TAGS)
> > >  #define TCR_KASAN_FLAGS TCR_TBI1
> > >  #else
> > >  #define TCR_KASAN_FLAGS 0
> >
> > I prefer to turn TBI1 on only if MTE is present. So on top of the v8
> > user series, just do this in __cpu_setup.
> 
> Started working on this, but realized that I don't understand what
> exactly is suggested here. TCR_KASAN_FLAGS are used in __cpu_setup(),
> so this already happens in __cpu_setup().
> 
> Do you mean that TBI1 should be enabled when CONFIG_ARM64_MTE is
> enabled, but CONFIG_KASAN_HW_TAGS is disabled?

What I meant is that we should turn TBI1 only when the MTE is present in
hardware (and the ARM64_MTE option is on). But I probably missed the way
MTE is used with KASAN.

So what happens if CONFIG_KASAN_HW_TAGS and CONFIG_ARM64_MTE are both on
but the hardware does not support MTE? Does KASAN still generate tagged
pointers? If yes, then the current patch is fine, we should always set
TBI1.

Andrey Konovalov Sept. 8, 2020, 2:12 p.m. UTC | #10

On Tue, Sep 8, 2020 at 4:06 PM Catalin Marinas <catalin.marinas@arm.com> wrote:
>
> On Tue, Sep 08, 2020 at 03:18:04PM +0200, Andrey Konovalov wrote:
> > On Thu, Aug 27, 2020 at 12:40 PM Catalin Marinas
> > <catalin.marinas@arm.com> wrote:
> > >
> > > On Fri, Aug 14, 2020 at 07:27:08PM +0200, Andrey Konovalov wrote:
> > > > diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S
> > > > index 152d74f2cc9c..6880ddaa5144 100644
> > > > --- a/arch/arm64/mm/proc.S
> > > > +++ b/arch/arm64/mm/proc.S
> > > > @@ -38,7 +38,7 @@
> > > >  /* PTWs cacheable, inner/outer WBWA */
> > > >  #define TCR_CACHE_FLAGS      TCR_IRGN_WBWA | TCR_ORGN_WBWA
> > > >
> > > > -#ifdef CONFIG_KASAN_SW_TAGS
> > > > +#if defined(CONFIG_KASAN_SW_TAGS) || defined(CONFIG_KASAN_HW_TAGS)
> > > >  #define TCR_KASAN_FLAGS TCR_TBI1
> > > >  #else
> > > >  #define TCR_KASAN_FLAGS 0
> > >
> > > I prefer to turn TBI1 on only if MTE is present. So on top of the v8
> > > user series, just do this in __cpu_setup.
> >
> > Started working on this, but realized that I don't understand what
> > exactly is suggested here. TCR_KASAN_FLAGS are used in __cpu_setup(),
> > so this already happens in __cpu_setup().
> >
> > Do you mean that TBI1 should be enabled when CONFIG_ARM64_MTE is
> > enabled, but CONFIG_KASAN_HW_TAGS is disabled?
>
> What I meant is that we should turn TBI1 only when the MTE is present in
> hardware (and the ARM64_MTE option is on). But I probably missed the way
> MTE is used with KASAN.
>
> So what happens if CONFIG_KASAN_HW_TAGS and CONFIG_ARM64_MTE are both on
> but the hardware does not support MTE? Does KASAN still generate tagged
> pointers? If yes, then the current patch is fine, we should always set
> TBI1.

No, the tag is always 0xFF when MTE is not supported.

Should we then only enable TBI1 if system_supports_mte() or something like that?

Catalin Marinas Sept. 8, 2020, 2:41 p.m. UTC | #11

On Tue, Sep 08, 2020 at 04:12:49PM +0200, Andrey Konovalov wrote:
> On Tue, Sep 8, 2020 at 4:06 PM Catalin Marinas <catalin.marinas@arm.com> wrote:
> > On Tue, Sep 08, 2020 at 03:18:04PM +0200, Andrey Konovalov wrote:
> > > On Thu, Aug 27, 2020 at 12:40 PM Catalin Marinas
> > > <catalin.marinas@arm.com> wrote:
> > > > On Fri, Aug 14, 2020 at 07:27:08PM +0200, Andrey Konovalov wrote:
> > > > > diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S
> > > > > index 152d74f2cc9c..6880ddaa5144 100644
> > > > > --- a/arch/arm64/mm/proc.S
> > > > > +++ b/arch/arm64/mm/proc.S
> > > > > @@ -38,7 +38,7 @@
> > > > >  /* PTWs cacheable, inner/outer WBWA */
> > > > >  #define TCR_CACHE_FLAGS      TCR_IRGN_WBWA | TCR_ORGN_WBWA
> > > > >
> > > > > -#ifdef CONFIG_KASAN_SW_TAGS
> > > > > +#if defined(CONFIG_KASAN_SW_TAGS) || defined(CONFIG_KASAN_HW_TAGS)
> > > > >  #define TCR_KASAN_FLAGS TCR_TBI1
> > > > >  #else
> > > > >  #define TCR_KASAN_FLAGS 0
> > > >
> > > > I prefer to turn TBI1 on only if MTE is present. So on top of the v8
> > > > user series, just do this in __cpu_setup.
> > >
> > > Started working on this, but realized that I don't understand what
> > > exactly is suggested here. TCR_KASAN_FLAGS are used in __cpu_setup(),
> > > so this already happens in __cpu_setup().
> > >
> > > Do you mean that TBI1 should be enabled when CONFIG_ARM64_MTE is
> > > enabled, but CONFIG_KASAN_HW_TAGS is disabled?
> >
> > What I meant is that we should turn TBI1 only when the MTE is present in
> > hardware (and the ARM64_MTE option is on). But I probably missed the way
> > MTE is used with KASAN.
> >
> > So what happens if CONFIG_KASAN_HW_TAGS and CONFIG_ARM64_MTE are both on
> > but the hardware does not support MTE? Does KASAN still generate tagged
> > pointers? If yes, then the current patch is fine, we should always set
> > TBI1.
> 
> No, the tag is always 0xFF when MTE is not supported.
> 
> Should we then only enable TBI1 if system_supports_mte() or something
> like that?

You could add it do this block in __cpu_setup:

https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git/tree/arch/arm64/mm/proc.S?h=for-next/mte#n429

It needs a few changes to have "mov_q x10, TCR_..." before the MTE
check so that you can add the TBI1 bit in there.

system_supports_mte() would be called too late, you want this set before
the MMU is turned on.

[26/35] kasan, arm64: Enable TBI EL1

Commit Message

Comments

Patch