Messages in this thread | | | Date | Tue, 23 May 2023 10:14:24 +0200 | Subject | Re: [PATCH] mm/slab: remove HAVE_HARDENED_USERCOPY_ALLOCATOR | From | David Hildenbrand <> |
| |
On 23.05.23 09:56, Lorenzo Stoakes wrote: > On Tue, May 23, 2023 at 09:46:46AM +0200, Vlastimil Babka wrote: >> On 5/23/23 09:42, Lorenzo Stoakes wrote: >>> On Tue, May 23, 2023 at 09:31:36AM +0200, Vlastimil Babka wrote: >>>> With SLOB removed, both remaining allocators support hardened usercopy, >>>> so remove the config and associated #ifdef. >>>> >>>> Signed-off-by: Vlastimil Babka <vbabka@suse.cz> >>>> --- >>>> mm/Kconfig | 2 -- >>>> mm/slab.h | 9 --------- >>>> security/Kconfig | 8 -------- >>>> 3 files changed, 19 deletions(-) >>>> >>>> diff --git a/mm/Kconfig b/mm/Kconfig >>>> index 7672a22647b4..041f0da42f2b 100644 >>>> --- a/mm/Kconfig >>>> +++ b/mm/Kconfig >>>> @@ -221,7 +221,6 @@ choice >>>> config SLAB >>>> bool "SLAB" >>>> depends on !PREEMPT_RT >>>> - select HAVE_HARDENED_USERCOPY_ALLOCATOR >>>> help >>>> The regular slab allocator that is established and known to work >>>> well in all environments. It organizes cache hot objects in >>>> @@ -229,7 +228,6 @@ config SLAB >>>> >>>> config SLUB >>>> bool "SLUB (Unqueued Allocator)" >>>> - select HAVE_HARDENED_USERCOPY_ALLOCATOR >>>> help >>>> SLUB is a slab allocator that minimizes cache line usage >>>> instead of managing queues of cached objects (SLAB approach). >>>> diff --git a/mm/slab.h b/mm/slab.h >>>> index f01ac256a8f5..695ef96b4b5b 100644 >>>> --- a/mm/slab.h >>>> +++ b/mm/slab.h >>>> @@ -832,17 +832,8 @@ struct kmem_obj_info { >>>> void __kmem_obj_info(struct kmem_obj_info *kpp, void *object, struct slab *slab); >>>> #endif >>>> >>>> -#ifdef CONFIG_HAVE_HARDENED_USERCOPY_ALLOCATOR >>>> void __check_heap_object(const void *ptr, unsigned long n, >>>> const struct slab *slab, bool to_user); >>>> -#else >>>> -static inline >>>> -void __check_heap_object(const void *ptr, unsigned long n, >>>> - const struct slab *slab, bool to_user) >>>> -{ >>>> -} >>>> -#endif >>> >>> Hm, this is still defined in slab.c/slub.c and invoked in usercopy.c, do we >>> not want the prototype? >> >> Well I didn't delete the prototype, just the ifdef/else around, so now it's >> there unconditionally. >> >>> Perhaps replacing with #ifdef >>> CONFIG_HARDENED_USERCOPY instead? I may be missing something here :) >> >> Putting it under that #ifdef would work and match that the implementations >> of that function are under that same ifdef, but maybe it's unnecessary noise >> in the header? >> > > Yeah my brain inserted extra '-'s there, sorry! > > Given we only define __check_heap_object() in sl[au]b.c if > CONFIG_HARDENED_USERCOPY wouldn't we need to keep the empty version around > if !CONFIG_HARDENED_USERCOPY since check_heap_object() appears to be called > unconditionally? >
The file is only compiled with CONFIG_HARDENED_USERCOPY:
mm/Makefile:obj-$(CONFIG_HARDENED_USERCOPY) += usercopy.o
Reviewed-by: David Hildenbrand <david@redhat.com>
-- Thanks,
David / dhildenb
| |