| Message ID | 706da77adfceb0c324e824d03b52d58a752577ea.1545139710.git.andreyknvl@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [mm] kasan, arm64: use ARCH_SLAB_MINALIGN instead of manual aligning | expand |
On Tue, 18 Dec 2018 14:30:33 +0100 Andrey Konovalov <andreyknvl@google.com> wrote: > Instead of changing cache->align to be aligned to KASAN_SHADOW_SCALE_SIZE > in kasan_cache_create() we can reuse the ARCH_SLAB_MINALIGN macro. > > ... > > --- a/arch/arm64/include/asm/kasan.h > +++ b/arch/arm64/include/asm/kasan.h > @@ -36,6 +36,10 @@ > #define KASAN_SHADOW_OFFSET (KASAN_SHADOW_END - (1ULL << \ > (64 - KASAN_SHADOW_SCALE_SHIFT))) > > +#ifdef CONFIG_KASAN_SW_TAGS > +#define ARCH_SLAB_MINALIGN (1ULL << KASAN_SHADOW_SCALE_SHIFT) > +#endif > + > void kasan_init(void); > void kasan_copy_shadow(pgd_t *pgdir); > asmlinkage void kasan_early_init(void); This looks unreliable. include/linux/slab.h has /* * Setting ARCH_SLAB_MINALIGN in arch headers allows a different alignment. * Intended for arches that get misalignment faults even for 64 bit integer * aligned buffers. */ #ifndef ARCH_SLAB_MINALIGN #define ARCH_SLAB_MINALIGN __alignof__(unsigned long long) #endif so if a .c file includes arch/arm64/include/asm/kasan.h after include/linux/slab.h, it can get a macro-redefined warning. If the .c file includes those headers in the other order, ARCH_SLAB_MINALIGN will get a different value compared to other .c files. Or something like that. Different architectures define ARCH_SLAB_MINALIGN in different place: ./arch/microblaze/include/asm/page.h:#define ARCH_SLAB_MINALIGN L1_CACHE_BYTES ./arch/arm/include/asm/cache.h:#define ARCH_SLAB_MINALIGN 8 ./arch/sh/include/asm/page.h:#define ARCH_SLAB_MINALIGN 8 ./arch/c6x/include/asm/cache.h:#define ARCH_SLAB_MINALIGN L1_CACHE_BYTES ./arch/sparc/include/asm/cache.h:#define ARCH_SLAB_MINALIGN __alignof__(unsigned long long) ./arch/xtensa/include/asm/processor.h:#define ARCH_SLAB_MINALIGN STACK_ALIGN which is rather bad of us. But still. I think your definition should occur in an arch header file which is reliably included from slab.h. And kasan code should get its definition of ARCH_SLAB_MINALIGN by including slab.h.
On Tue, Dec 18, 2018 at 9:55 PM Andrew Morton <akpm@linux-foundation.org> wrote: > > On Tue, 18 Dec 2018 14:30:33 +0100 Andrey Konovalov <andreyknvl@google.com> wrote: > > > Instead of changing cache->align to be aligned to KASAN_SHADOW_SCALE_SIZE > > in kasan_cache_create() we can reuse the ARCH_SLAB_MINALIGN macro. > > > > ... > > > > --- a/arch/arm64/include/asm/kasan.h > > +++ b/arch/arm64/include/asm/kasan.h > > @@ -36,6 +36,10 @@ > > #define KASAN_SHADOW_OFFSET (KASAN_SHADOW_END - (1ULL << \ > > (64 - KASAN_SHADOW_SCALE_SHIFT))) > > > > +#ifdef CONFIG_KASAN_SW_TAGS > > +#define ARCH_SLAB_MINALIGN (1ULL << KASAN_SHADOW_SCALE_SHIFT) > > +#endif > > + > > void kasan_init(void); > > void kasan_copy_shadow(pgd_t *pgdir); > > asmlinkage void kasan_early_init(void); > > This looks unreliable. include/linux/slab.h has > > /* > * Setting ARCH_SLAB_MINALIGN in arch headers allows a different alignment. > * Intended for arches that get misalignment faults even for 64 bit integer > * aligned buffers. > */ > #ifndef ARCH_SLAB_MINALIGN > #define ARCH_SLAB_MINALIGN __alignof__(unsigned long long) > #endif > > so if a .c file includes arch/arm64/include/asm/kasan.h after > include/linux/slab.h, it can get a macro-redefined warning. If the .c > file includes those headers in the other order, ARCH_SLAB_MINALIGN will > get a different value compared to other .c files. > > Or something like that. > > Different architectures define ARCH_SLAB_MINALIGN in different place: > > ./arch/microblaze/include/asm/page.h:#define ARCH_SLAB_MINALIGN L1_CACHE_BYTES > ./arch/arm/include/asm/cache.h:#define ARCH_SLAB_MINALIGN 8 > ./arch/sh/include/asm/page.h:#define ARCH_SLAB_MINALIGN 8 > ./arch/c6x/include/asm/cache.h:#define ARCH_SLAB_MINALIGN L1_CACHE_BYTES > ./arch/sparc/include/asm/cache.h:#define ARCH_SLAB_MINALIGN __alignof__(unsigned long long) > ./arch/xtensa/include/asm/processor.h:#define ARCH_SLAB_MINALIGN STACK_ALIGN > > which is rather bad of us. > > But still. I think your definition should occur in an arch header file > which is reliably included from slab.h. And kasan code should get its > definition of ARCH_SLAB_MINALIGN by including slab.h. > KASAN code doesn't use this macro directly, so I don't think it needs to get it's definition. What do you think about adding #include <linux/kasan.h> into linux/slab.h? Perhaps with a comment that this is needed to get definition of ARCH_SLAB_MINALIGN?
diff --git a/arch/arm64/include/asm/kasan.h b/arch/arm64/include/asm/kasan.h index b52aacd2c526..ba26150d578d 100644 --- a/arch/arm64/include/asm/kasan.h +++ b/arch/arm64/include/asm/kasan.h @@ -36,6 +36,10 @@ #define KASAN_SHADOW_OFFSET (KASAN_SHADOW_END - (1ULL << \ (64 - KASAN_SHADOW_SCALE_SHIFT))) +#ifdef CONFIG_KASAN_SW_TAGS +#define ARCH_SLAB_MINALIGN (1ULL << KASAN_SHADOW_SCALE_SHIFT) +#endif + void kasan_init(void); void kasan_copy_shadow(pgd_t *pgdir); asmlinkage void kasan_early_init(void); diff --git a/mm/kasan/common.c b/mm/kasan/common.c index 03d5d1374ca7..44390392d4c9 100644 --- a/mm/kasan/common.c +++ b/mm/kasan/common.c @@ -298,8 +298,6 @@ void kasan_cache_create(struct kmem_cache *cache, unsigned int *size, return; } - cache->align = round_up(cache->align, KASAN_SHADOW_SCALE_SIZE); - *flags |= SLAB_KASAN; }
Instead of changing cache->align to be aligned to KASAN_SHADOW_SCALE_SIZE in kasan_cache_create() we can reuse the ARCH_SLAB_MINALIGN macro. Suggested-by: Vincenzo Frascino <vincenzo.frascino@arm.com> Signed-off-by: Andrey Konovalov <andreyknvl@google.com> --- arch/arm64/include/asm/kasan.h | 4 ++++ mm/kasan/common.c | 2 -- 2 files changed, 4 insertions(+), 2 deletions(-)