[PATCH] arm64: fix strlen() with CONFIG_KASAN_HW_TAGS

[Robin Murphy]

On 2021-07-12 15:03, Mark Rutland wrote:
> On Mon, Jul 12, 2021 at 12:19:06PM +0100, Catalin Marinas wrote:
>> On Mon, Jul 12, 2021 at 11:24:14AM +0200, Marco Elver wrote:
>>> On Mon, 12 Jul 2021 at 11:00, Mark Rutland <mark.rutland at arm.com> wrote:
>>>> diff --git a/arch/arm64/lib/strlen.S b/arch/arm64/lib/strlen.S
>>>> index 35fbdb7d6e1a..1648790e91b3 100644
>>>> --- a/arch/arm64/lib/strlen.S
>>>> +++ b/arch/arm64/lib/strlen.S
>>>> @@ -8,6 +8,7 @@
>>>>
>>>>   #include <linux/linkage.h>
>>>>   #include <asm/assembler.h>
>>>> +#include <asm/mte-def.h>
>>>>
>>>>   /* Assumptions:
>>>>    *
>>>> @@ -42,7 +43,16 @@
>>>>   #define REP8_7f 0x7f7f7f7f7f7f7f7f
>>>>   #define REP8_80 0x8080808080808080
>>>>
>>>> +/*
>>>> + * When KASAN_HW_TAGS is in use, memory is checked at MTE_GRANULE_SIZE
>>>> + * (16-byte) granularity, and we must ensure that no access straddles this
>>>> + * alignment boundary.
>>>> + */
>>>> +#ifdef CONFIG_KASAN_HW_TAGS
>>>> +#define MIN_PAGE_SIZE MTE_GRANULE_SIZE
>>>> +#else
>>>>   #define MIN_PAGE_SIZE 4096
>>>> +#endif
>>>>
>>>>          /* Since strings are short on average, we check the first 16 bytes
>>>>             of the string for a NUL character.  In order to do an unaligned ldp
>>>
>>> Glancing at the code below this hunk, I see some hard coded 16,
>>> presumably referencing the comment here. Is the algorithm tolerant to
>>> MIN_PAGE_SIZE<16? I suppose that's only an issue if the granule size
>>> might become less than 16 for some config in future.
> 
> As-is, the algorithm is not tolerant to MIN_PAGE_SIZE<16, since it only
> makes 16-byte accesses (an LDP of two 8-byte registers). Even if we
> reworked the alignment logic it to say 8 bytes, it would still straddle
> 8-byte boundaries.
> 
> It's possible to create {8,4,2,1} byte versions, but that requires more
> significant alteration to the assembly, including hte load instructions
> used, the alignment masking, and all the byte checks.
> 
>> It's probably not tolerant as we have a cmp with MIN_PAGE_SIZE - 16. But
>> I'm not worried, the architecture won't go below this without some
>> explicit opt-in. Anyway, we can add an #error MTE_GRANULE_SIZE < 16,
>> just in case anyone plays with this macro.
> 
> I assumed we'd catch this during review, since I don't see a sensible
> reason to shrink this, but I can spin a patch for that if you'd like?

FWIW even the MTE-safe userspace version is still operating on 16-byte 
chunks - AFAICS from skimming the Arm ARM it's a pretty fundamental 
assumption, so I can't see the architecture changing that in a way where 
we wouldn't have to do a bunch of explicit work and catch things like 
this in testing anyway.

Robin.