On Tue, 29 Nov 2022 at 12:01, Vlastimil Babka <vbabka@xxxxxxx> wrote: > > On 11/29/22 10:31, Marco Elver wrote: > > On Tue, 29 Nov 2022 at 07:37, Feng Tang <feng.tang@xxxxxxxxx> wrote: > >> diff --git a/mm/slab.h b/mm/slab.h > >> index c71590f3a22b..b6cd98b16ba7 100644 > >> --- a/mm/slab.h > >> +++ b/mm/slab.h > >> @@ -327,7 +327,8 @@ static inline slab_flags_t kmem_cache_flags(unsigned int object_size, > >> /* Legal flag mask for kmem_cache_create(), for various configurations */ > >> #define SLAB_CORE_FLAGS (SLAB_HWCACHE_ALIGN | SLAB_CACHE_DMA | \ > >> SLAB_CACHE_DMA32 | SLAB_PANIC | \ > >> - SLAB_TYPESAFE_BY_RCU | SLAB_DEBUG_OBJECTS ) > >> + SLAB_KMALLOC | SLAB_SKIP_KFENCE | \ > >> + SLAB_TYPESAFE_BY_RCU | SLAB_DEBUG_OBJECTS) > > > > Shouldn't this hunk be in the previous patch, otherwise that patch > > alone will fail? > > Good point. > > > This will also make SLAB_SKIP_KFENCE generally available to be used > > for cache creation. This is a significant change, and before it wasn't > > possible. Perhaps add a brief note to the commit message (or have a > > separate patch). We were trying to avoid making this possible, as it > > might be abused - however, given it's required for tests like these, I > > suppose there's no way around it. > > For SLAB_SKIP_KFENCE, we could also add the flag after creation to avoid > this trouble? After all there is a sysfs file to control it at runtime > anyway (via skip_kfence_store()). > In that case patch 1 would have to wrap kmem_cache_create() and the flag > addition with a new function to avoid repeating. That function could also be > adding SLAB_NO_USER_FLAGS to kmem_cache_create(), instead of the #define > DEFAULT_FLAGS. I wouldn't overcomplicate it, all we need is a way to say "this flag should not be used directly" - and only have it available via an indirect step. Availability via sysfs is one such step. And for tests, there are 2 options: 1. we could provide a function "kmem_cache_set_test_flags(cache, gfp_flags)" and define SLAB_TEST_FLAGS (which would include SLAB_SKIP_KFENCE). This still allows to set it generally, but should make abuse less likely due to the "test" in the name of that function. 2. just set it directly, s->flags |= SLAB_SKIP_KFENCE. If you're fine with #2, that seems simplest and would be my preference. > For SLAB_KMALLOC there's probably no such way unless we abuse the internal > APIs even more and call e.g. create_boot_cache() instead of > kmem_cache_create(). But that one is __init, so probably not. If we do > instead allow the flag, I wouldn't add it to SLAB_CORE_FLAGS but rather > SLAB_CACHE_FLAGS and SLAB_FLAGS_PERMITTED. I'd probably go with the simplest solution here.
