On 23.05.23 10:19, Lorenzo Stoakes wrote:
On Tue, May 23, 2023 at 10:14:24AM +0200, David Hildenbrand wrote:
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@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?
The file is only compiled with CONFIG_HARDENED_USERCOPY:
mm/Makefile:obj-$(CONFIG_HARDENED_USERCOPY) += usercopy.o
Yeah ugh at this sort of implicit thing. Anyway it'd be preferable to stick
#ifdef CONFIG_HARDENED_USERCOPY around the prototype just so it's
abundantly clear this function doesn't exist unless that is set.
I recall that it is very common to not use ifdefs unless really
required. Because less ifefs are obviously preferable ;)
Compilation+linking will fail in any case.
--
Thanks,
David / dhildenb
