slab: Introduce kmalloc_obj() and family
Subsystems (2)
PATCH 1/1 slab: Introduce kmalloc_obj() and family
Introduce type-aware kmalloc-family helpers to replace the common idioms for single, array, and flexible object allocations:
ptr = kmalloc(sizeof(*ptr), gfp); ptr = kcalloc(count, sizeof(*ptr), gfp); ptr = kmalloc_array(count, sizeof(*ptr), gfp); ptr = kcalloc(count, sizeof(*ptr), gfp); ptr = kmalloc(struct_size(ptr, flex_member, count), gfp);
These become, respectively:
kmalloc_obj(p, gfp); kzalloc_obj(p, count, gfp); kmalloc_obj(p, count, gfp); kzalloc_obj(p, count, gfp); kmalloc_obj(p, flex_member, count, gfp);
These each return the size of the allocation, so that other common idioms can be converted easily as well. For example:
info->size = struct_size(ptr, flex_member, count); ptr = kmalloc(info->size, gfp);
becomes:
info->size = kmalloc_obj(ptr, flex_member, count, gfp);
Internal introspection of allocated type also becomes possible, allowing for alignment-aware choices and future hardening work. For example, adding __alignof(*ptr) as an argument to the internal allocators so that appropriate/efficient alignment choices can be made, or being able to correctly choose per-allocation offset randomization within a bucket that does not break alignment requirements.
Additionally, once __builtin_get_counted_by() is added by GCC[1] and Clang[2], it will be possible to automatically set the counted member of a struct with a counted_by FAM, further eliminating open-coded redundant initializations, and can internally check for "too large" allocations based on the type size of the counter variable:
if (count > type_max(ptr->flex_count)) fail...; info->size = struct_size(ptr, flex_member, count); ptr = kmalloc(info->size, gfp); ptr->flex_count = count;
becomes (i.e. unchanged from earlier example):
info->size = kmalloc_obj(ptr, flex_member, count, gfp);
Replacing all existing simple code patterns found via Coccinelle[3] shows what could be replaced immediately (saving roughly 1,500 lines):
7040 files changed, 14128 insertions(+), 15557 deletions(-)
---
Cc: Vlastimil Babka <[email protected]>
Cc: Christoph Lameter <[email protected]>
Cc: Pekka Enberg <[email protected]>
Cc: David Rientjes <[email protected]>
Cc: Joonsoo Kim <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: Roman Gushchin <[email protected]>
Cc: Hyeonggon Yoo <[email protected]>
Cc: Gustavo A. R. Silva <[email protected]>
Cc: Bill Wendling <[email protected]>
Cc: Justin Stitt <[email protected]>
Cc: Jann Horn <[email protected]>
Cc: Przemek Kitszel <[email protected]>
Cc: Marco Elver <[email protected]>
Cc: [email protected]
--- include/linux/slab.h | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+)
include/linux/slab.h +38 -0
@@ -686,6 +686,44 @@ static __always_inline __alloc_size(1) void *kmalloc_noprof(size_t size, gfp_t f }#define kmalloc (...) alloc_hooks(kmalloc_noprof(__VA_ARGS__)) +#define __alloc_obj3 (ALLOC ,P ,COUNT ,FLAGS ) \ +({ \ + size_t __obj_size = size_mul(sizeof(*P), COUNT); \ + void *__obj_ptr; \ + (P) = __obj_ptr = ALLOC(__obj_size, FLAGS); \ + if (!__obj_ptr) \ + __obj_size = 0; \ + __obj_size; \ +}) + +#define __alloc_obj2 (ALLOC ,P ,FLAGS ) __alloc_obj3(ALLOC, P, 1, FLAGS) + +#define __alloc_obj4 (ALLOC ,P ,FAM ,COUNT ,FLAGS ) \ +({ \ + size_t __obj_size = struct_size(P, FAM, COUNT); \ + void *__obj_ptr; \ + (P) = __obj_ptr = ALLOC(__obj_size, FLAGS); \ + if (!__obj_ptr) \ + __obj_size = 0; \ + __obj_size; \ +}) + +#define kmalloc_obj (...) \ + CONCATENATE(__alloc_obj, \ + COUNT_ARGS(__VA_ARGS__))(kmalloc, __VA_ARGS__) + +#define kzalloc_obj (...) \ + CONCATENATE(__alloc_obj, \ + COUNT_ARGS(__VA_ARGS__))(kzalloc, __VA_ARGS__) + +#define kvmalloc_obj (...) \ + CONCATENATE(__alloc_obj, \ + COUNT_ARGS(__VA_ARGS__))(kvmalloc, __VA_ARGS__) + +#define kvzalloc_obj (...) \ + CONCATENATE(__alloc_obj, \ + COUNT_ARGS(__VA_ARGS__))(kvzalloc, __VA_ARGS__) +#define kmem_buckets_alloc (_b, _size, _flags) \ alloc_hooks(__kmalloc_node_noprof(PASS_BUCKET_PARAMS(_size, _b), _flags, NUMA_NO_NODE))
Signature · 1 line
2.34.1
Quoted 9 lines · Kees Cook
Introduce type-aware kmalloc-family helpers to replace the common idioms for single, array, and flexible object allocations:
ptr = kmalloc(sizeof(*ptr), gfp); ptr = kcalloc(count, sizeof(*ptr), gfp); ptr = kmalloc_array(count, sizeof(*ptr), gfp); ptr = kcalloc(count, sizeof(*ptr), gfp); ptr = kmalloc(struct_size(ptr, flex_member, count), gfp);
Quoted 7 lines
These become, respectively:
kmalloc_obj(p, gfp); kzalloc_obj(p, count, gfp); kmalloc_obj(p, count, gfp); kzalloc_obj(p, count, gfp); kmalloc_obj(p, flex_member, count, gfp);
So I'm not a huge fan in hiding the assignment, but I understand there's value in having guaranteed the target of the assignment is really the same thing as the one used for sizeof() etc.
But returning size seems awkward, it would be IMHO less confusing if it still returned the object pointer, that could be then also assigned elsewhere if needed, tested for NULL and ZERO_SIZE_PTR (now it's both 0?).
I'm also not sure that having it all called kmalloc_obj() with 3 variants of how many parameters it takes is such a win? e.g. kmalloc_obj(), kcalloc_obj() and kcalloc_obj_flex() would be more obvious?
Quoted 9 lines
These each return the size of the allocation, so that other common idioms can be converted easily as well. For example:
info->size = struct_size(ptr, flex_member, count); ptr = kmalloc(info->size, gfp);becomes:
info->size = kmalloc_obj(ptr, flex_member, count, gfp);
How about instead taking an &info->size parameter that assigns size to it, so the ptr can be still returned but we also can record the size?
Also the last time David asked for documentation, you say you would try, but there's nothing new here? Dunno if the kerneldocs are feasible but there's at least Documentation/core-api/memory-allocation.rst ...
Thanks,
Vlastimil
Quoted 101 lines
Internal introspection of allocated type also becomes possible, allowing for alignment-aware choices and future hardening work. For example, adding
__alignof(*ptr)as an argument to the internal allocators so that appropriate/efficient alignment choices can be made, or being able to correctly choose per-allocation offset randomization within a bucket that does not break alignment requirements.Additionally, once
__builtin_get_counted_by()is added by GCC[1] and Clang[2], it will be possible to automatically set the counted member of a struct with acounted_byFAM, further eliminating open-coded redundant initializations, and can internally check for "too large" allocations based on the type size of the counter variable:if (count > type_max(ptr->flex_count)) fail...; info->size = struct_size(ptr, flex_member, count); ptr = kmalloc(info->size, gfp); ptr->flex_count = count;becomes (i.e. unchanged from earlier example):
info->size = kmalloc_obj(ptr, flex_member, count, gfp);Replacing all existing simple code patterns found via Coccinelle[3] shows what could be replaced immediately (saving roughly 1,500 lines):
7040 files changed, 14128 insertions(+), 15557 deletions(-)Link https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/kmalloc_obj-assign-size.cocci [3]Signed-off-by Kees Cook <[email protected]>---
Cc: Vlastimil Babka <[email protected]>
Cc: Christoph Lameter <[email protected]>
Cc: Pekka Enberg <[email protected]>
Cc: David Rientjes <[email protected]>
Cc: Joonsoo Kim <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: Roman Gushchin <[email protected]>
Cc: Hyeonggon Yoo <[email protected]>
Cc: Gustavo A. R. Silva <[email protected]>
Cc: Bill Wendling <[email protected]>
Cc: Justin Stitt <[email protected]>
Cc: Jann Horn <[email protected]>
Cc: Przemek Kitszel <[email protected]>
Cc: Marco Elver <[email protected]>
Cc: [email protected]--- include/linux/slab.h | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+)include/linux/slab.h +38
-0@@ -686,6 +686,44 @@ static __always_inline __alloc_size(1) void *kmalloc_noprof(size_t size, gfp_t f }#define kmalloc (...) alloc_hooks(kmalloc_noprof(__VA_ARGS__)) +#define __alloc_obj3 (ALLOC ,P ,COUNT ,FLAGS ) \ +({ \ + size_t __obj_size = size_mul(sizeof(*P), COUNT); \ + void *__obj_ptr; \ + (P) = __obj_ptr = ALLOC(__obj_size, FLAGS); \ + if (!__obj_ptr) \ + __obj_size = 0; \ + __obj_size; \ +}) + +#define __alloc_obj2 (ALLOC ,P ,FLAGS ) __alloc_obj3(ALLOC, P, 1, FLAGS) + +#define __alloc_obj4 (ALLOC ,P ,FAM ,COUNT ,FLAGS ) \ +({ \ + size_t __obj_size = struct_size(P, FAM, COUNT); \ + void *__obj_ptr; \ + (P) = __obj_ptr = ALLOC(__obj_size, FLAGS); \ + if (!__obj_ptr) \ + __obj_size = 0; \ + __obj_size; \ +}) + +#define kmalloc_obj (...) \ + CONCATENATE(__alloc_obj, \ + COUNT_ARGS(__VA_ARGS__))(kmalloc, __VA_ARGS__) + +#define kzalloc_obj (...) \ + CONCATENATE(__alloc_obj, \ + COUNT_ARGS(__VA_ARGS__))(kzalloc, __VA_ARGS__) + +#define kvmalloc_obj (...) \ + CONCATENATE(__alloc_obj, \ + COUNT_ARGS(__VA_ARGS__))(kvmalloc, __VA_ARGS__) + +#define kvzalloc_obj (...) \ + CONCATENATE(__alloc_obj, \ + COUNT_ARGS(__VA_ARGS__))(kvzalloc, __VA_ARGS__) +#define kmem_buckets_alloc (_b, _size, _flags) \ alloc_hooks(__kmalloc_node_noprof(PASS_BUCKET_PARAMS(_size, _b), _flags, NUMA_NO_NODE))
Quoted 18 lines · Vlastimil Babka
Quoted 9 lines · Kees Cook
Introduce type-aware kmalloc-family helpers to replace the common idioms for single, array, and flexible object allocations:
ptr = kmalloc(sizeof(*ptr), gfp); ptr = kcalloc(count, sizeof(*ptr), gfp); ptr = kmalloc_array(count, sizeof(*ptr), gfp); ptr = kcalloc(count, sizeof(*ptr), gfp); ptr = kmalloc(struct_size(ptr, flex_member, count), gfp);Quoted 7 lines
These become, respectively:
kmalloc_obj(p, gfp); kzalloc_obj(p, count, gfp); kmalloc_obj(p, count, gfp); kzalloc_obj(p, count, gfp); kmalloc_obj(p, flex_member, count, gfp);
Quoted 7 lines
So I'm not a huge fan in hiding the assignment, but I understand there's value in having guaranteed the target of the assignment is really the same thing as the one used for
sizeof()etc.But returning size seems awkward, it would be IMHO less confusing if it still returned the object pointer, that could be then also assigned elsewhere if needed, tested for NULL and
ZERO_SIZE_PTR(now it's both 0?).
Ah, I made this changed based on requests in earlier threads. But yes, the ambiguity with ZERO_SIZE_PTR does make me uncomfortable too. I think I can make the size an optional argument when splitting the functions as you request below...
Quoted 3 lines
I'm also not sure that having it all called
kmalloc_obj()with 3 variants of how many parameters it takes is such a win? e.g.kmalloc_obj(),kcalloc_obj()andkcalloc_obj_flex()would be more obvious?
I liked it because it's always doing the same thing: allocating a structure. But yes, I do see that it's a bit weird. Since "kcalloc" means "zero it also", how about the naming just uses pluralization instead?
kmalloc_obj(p, gfp); kmalloc_objs(p, count, gfp); kmalloc_flex(p, flex_member, count, gfp);
Does that looks okay?
Quoted 6 lines
Quoted 6 lines
These each return the size of the allocation, so that other common idioms can be converted easily as well. For example:
info->size = struct_size(ptr, flex_member, count); ptr = kmalloc(info->size, gfp);
Quoted 6 lines
Quoted 3 lines
becomes:
info->size = kmalloc_obj(ptr, flex_member, count, gfp);How about instead taking an
&info->sizeparameter that assigns size to it, so the ptr can be still returned but we also can record the size?
If we wanted size output, we could add an optional final argument:
kmalloc_obj(p, gfp, &size); kmalloc_objs(p, count, gfp, &size); kmalloc_flex(p, flex_member, count, gfp, &size);
And as far as solving the concern of "hiding the assignment", what about trying to "show" that "p" is being assigned by requiring that it also use "&"? For example:
kmalloc_obj(&p, gfp); kmalloc_objs(&p, count, gfp); kmalloc_flex(&p, flex_member, count, gfp);
Quoted 3 lines
Also the last time David asked for documentation, you say you would try, but there's nothing new here? Dunno if the kerneldocs are feasible but there's at least
Documentation/core-api/memory-allocation.rst...
Whoops, yes. I totally missed adding those. I will add that once I have some direction on the above design ideas.
Thanks for looking at this!
-Kees
Signature · 1 line
Kees Cook