On Wed, Nov 11, 2020 at 4:13 PM Marco Elver <elver@xxxxxxxxxx> wrote: > > On Tue, Nov 10, 2020 at 11:20PM +0100, Andrey Konovalov wrote: > > The reason cache merging is disabled with KASAN is because KASAN puts its > > metadata right after the allocated object. When the merged caches have > > slightly different sizes, the metadata ends up in different places, which > > KASAN doesn't support. > > > > It might be possible to adjust the metadata allocation algorithm and make > > it friendly to the cache merging code. Instead this change takes a simpler > > approach and allows merging caches when no metadata is present. Which is > > the case for hardware tag-based KASAN with kasan.mode=prod. > > > > Signed-off-by: Andrey Konovalov <andreyknvl@xxxxxxxxxx> > > Link: https://linux-review.googlesource.com/id/Ia114847dfb2244f297d2cb82d592bf6a07455dba > > --- > > include/linux/kasan.h | 26 ++++++++++++++++++++++++-- > > mm/kasan/common.c | 11 +++++++++++ > > mm/slab_common.c | 11 ++++++++--- > > 3 files changed, 43 insertions(+), 5 deletions(-) > > > > diff --git a/include/linux/kasan.h b/include/linux/kasan.h > > index 534ab3e2935a..c754eca356f7 100644 > > --- a/include/linux/kasan.h > > +++ b/include/linux/kasan.h > > @@ -81,17 +81,35 @@ struct kasan_cache { > > }; > > > > #ifdef CONFIG_KASAN_HW_TAGS > > + > > DECLARE_STATIC_KEY_FALSE(kasan_flag_enabled); > > + > > static inline kasan_enabled(void) > > { > > return static_branch_likely(&kasan_flag_enabled); > > } > > -#else > > + > > +slab_flags_t __kasan_never_merge(slab_flags_t flags); > > +static inline slab_flags_t kasan_never_merge(slab_flags_t flags) > > +{ > > + if (kasan_enabled()) > > + return __kasan_never_merge(flags); > > + return flags; > > +} > > + > > +#else /* CONFIG_KASAN_HW_TAGS */ > > + > > static inline kasan_enabled(void) > > { > > return true; > > } > > -#endif > > + > > +static inline slab_flags_t kasan_never_merge(slab_flags_t flags) > > +{ > > + return flags; > > +} > > + > > +#endif /* CONFIG_KASAN_HW_TAGS */ > > > > void __kasan_alloc_pages(struct page *page, unsigned int order); > > static inline void kasan_alloc_pages(struct page *page, unsigned int order) > > @@ -240,6 +258,10 @@ static inline kasan_enabled(void) > > { > > return false; > > } > > +static inline slab_flags_t kasan_never_merge(slab_flags_t flags) > > +{ > > + return flags; > > +} > > static inline void kasan_alloc_pages(struct page *page, unsigned int order) {} > > static inline void kasan_free_pages(struct page *page, unsigned int order) {} > > static inline void kasan_cache_create(struct kmem_cache *cache, > > diff --git a/mm/kasan/common.c b/mm/kasan/common.c > > index 940b42231069..25b18c145b06 100644 > > --- a/mm/kasan/common.c > > +++ b/mm/kasan/common.c > > @@ -81,6 +81,17 @@ asmlinkage void kasan_unpoison_task_stack_below(const void *watermark) > > } > > #endif /* CONFIG_KASAN_STACK */ > > > > +/* > > + * Only allow cache merging when stack collection is disabled and no metadata > > + * is present. > > + */ > > +slab_flags_t __kasan_never_merge(slab_flags_t flags) > > +{ > > + if (kasan_stack_collection_enabled()) > > + return flags; > > + return flags & ~SLAB_KASAN; > > +} > > + > > void __kasan_alloc_pages(struct page *page, unsigned int order) > > { > > u8 tag; > > diff --git a/mm/slab_common.c b/mm/slab_common.c > > index f1b0c4a22f08..3042ee8ea9ce 100644 > > --- a/mm/slab_common.c > > +++ b/mm/slab_common.c > > @@ -18,6 +18,7 @@ > > #include <linux/seq_file.h> > > #include <linux/proc_fs.h> > > #include <linux/debugfs.h> > > +#include <linux/kasan.h> > > #include <asm/cacheflush.h> > > #include <asm/tlbflush.h> > > #include <asm/page.h> > > @@ -49,12 +50,16 @@ static DECLARE_WORK(slab_caches_to_rcu_destroy_work, > > slab_caches_to_rcu_destroy_workfn); > > > > /* > > - * Set of flags that will prevent slab merging > > + * Set of flags that will prevent slab merging. > > + * Use slab_never_merge() instead. > > */ > > #define SLAB_NEVER_MERGE (SLAB_RED_ZONE | SLAB_POISON | SLAB_STORE_USER | \ > > SLAB_TRACE | SLAB_TYPESAFE_BY_RCU | SLAB_NOLEAKTRACE | \ > > SLAB_FAILSLAB | SLAB_KASAN) > > Rather than changing this to require using slab_never_merge() which > removes SLAB_KASAN, could we not just have a function > kasan_never_merge() that returns KASAN-specific flags that should never > result in merging -- because as-is now, making kasan_never_merge() > remove the SLAB_KASAN flag seems the wrong way around. > > Could we not just do this: > > #define SLAB_NEVER_MERGE (SLAB_RED_ZONE | SLAB_POISON | SLAB_STORE_USER | \ > SLAB_TRACE | SLAB_TYPESAFE_BY_RCU | SLAB_NOLEAKTRACE | \ > SLAB_FAILSLAB | kasan_never_merge()) > > ?? The issue here was that SLAB_KASAN is defined in slab.h, which includes kasan.h, so we can't have a static inline definition of this function for generic and software tag-based modes. So we can do this, as long as we're fine with having kasan_never_merge() to be an actual function call for all KASAN modes. I guess it's not a problem, so let's do it this way. > > Of course that might be problematic if this always needs to be a > compile-time constant, but currently that's not a requirement. > > > +/* KASAN allows merging in some configurations and will remove SLAB_KASAN. */ > > +#define slab_never_merge() (kasan_never_merge(SLAB_NEVER_MERGE)) > > Braces unnecessary.
