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@xxxxxxx>
> >> ---
> >> 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?
