linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH mm] kasan, arm64: use ARCH_SLAB_MINALIGN instead of manual aligning
@ 2018-12-18 13:30 Andrey Konovalov
  2018-12-18 20:54 ` Andrew Morton
  0 siblings, 1 reply; 3+ messages in thread
From: Andrey Konovalov @ 2018-12-18 13:30 UTC (permalink / raw)
  To: Andrey Ryabinin, Alexander Potapenko, Dmitry Vyukov,
	Catalin Marinas, Will Deacon, Christoph Lameter, Andrew Morton,
	Mark Rutland, Nick Desaulniers, Marc Zyngier, Dave Martin,
	Ard Biesheuvel, Eric W . Biederman, Ingo Molnar, Paul Lawrence,
	Geert Uytterhoeven, Arnd Bergmann, Kirill A . Shutemov,
	Greg Kroah-Hartman, Kate Stewart, Mike Rapoport,
	Vincenzo Frascino, kasan-dev, linux-doc, linux-kernel,
	linux-arm-kernel, linux-sparse, linux-mm, linux-kbuild
  Cc: Vishwath Mohan, Chintan Pandya, Jacob Bramley, Jann Horn,
	Ruben Ayrapetyan, Andrey Konovalov, Lee Smith, Kostya Serebryany,
	Mark Brand, Ramana Radhakrishnan, Evgeniy Stepanov

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(-)

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;
 }
 
-- 
2.20.0.405.gbc1bbc6f85-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH mm] kasan, arm64: use ARCH_SLAB_MINALIGN instead of manual aligning
  2018-12-18 13:30 [PATCH mm] kasan, arm64: use ARCH_SLAB_MINALIGN instead of manual aligning Andrey Konovalov
@ 2018-12-18 20:54 ` Andrew Morton
  2018-12-20 13:02   ` Andrey Konovalov
  0 siblings, 1 reply; 3+ messages in thread
From: Andrew Morton @ 2018-12-18 20:54 UTC (permalink / raw)
  To: Andrey Konovalov
  Cc: Mark Rutland, Kate Stewart, linux-doc, Catalin Marinas,
	Will Deacon, Paul Lawrence, linux-mm, Alexander Potapenko,
	Chintan Pandya, Christoph Lameter, Vincenzo Frascino,
	Ingo Molnar, Jacob Bramley, Ruben Ayrapetyan, Mark Brand,
	kasan-dev, linux-sparse, Geert Uytterhoeven, linux-arm-kernel,
	Andrey Ryabinin, Dave Martin, Evgeniy Stepanov, Vishwath Mohan,
	Arnd Bergmann, linux-kbuild, Marc Zyngier, Ramana Radhakrishnan,
	Mike Rapoport, Dmitry Vyukov, Kostya Serebryany, Jann Horn,
	Ard Biesheuvel, Greg Kroah-Hartman, Nick Desaulniers,
	linux-kernel, Eric W . Biederman, Lee Smith, Kirill A . Shutemov

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.


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH mm] kasan, arm64: use ARCH_SLAB_MINALIGN instead of manual aligning
  2018-12-18 20:54 ` Andrew Morton
@ 2018-12-20 13:02   ` Andrey Konovalov
  0 siblings, 0 replies; 3+ messages in thread
From: Andrey Konovalov @ 2018-12-20 13:02 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Mark Rutland, Kate Stewart, open list:DOCUMENTATION,
	Catalin Marinas, Will Deacon, Paul Lawrence,
	Linux Memory Management List, Alexander Potapenko,
	Chintan Pandya, Christoph Lameter, Vincenzo Frascino,
	Ingo Molnar, Jacob Bramley, Ruben Ayrapetyan, Mark Brand,
	kasan-dev, linux-sparse, Geert Uytterhoeven, Linux ARM,
	Andrey Ryabinin, Dave Martin, Evgeniy Stepanov, Vishwath Mohan,
	Arnd Bergmann, Linux Kbuild mailing list, Marc Zyngier,
	Ramana Radhakrishnan, Mike Rapoport, Dmitry Vyukov,
	Kostya Serebryany, Jann Horn, Ard Biesheuvel, Greg Kroah-Hartman,
	Nick Desaulniers, LKML, Eric W . Biederman, Lee Smith,
	Kirill A . Shutemov

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?

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2018-12-20 13:03 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-18 13:30 [PATCH mm] kasan, arm64: use ARCH_SLAB_MINALIGN instead of manual aligning Andrey Konovalov
2018-12-18 20:54 ` Andrew Morton
2018-12-20 13:02   ` Andrey Konovalov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).