[PATCH] arm64: fix strlen() with CONFIG_KASAN_HW_TAGS

[Mark Rutland]

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?

Thanks,
Mark.