[PATCH v4 0/2] slab: Introduce kmalloc_obj() and family

[PATCH v4 0/2] slab: Introduce kmalloc_obj() and family

[Kees Cook]

Hi,

Here's a refresh and update on the kmalloc_obj() API proposal. Please
see patch 2 for the specific details. And note that this is obviously
not v6.15 material! :)

Thanks!

-Kees

 v4:
 - split __flex_counter() out and add appropriate helpers
 - add flex array examples to commit log
 - add "size" details to commit log
 - add treewide conversion details to commit log
 - improve treewide Coccinelle scripting
 - fix documentation typos
 v3: https://lore.kernel.org/lkml/20240822231324.make.666-kees@kernel.org/
 v2: https://lore.kernel.org/lkml/20240807235433.work.317-kees@kernel.org/
 v1: https://lore.kernel.org/lkml/20240719192744.work.264-kees@kernel.org/

Kees Cook (2):
  compiler_types: Introduce __flex_counter() and family
  slab: Introduce kmalloc_obj() and family

 Documentation/process/deprecated.rst |  42 +++++++
 include/linux/compiler_types.h       |  31 +++++
 include/linux/overflow.h             |  36 ++++++
 include/linux/slab.h                 | 170 +++++++++++++++++++++++++++
 4 files changed, 279 insertions(+)

-- 
2.34.1

[PATCH v4 1/2] compiler_types: Introduce __flex_counter() and family

[Kees Cook]

Introduce __flex_counter() which wraps __builtin_counted_by_ref(),
as newly introduced by GCC[1] and Clang[2]. Use of __flex_counter()
allows access to the counter member of a struct's flexible array member
when it has been annotated with __counted_by().

Introduce typeof_flex_counter(), can_set_flex_counter(), and
set_flex_counter() to provide the needed _Generic() wrappers to get sane
results out of __flex_counter().

For example, with:

	struct foo {
		int counter;
		short array[] __counted_by(counter);
	} *p;

__flex_counter(p->array) will resolve to: &p->counter

typeof_flex_counter(p->array) will resolve to "int". (If p->array was not
annotated, it would resolve to "size_t".)

can_set_flex_counter(p->array, COUNT) is the same as:

	COUNT <= type_max(p->counter) && COUNT >= type_min(p->counter)

(If p->array was not annotated it would return true since everything
fits in size_t.)

set_flex_counter(p->array, COUNT) is the same as:

	p->counter = COUNT;

(It is a no-op if p->array is not annotated with __counted_by().)

Signed-off-by: Kees Cook <kees@kernel.org>
---
Cc: Miguel Ojeda <ojeda@kernel.org>
Cc: "Gustavo A. R. Silva" <gustavoars@kernel.org>
Cc: Nathan Chancellor <nathan@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Nick Desaulniers <nick.desaulniers+lkml@gmail.com>
Cc: Marco Elver <elver@google.com>
Cc: Przemek Kitszel <przemyslaw.kitszel@intel.com>
Cc: linux-hardening@vger.kernel.org
---
 include/linux/compiler_types.h | 31 +++++++++++++++++++++++++++++
 include/linux/overflow.h       | 36 ++++++++++++++++++++++++++++++++++
 2 files changed, 67 insertions(+)

diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
index 981cc3d7e3aa..8b45ecfad5b1 100644
--- a/include/linux/compiler_types.h
+++ b/include/linux/compiler_types.h
@@ -453,6 +453,37 @@ struct ftrace_likely_data {
 #define __annotated(var, attr)	(false)
 #endif
 
+/*
+ * Optional: only supported since gcc >= 15, clang >= 19
+ *
+ *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html#index-_005f_005fbuiltin_005fcounted_005fby_005fref
+ * clang: https://github.com/llvm/llvm-project/pull/102549
+ */
+#if __has_builtin(__builtin_counted_by_ref)
+/**
+ * __flex_counter() - Get pointer to counter member for the given
+ *                    flexible array, if it was annotated with __counted_by()
+ * @FAM: Pointer to flexible array member of an addressable struct instance
+ *
+ * For example, with:
+ *
+ *	struct foo {
+ *		int counter;
+ *		short array[] __counted_by(counter);
+ *	} *p;
+ *
+ * __flex_counter(p->array) will resolve to &p->counter.
+ *
+ * Note that Clang may not allow this to be assigned to a separate
+ * variable; it must be used directly.
+ *
+ * If p->array is unannotated, this returns (void *)NULL.
+ */
+#define __flex_counter(FAM)	__builtin_counted_by_ref(FAM)
+#else
+#define __flex_counter(FAM)	((void *)NULL)
+#endif
+
 /*
  * Some versions of gcc do not mark 'asm goto' volatile:
  *
diff --git a/include/linux/overflow.h b/include/linux/overflow.h
index 0c7e3dcfe867..e2b81cb5576e 100644
--- a/include/linux/overflow.h
+++ b/include/linux/overflow.h
@@ -440,4 +440,40 @@ static inline size_t __must_check size_sub(size_t minuend, size_t subtrahend)
 #define DEFINE_FLEX(TYPE, NAME, MEMBER, COUNTER, COUNT)	\
 	_DEFINE_FLEX(TYPE, NAME, MEMBER, COUNT, = { .obj.COUNTER = COUNT, })
 
+/**
+ * typeof_flex_counter() - Return the type of the counter variable of a given
+ *                         flexible array member annotated by __counted_by().
+ * @FAM: Pointer to the flexible array member within a given struct.
+ *
+ * Returns "size_t" if no annotation exists.
+ */
+#define typeof_flex_counter(FAM)				\
+	typeof(_Generic(__flex_counter(FAM),			\
+			void *: (size_t)0,			\
+			default: *__flex_counter(FAM)))
+
+/** can_set_flex_counter() - Check if the counter associated with the given
+ *                           flexible array member can represent a value.
+ * @FAM: Pointer to the flexible array member within a given struct.
+ * @COUNT: Value to check against the __counted_by annotated @FAM's counter.
+ */
+#define can_set_flex_counter(FAM, COUNT)			\
+	(!overflows_type(COUNT, typeof_flex_counter(FAM)))
+
+/**
+ * set_flex_counter() - Set the counter associated with the given flexible
+ *                      array member that has been annoated by __counted_by().
+ * @FAM: Pointer to the flexible array member within a given struct.
+ * @COUNT: Value to store to the __counted_by annotated @FAM's counter.
+ *
+ * This is a no-op if no annotation exists. Count needs to be checked with
+ * can_set_flex_counter(FAM, COUNT) before using this function.
+ */
+#define set_flex_counter(FAM, COUNT)				\
+({								\
+	*_Generic(__flex_counter(FAM),				\
+		  void *:  &(size_t){ 0 },			\
+		  default: __flex_counter(FAM)) = (COUNT);	\
+})
+
 #endif /* __LINUX_OVERFLOW_H */
-- 
2.34.1

[PATCH v4 2/2] slab: Introduce kmalloc_obj() and family

[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 = kmalloc(sizeof(struct some_obj_name), gfp);
	ptr = kzalloc(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(ptr, gfp);
	kmalloc_obj(ptr, gfp);
	kzalloc_obj(ptr, gfp);
	kmalloc_objs(ptr, count, gfp);
	kzalloc_objs(ptr, count, gfp);
	kmalloc_flex(ptr, flex_member, count, gfp);

Beyond the other benefits outlined below, the primary ergonomic benefit
is the elimination of type redundancy (and the elimination of potential
type mismatches), as the existing kmalloc assignment code pattern must
always repeat the variable or the variable type on the right hand side.

These each return the assigned value of ptr (which may be NULL on
failure). For cases where the total size of the allocation is needed,
the kmalloc_obj_sz(), kmalloc_objs_sz(), and kmalloc_flex_sz() family
of macros can be used. For example:

	info->size = struct_size(ptr, flex_member, count);
	ptr = kmalloc(info->size, gfp);

becomes:

	kmalloc_flex_sz(ptr, flex_member, count, gfp, &info->size);

With the *_sz() helpers, it becomes possible to do bounds checking of
the final size to make sure no arithmetic overflow has happened that
exceeds the storage size of the target size variable. e.g. it was possible
before to end up wrapping an allocation size and not noticing, there by
allocating too small a size. (Most of Linux's exposure on that particular
problem is via newly written code as we already did bulk conversions[1],
but we continue to have a steady stream of patches catching additional
cases[2] that would just go away with this API.)

Internal introspection of the allocated type now becomes possible,
allowing for future alignment-aware choices to be made by the allocator
and future hardening work that can be type sensitive. 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.

For the flexible array helpers, the internal use of __flex_counter()
allows for automatically setting the counter member of a struct's flexible
array member when it has been annotated with __counted_by(), avoiding
any missed early size initializations while __counted_by() annotations
are added to the kernel. Additionally, this also checks for "too large"
allocations based on the type size of the counter variable. For example:

	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 (n.b. unchanged from earlier example):

	kmalloc_flex_sz(ptr, flex_member, count, gfp, &info->size);
	ptr->flex_count = count;

Note that manual initialization of the flexible array counter is still
required (at some point) after allocation as not all compiler versions
support the __counted_by annotation yet. But doing it internally makes
sure they cannot be missed when __counted_by _is_ available, meaning
that the bounds checker will not trip due to the lack of "early enough"
initializations that used to work before enabling the stricter bounds
checking. For example:

	kmalloc_flex(ptr, flex_member, count);
	fill(ptr->flex, count);
	ptr->flex_count = count;

This works correctly before adding a __counted_by annotation (since
nothing is checking ptr->flex accesses against ptr->flex_count). After
adding the annotation, the bounds sanitizer would trip during fill()
because ptr->flex_count wasn't set yet. But with kmalloc_flex() setting
ptr->flex_count internally at allocation time, the existing code works
without needing to move the ptr->flex_count assignment before the call
to fill(). (This has been a stumbling block for __counted_by adoption.)

Replacing all existing simple code patterns found via Coccinelle[3]
shows what could be replaced immediately (also saving roughly 2200 lines):

 7568 files changed, 16342 insertions(+), 18580 deletions(-)

This would take us from 23927 k*alloc assignments to 8378:

$ git grep ' = kv\?[mzcv]alloc\(\|_array\)(' | wc -l
23927
$ git reset --hard HEAD^
HEAD is now at 8bccc91e6cdf treewide: kmalloc_obj conversion
$ git grep ' = kv\?[mzcv]alloc\(\|_array\)(' | wc -l
8378

This treewide change could be done at the end of the merge window just
before -rc1 is released (as is common for treewide changes). Handling
this API change in backports to -stable should be possible without much
hassle by backporting the __flex_counter() patch and this patch, while
taking conversions as-needed.

The impact on my bootable testing image size (with the treewide patch
applied) is tiny. With both GCC 13 (no __counted_by support) and GCC 15
(with __counted_by) the images are actually very slightly smaller:

$ size -G gcc-boot/vmlinux.gcc*
      text       data        bss      total filename
  29975593   21527689   16601200   68104482 gcc-boot/vmlinux.gcc13-before
  29969263   21528663   16601112   68099038 gcc-boot/vmlinux.gcc13-after
  30555626   21291299   17086620   68933545 gcc-boot/vmlinux.gcc15-before
  30550144   21292039   17086540   68928723 gcc-boot/vmlinux.gcc15-after

Link: https://web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=b08fc5277aaa1d8ea15470d38bf36f19dfb0e125 [1]
Link: https://lore.kernel.org/all/?q=s%3Akcalloc+-s%3ARe%3A [2]
Link: https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/kmalloc_objs.cocci [3]
Signed-off-by: Kees Cook <kees@kernel.org>
---
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Christoph Lameter <cl@linux.com>
Cc: Pekka Enberg <penberg@kernel.org>
Cc: David Rientjes <rientjes@google.com>
Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Roman Gushchin <roman.gushchin@linux.dev>
Cc: Hyeonggon Yoo <42.hyeyoo@gmail.com>
Cc: Gustavo A. R. Silva <gustavoars@kernel.org>
Cc: Bill Wendling <morbo@google.com>
Cc: Justin Stitt <justinstitt@google.com>
Cc: Jann Horn <jannh@google.com>
Cc: Przemek Kitszel <przemyslaw.kitszel@intel.com>
Cc: Marco Elver <elver@google.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Sasha Levin <sashal@kernel.org>
Cc: linux-mm@kvack.org
---
 Documentation/process/deprecated.rst |  42 +++++++
 include/linux/slab.h                 | 170 +++++++++++++++++++++++++++
 2 files changed, 212 insertions(+)

diff --git a/Documentation/process/deprecated.rst b/Documentation/process/deprecated.rst
index 1f7f3e6c9cda..c5a4b245c895 100644
--- a/Documentation/process/deprecated.rst
+++ b/Documentation/process/deprecated.rst
@@ -372,3 +372,45 @@ The helper must be used::
 			DECLARE_FLEX_ARRAY(struct type2, two);
 		};
 	};
+
+Open-coded kmalloc assignments for struct objects
+-------------------------------------------------
+Performing open-coded kmalloc()-family allocation assignments prevents
+the kernel (and compiler) from being able to examine the type of the
+variable being assigned, which limits any related introspection that
+may help with alignment, wrap-around, or additional hardening. The
+kmalloc_obj()-family of macros provide this introspection, which can be
+used for the common code patterns for single, array, and flexible object
+allocations. For example, these open coded assignments::
+
+	ptr = kmalloc(sizeof(*ptr), gfp);
+	ptr = kmalloc(sizeof(struct the_type_of_ptr_obj), gfp);
+	ptr = kzalloc(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);
+
+become, respectively::
+
+	kmalloc_obj(ptr, gfp);
+	kzalloc_obj(ptr, gfp);
+	kmalloc_objs(ptr, count, gfp);
+	kzalloc_objs(ptr, count, gfp);
+	kmalloc_flex(ptr, flex_member, count, gfp);
+
+For the cases where the total size of the allocation is also needed,
+the kmalloc_obj_sz(), kmalloc_objs_sz(), and kmalloc_flex_sz() family of
+macros can be used. For example, converting these assignments::
+
+	total_size = struct_size(ptr, flex_member, count);
+	ptr = kmalloc(total_size, gfp);
+
+becomes::
+
+	kmalloc_flex_sz(ptr, flex_member, count, gfp, &total_size);
+
+If `ptr->flex_member` is annotated with __counted_by(), the allocation
+will automatically fail if `count` is larger than the maximum
+representable value that can be stored in the counter member associated
+with `flex_member`. Similarly, the allocation will fail if the total
+size of the allocation exceeds the maximum value `*total_size` can hold.
diff --git a/include/linux/slab.h b/include/linux/slab.h
index 09eedaecf120..c51edc046835 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -12,6 +12,7 @@
 #ifndef _LINUX_SLAB_H
 #define	_LINUX_SLAB_H
 
+#include <linux/bug.h>
 #include <linux/cache.h>
 #include <linux/gfp.h>
 #include <linux/overflow.h>
@@ -906,6 +907,175 @@ 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_objs(ALLOC, P, COUNT, FLAGS, SIZE)		\
+({								\
+	size_t __obj_size = size_mul(sizeof(*P), COUNT);	\
+	const typeof(_Generic(SIZE,				\
+			void *: (size_t *)NULL,			\
+			default: SIZE)) __size_ptr = (SIZE);	\
+	typeof(P) __obj_ptr = NULL;				\
+	/* Does the total size fit in the *SIZE variable? */	\
+	if (!WARN_ON_ONCE(__size_ptr && __obj_size > type_max(*__size_ptr))) \
+		__obj_ptr = ALLOC(__obj_size, FLAGS);		\
+	if (!__obj_ptr)						\
+		__obj_size = 0;					\
+	if (__size_ptr)						\
+		*__size_ptr = __obj_size;			\
+	(P) = __obj_ptr;					\
+})
+
+#define __alloc_flex(ALLOC, P, FAM, COUNT, FLAGS, SIZE)			\
+({									\
+	const size_t __count = (COUNT);					\
+	size_t __obj_size = struct_size(P, FAM, __count);		\
+	/* "*SIZE = ...;" below is unbuildable when SIZE is "NULL" */	\
+	const typeof(_Generic(SIZE,					\
+			void *: (size_t *)NULL,				\
+			default: SIZE)) __size_ptr = (SIZE);		\
+	typeof(P) __obj_ptr = NULL;					\
+	if (!WARN_ON_ONCE(!can_set_flex_counter(__obj_ptr->FAM, __count)) && \
+	    !WARN_ON_ONCE(__size_ptr && __obj_size > type_max(*__size_ptr))) \
+		__obj_ptr = ALLOC(__obj_size, FLAGS);			\
+	if (__obj_ptr) {						\
+		set_flex_counter(__obj_ptr->FAM, __count);		\
+	} else {							\
+		__obj_size = 0;						\
+	}								\
+	if (__size_ptr)							\
+		*__size_ptr = __obj_size;				\
+	(P) = __obj_ptr;						\
+})
+
+/**
+ * kmalloc_obj - Allocate a single instance of the given structure
+ * @P: Pointer to hold allocation of the structure
+ * @FLAGS: GFP flags for the allocation
+ *
+ * Returns the newly allocated value of @P on success, NULL on failure.
+ * @P is assigned the result, either way.
+ */
+#define kmalloc_obj(P, FLAGS)				\
+	__alloc_objs(kmalloc, P, 1, FLAGS, NULL)
+
+/**
+ * kmalloc_obj_sz - Allocate a single instance of the given structure and
+ *		 store total size
+ * @P: Pointer to hold allocation of the structure
+ * @FLAGS: GFP flags for the allocation
+ * @SIZE: Pointer to variable to hold the total allocation size
+ *
+ * Returns the newly allocated value of @P on success, NULL on failure.
+ * @P is assigned the result, either way. If @SIZE is non-NULL, the
+ * allocation will immediately fail if the total allocation size is larger
+ * than what the type of *@SIZE can represent.
+ */
+#define kmalloc_obj_sz(P, FLAGS, SIZE)			\
+	__alloc_objs(kmalloc, P, 1, FLAGS, SIZE)
+
+/**
+ * kmalloc_objs - Allocate an array of the given structure
+ * @P: Pointer to hold allocation of the structure array
+ * @COUNT: How many elements in the array
+ * @FLAGS: GFP flags for the allocation
+ *
+ * Returns the newly allocated value of @P on success, NULL on failure.
+ * @P is assigned the result, either way.
+ */
+#define kmalloc_objs(P, COUNT, FLAGS)			\
+	__alloc_objs(kmalloc, P, COUNT, FLAGS, NULL)
+
+/**
+ * kmalloc_objs_sz - Allocate an array of the given structure and store
+ *		     total size
+ * @P: Pointer to hold allocation of the structure array
+ * @COUNT: How many elements in the array
+ * @FLAGS: GFP flags for the allocation
+ * @SIZE: Pointer to variable to hold the total allocation size
+ *
+ * Returns the newly allocated value of @P on success, NULL on failure.
+ * @P is assigned the result, either way. If @SIZE is non-NULL, the
+ * allocation will immediately fail if the total allocation size is larger
+ * than what the type of *@SIZE can represent.
+ */
+#define kmalloc_objs_sz(P, COUNT, FLAGS, SIZE)		\
+	__alloc_objs(kmalloc, P, COUNT, FLAGS, SIZE)
+
+/**
+ * kmalloc_flex - Allocate a single instance of the given flexible structure
+ * @P: Pointer to hold allocation of the structure
+ * @FAM: The name of the flexible array member of the structure
+ * @COUNT: How many flexible array member elements are desired
+ * @FLAGS: GFP flags for the allocation
+ *
+ * Returns the newly allocated value of @P on success, NULL on failure.
+ * @P is assigned the result, either way. If @FAM has been annotated with
+ * __counted_by(), the allocation will immediately fail if @COUNT is larger
+ * than what the type of the struct's counter variable can represent.
+ */
+#define kmalloc_flex(P, FAM, COUNT, FLAGS)		\
+	__alloc_flex(kmalloc, P, FAM, COUNT, FLAGS, NULL)
+
+/**
+ * kmalloc_flex_sz - Allocate a single instance of the given flexible
+ *		     structure and store total size
+ * @P: Pointer to hold allocation of the structure
+ * @FAM: The name of the flexible array member of the structure
+ * @COUNT: How many flexible array member elements are desired
+ * @FLAGS: GFP flags for the allocation
+ * @SIZE: Pointer to variable to hold the total allocation size
+ *
+ * Returns the newly allocated value of @P on success, NULL on failure.
+ * @P is assigned the result, either way. If @FAM has been annotated with
+ * __counted_by(), the allocation will immediately fail if @COUNT is larger
+ * than what the type of the struct's counter variable can represent. If
+ * @SIZE is non-NULL, the allocation will immediately fail if the total
+ * allocation size is larger than what the type of *@SIZE can represent.
+ */
+#define kmalloc_flex_sz(P, FAM, COUNT, FLAGS, SIZE)	\
+	__alloc_flex(kmalloc, P, FAM, COUNT, FLAGS, SIZE)
+
+/* All kzalloc aliases for kmalloc_(obj|objs|fam)(|_sz). */
+#define kzalloc_obj(P, FLAGS)				\
+	__alloc_objs(kzalloc, P, 1, FLAGS, NULL)
+#define kzalloc_obj_sz(P, FLAGS, SIZE)			\
+	__alloc_objs(kzalloc, P, 1, FLAGS, SIZE)
+#define kzalloc_objs(P, COUNT, FLAGS)			\
+	__alloc_objs(kzalloc, P, COUNT, FLAGS, NULL)
+#define kzalloc_objs_sz(P, COUNT, FLAGS, SIZE)		\
+	__alloc_objs(kzalloc, P, COUNT, FLAGS, SIZE)
+#define kzalloc_flex(P, FAM, COUNT, FLAGS)		\
+	__alloc_flex(kzalloc, P, FAM, COUNT, FLAGS, NULL)
+#define kzalloc_flex_sz(P, FAM, COUNT, FLAGS, SIZE)	\
+	__alloc_flex(kzalloc, P, FAM, COUNT, FLAGS, SIZE)
+
+/* All kvmalloc aliases for kmalloc_(obj|objs|fam)(|_sz). */
+#define kvmalloc_obj(P, FLAGS)				\
+	__alloc_objs(kvmalloc, P, 1, FLAGS, NULL)
+#define kvmalloc_obj_sz(P, FLAGS, SIZE)			\
+	__alloc_objs(kvmalloc, P, 1, FLAGS, SIZE)
+#define kvmalloc_objs(P, COUNT, FLAGS)			\
+	__alloc_objs(kvmalloc, P, COUNT, FLAGS, NULL)
+#define kvmalloc_objs_sz(P, COUNT, FLAGS, SIZE)		\
+	__alloc_objs(kvmalloc, P, COUNT, FLAGS, SIZE)
+#define kvmalloc_flex(P, FAM, COUNT, FLAGS)		\
+	__alloc_flex(kvmalloc, P, FAM, COUNT, FLAGS, NULL)
+#define kvmalloc_flex_sz(P, FAM, COUNT, FLAGS, SIZE)	\
+	__alloc_flex(kvmalloc, P, FAM, COUNT, FLAGS, SIZE)
+
+/* All kvzalloc aliases for kmalloc_(obj|objs|fam)(|_sz). */
+#define kvzalloc_obj(P, FLAGS)				\
+	__alloc_objs(kvzalloc, P, 1, FLAGS, NULL)
+#define kvzalloc_obj_sz(P, FLAGS, SIZE)			\
+	__alloc_objs(kvzalloc, P, 1, FLAGS, SIZE)
+#define kvzalloc_objs(P, COUNT, FLAGS)			\
+	__alloc_objs(kvzalloc, P, COUNT, FLAGS, NULL)
+#define kvzalloc_objs_sz(P, COUNT, FLAGS, SIZE)		\
+	__alloc_objs(kvzalloc, P, COUNT, FLAGS, SIZE)
+#define kvzalloc_flex(P, FAM, COUNT, FLAGS)		\
+	__alloc_flex(kvzalloc, P, FAM, COUNT, FLAGS, NULL)
+#define kvzalloc_flex_sz(P, FAM, COUNT, FLAGS, SIZE)	\
+	__alloc_flex(kvzalloc, P, FAM, COUNT, FLAGS, SIZE)
+
 #define kmem_buckets_alloc(_b, _size, _flags)	\
 	alloc_hooks(__kmalloc_node_noprof(PASS_BUCKET_PARAMS(_size, _b), _flags, NUMA_NO_NODE))
 
-- 
2.34.1

Re: [PATCH v4 1/2] compiler_types: Introduce __flex_counter() and family

[Randy Dunlap]

Hi Kees,


On 3/14/25 8:15 PM, Kees Cook wrote:


> diff --git a/include/linux/overflow.h b/include/linux/overflow.h
> index 0c7e3dcfe867..e2b81cb5576e 100644
> --- a/include/linux/overflow.h
> +++ b/include/linux/overflow.h
> @@ -440,4 +440,40 @@ static inline size_t __must_check size_sub(size_t minuend, size_t subtrahend)
>  #define DEFINE_FLEX(TYPE, NAME, MEMBER, COUNTER, COUNT)	\
>  	_DEFINE_FLEX(TYPE, NAME, MEMBER, COUNT, = { .obj.COUNTER = COUNT, })
>  
> +/**
> + * typeof_flex_counter() - Return the type of the counter variable of a given
> + *                         flexible array member annotated by __counted_by().
> + * @FAM: Pointer to the flexible array member within a given struct.
> + *
> + * Returns "size_t" if no annotation exists.

Please use
 * Returns: <text>
instead so that kernel-doc can make a special doc section for it.

Same for patch 2/2.

> + */
> +#define typeof_flex_counter(FAM)				\
> +	typeof(_Generic(__flex_counter(FAM),			\
> +			void *: (size_t)0,			\
> +			default: *__flex_counter(FAM)))
> +
> +/** can_set_flex_counter() - Check if the counter associated with the given

Needs a newline between /** and the function name, as in set_flex_counter() below.

> + *                           flexible array member can represent a value.
> + * @FAM: Pointer to the flexible array member within a given struct.
> + * @COUNT: Value to check against the __counted_by annotated @FAM's counter.
> + */
> +#define can_set_flex_counter(FAM, COUNT)			\
> +	(!overflows_type(COUNT, typeof_flex_counter(FAM)))
> +
> +/**
> + * set_flex_counter() - Set the counter associated with the given flexible
> + *                      array member that has been annoated by __counted_by().
> + * @FAM: Pointer to the flexible array member within a given struct.
> + * @COUNT: Value to store to the __counted_by annotated @FAM's counter.
> + *
> + * This is a no-op if no annotation exists. Count needs to be checked with
> + * can_set_flex_counter(FAM, COUNT) before using this function.
> + */
> +#define set_flex_counter(FAM, COUNT)				\
> +({								\
> +	*_Generic(__flex_counter(FAM),				\
> +		  void *:  &(size_t){ 0 },			\
> +		  default: __flex_counter(FAM)) = (COUNT);	\
> +})
> +
>  #endif /* __LINUX_OVERFLOW_H */

-- 
~Randy

Re: [PATCH v4 2/2] slab: Introduce kmalloc_obj() and family

[Gustavo A. R. Silva]

> These each return the assigned value of ptr (which may be NULL on
> failure). For cases where the total size of the allocation is needed,
> the kmalloc_obj_sz(), kmalloc_objs_sz(), and kmalloc_flex_sz() family
> of macros can be used. For example:
> 
> 	info->size = struct_size(ptr, flex_member, count);
> 	ptr = kmalloc(info->size, gfp);
> 
> becomes:
> 
> 	kmalloc_flex_sz(ptr, flex_member, count, gfp, &info->size);

I wonder if it'd be better to keep the gfp flags as the last argument
for all these `*_sz()` cases:

	kmalloc_flex_sz(ptr, flex_member, count, &info->size, gpf);

Probably, even for __alloc_objs()

--
Gustavo

Re: [PATCH v4 2/2] slab: Introduce kmalloc_obj() and family

[Randy Dunlap]

On 3/14/25 10:18 PM, Gustavo A. R. Silva wrote:
> 
>> These each return the assigned value of ptr (which may be NULL on
>> failure). For cases where the total size of the allocation is needed,
>> the kmalloc_obj_sz(), kmalloc_objs_sz(), and kmalloc_flex_sz() family
>> of macros can be used. For example:
>>
>>     info->size = struct_size(ptr, flex_member, count);
>>     ptr = kmalloc(info->size, gfp);
>>
>> becomes:
>>
>>     kmalloc_flex_sz(ptr, flex_member, count, gfp, &info->size);
> 
> I wonder if it'd be better to keep the gfp flags as the last argument
> for all these `*_sz()` cases:

That was my reaction when I reviewed the patch also. [I just didn't express it.]

> 
>     kmalloc_flex_sz(ptr, flex_member, count, &info->size, gpf);
> 
> Probably, even for __alloc_objs()


-- 
~Randy

Re: [PATCH v4 2/2] slab: Introduce kmalloc_obj() and family

[Linus Torvalds]

On Fri, 14 Mar 2025 at 17:15, Kees Cook <kees@kernel.org> wrote:
>
> Introduce type-aware kmalloc-family helpers to replace the common
> idioms for single, array, and flexible object allocations:
>
>         kmalloc_obj(ptr, gfp);
> [ ... ]

Honestly, I absolutely hate this.

Don't do this. It's a huge mistake.

Yes, it's a really easy and convenient interface to use.

And it's a ABSOLUTELY HORRENDOUSLY BAD interface to actually then *maintain*.

Why? Because it's simply visually and syntactically entirely wrong.
It's much much too easy to miss that there's an assignment there,
because the assignment is hidden inside that macro that visually looks
like a function call.

So when you scan the code, the data flow becomes very hard to see.

So no. A hard NAK on this. It's wrong, it's bad, and it's crap.

Maintaining code is AT LEAST as important as writing it, and arguably
much more so. And making code and data flow visually clear is
important, and this is actively detrimental to that.

So I understand why you want to do this, but no, this is absolutely
not the way to do it.

It needs at a minimum some way to make it very very visually clear
that this is an assignment to 'ptr', and honestly, I do not see how to
do that cleanly.

Alternatively, this might be acceptable if the syntax makes mistakes
much harder to do. So for example, if it wasn't just an assignment,
but also declared the 'ptr' variable, maybe it would become much safer
simply because it would make the compiler warn about mis-uses.

Using visual cues (something that makes it look like it's not a
regular function call) might also help. The traditional C way is
obviously to use ALL_CAPS() names, which is how we visually show "this
is a macro that doesn't necessarily work like the normal stuff". Some
variation of that might help the fundamental issue with your
horrendous thing.

But something very serious needs to be done before this is acceptable.
Because no, the advantage of writing

        kmalloc_obj(ptr, gfp);

instead of

        ptr = kmalloc(sizeof(*ptr), gfp);

is absolutely NOT worth the horrendous disadvantages of that
disgusting and wrong syntax. You saved a handful of characters, and
made the code faster to write, at the cost of making the result be
much worse.

My suggestion would be to look at some bigger pattern, maybe including
that declaration. To take a real example matching that kind of
pattern, we have

        struct mod_unload_taint *mod_taint;
        ...
        mod_taint = kmalloc(sizeof(*mod_taint), GFP_KERNEL);

and maybe those *two* lines could be combined into something saner like

        ALLOC(mod_taint, struct mod_unload_taint, GFP_KERNEL);

which would stand out visually (which is admittedly very different
from being "pretty" ;), and would also save you some typing because it
also gets rid of the declaration.

We allow declarations in the middle of code these days because we
needed it for the guarding macros, so this would be a new kind of
interface that dos that.

And no, I'm not married to that disgusting "ALLOC()" thing. I'm
throwing it out not as TheSOlution(tm), but as a "this interface
absolutely needs clarity and must stand out syntactically both to
humans and the compiler".

          Linus

Re: [PATCH v4 1/2] compiler_types: Introduce __flex_counter() and family

[Kees Cook]

On Fri, Mar 14, 2025 at 09:53:41PM -0700, Randy Dunlap wrote:
> Hi Kees,
> 
> 
> On 3/14/25 8:15 PM, Kees Cook wrote:
> 
> 
> > diff --git a/include/linux/overflow.h b/include/linux/overflow.h
> > index 0c7e3dcfe867..e2b81cb5576e 100644
> > --- a/include/linux/overflow.h
> > +++ b/include/linux/overflow.h
> > @@ -440,4 +440,40 @@ static inline size_t __must_check size_sub(size_t minuend, size_t subtrahend)
> >  #define DEFINE_FLEX(TYPE, NAME, MEMBER, COUNTER, COUNT)	\
> >  	_DEFINE_FLEX(TYPE, NAME, MEMBER, COUNT, = { .obj.COUNTER = COUNT, })
> >  
> > +/**
> > + * typeof_flex_counter() - Return the type of the counter variable of a given
> > + *                         flexible array member annotated by __counted_by().
> > + * @FAM: Pointer to the flexible array member within a given struct.
> > + *
> > + * Returns "size_t" if no annotation exists.
> 
> Please use
>  * Returns: <text>
> instead so that kernel-doc can make a special doc section for it.

Ah! Thanks -- I hadn't realized that the ":" induced special sections. I
think I have a bunch of other kern-doc clean-ups to do as well.

> 
> Same for patch 2/2.
> 
> > + */
> > +#define typeof_flex_counter(FAM)				\
> > +	typeof(_Generic(__flex_counter(FAM),			\
> > +			void *: (size_t)0,			\
> > +			default: *__flex_counter(FAM)))
> > +
> > +/** can_set_flex_counter() - Check if the counter associated with the given
> 
> Needs a newline between /** and the function name, as in set_flex_counter() below.

Whoops, thanks!

-Kees

-- 
Kees Cook

Re: [PATCH v4 2/2] slab: Introduce kmalloc_obj() and family

[Kees Cook]

On Sat, Mar 15, 2025 at 03:48:30PM +1030, Gustavo A. R. Silva wrote:
> 
> > These each return the assigned value of ptr (which may be NULL on
> > failure). For cases where the total size of the allocation is needed,
> > the kmalloc_obj_sz(), kmalloc_objs_sz(), and kmalloc_flex_sz() family
> > of macros can be used. For example:
> > 
> > 	info->size = struct_size(ptr, flex_member, count);
> > 	ptr = kmalloc(info->size, gfp);
> > 
> > becomes:
> > 
> > 	kmalloc_flex_sz(ptr, flex_member, count, gfp, &info->size);
> 
> I wonder if it'd be better to keep the gfp flags as the last argument
> for all these `*_sz()` cases:
> 
> 	kmalloc_flex_sz(ptr, flex_member, count, &info->size, gpf);
> 
> Probably, even for __alloc_objs()

I was following the pattern of the other "alternative helpers", like
kmalloc_node(), which adds the additional argument to the end. I have no
real opinion about it, so I defer to the slab developers. :)

-Kees

-- 
Kees Cook

Re: [PATCH v4 2/2] slab: Introduce kmalloc_obj() and family

[Kees Cook]

On Sat, Mar 15, 2025 at 08:31:21AM -1000, Linus Torvalds wrote:
> Alternatively, this might be acceptable if the syntax makes mistakes
> much harder to do. So for example, if it wasn't just an assignment,
> but also declared the 'ptr' variable, maybe it would become much safer
> simply because it would make the compiler warn about mis-uses.

Yeah, this is the real goal (it just so happens that it's fewer
characters). We need some way to gain both compile-time and run-time
sanity checking while making the writing of allocations easier.

> Using visual cues (something that makes it look like it's not a
> regular function call) might also help. The traditional C way is
> obviously to use ALL_CAPS() names, which is how we visually show "this
> is a macro that doesn't necessarily work like the normal stuff". Some
> variation of that might help the fundamental issue with your
> horrendous thing.

Yeah, I really didn't like using &ptr, etc. It just make it weirder.

> My suggestion would be to look at some bigger pattern, maybe including
> that declaration. To take a real example matching that kind of
> pattern, we have
> 
>         struct mod_unload_taint *mod_taint;
>         ...
>         mod_taint = kmalloc(sizeof(*mod_taint), GFP_KERNEL);
> 
> and maybe those *two* lines could be combined into something saner like
> 
>         ALLOC(mod_taint, struct mod_unload_taint, GFP_KERNEL);

Yeah, this covers a fair bit, but there's still an absolute ton of
"allocating stuff tracked by pointers in another structure", like:

	foo->items = kmalloc_array(sizeof(*foo->items), count, GFP_KERNEL)

There's no declaration there. :(

One thing that I noticed at the tail end of building up the Coccinelle
script was the pattern of variable-less "return" allocations:

struct foo *something(...)
{
	...
	return kmalloc(sizeof(struct foo), GFP_KERNEL);
}

And that doesn't fit either my proposal nor the ALLOC() proposal very
well.

> We allow declarations in the middle of code these days because we
> needed it for the guarding macros, so this would be a new kind of
> interface that dos that.

Yeah, that does make part of it easier.

> And no, I'm not married to that disgusting "ALLOC()" thing. I'm
> throwing it out not as TheSOlution(tm), but as a "this interface
> absolutely needs clarity and must stand out syntactically both to
> humans and the compiler".

What about making the redundant information the type/var itself instead
of just the size info of the existing API? For example:

	ptr = kmalloc_obj(ptr, GFP_KERNEL);

as in, don't pass a size, but pass the variable (or type) that can be
examined for type, size, alignment, etc info, but still require that the
assignment happen externally? In other words, at the end of the proposed
macro, instead of:

	(P) = __obj_ptr;

it just does:

	__obj_ptr;

so that the macro usage can't "drift" towards being used without an
assignment? And this would work for the "return" case as well:

	return kmalloc_objs(struct foo, count, GFP_KERNEL);

That would be a much smaller shift in the "usage" of the exist API.

Shall I refactor this proposal for that?

-- 
Kees Cook

Re: [PATCH v4 2/2] slab: Introduce kmalloc_obj() and family

[Linus Torvalds]

[-- Attachment #1: Type: text/plain, Size: 599 bytes --]

[ Sorry, on mobile now, so html crud and no lists ]

On Sat, Mar 15, 2025, 08:56 Kees Cook <kees@kernel.org> wrote:

>
> What about making the redundant information the type/var itself instead
> of just the size info of the existing API? For example:
>
>         ptr = kmalloc_obj(ptr, GFP_KERNEL);
>


Yes, using "sizeof" and "typeof" on that first argument (for the malloc and
then the final cast, respectively) sounds like a fine interface, and it's
still obviously visually an assignment.

I think we already have some interfaces like this, I have no objections to
that pattern.

         Linus

[-- Attachment #2: Type: text/html, Size: 1102 bytes --]

Re: [PATCH v4 1/2] compiler_types: Introduce __flex_counter() and family

[Miguel Ojeda]

On Sat, Mar 15, 2025 at 4:15 AM Kees Cook <kees@kernel.org> wrote:
>
> + * clang: https://github.com/llvm/llvm-project/pull/102549

That is an unmerged PR -- should we link to the merged one?

Or, better, to the docs, since they seem to exist:

    https://clang.llvm.org/docs/LanguageExtensions.html#builtin-counted-by-ref

?

Thanks!

Cheers,
Miguel

Re: [PATCH v4 1/2] compiler_types: Introduce __flex_counter() and family

[Kees Cook]

On Sat, Mar 15, 2025 at 08:47:46PM +0100, Miguel Ojeda wrote:
> On Sat, Mar 15, 2025 at 4:15 AM Kees Cook <kees@kernel.org> wrote:
> >
> > + * clang: https://github.com/llvm/llvm-project/pull/102549
> 
> That is an unmerged PR -- should we link to the merged one?
> 
> Or, better, to the docs, since they seem to exist:
> 
>     https://clang.llvm.org/docs/LanguageExtensions.html#builtin-counted-by-ref

Ah! Perfect. I was looking under builtins and couldn't find it. :) I
will update the link.

-- 
Kees Cook

Re: [PATCH v4 1/2] compiler_types: Introduce __flex_counter() and family

[Przemek Kitszel]

On 3/15/25 04:15, Kees Cook wrote:
> Introduce __flex_counter() which wraps __builtin_counted_by_ref(),
> as newly introduced by GCC[1] and Clang[2]. Use of __flex_counter()
> allows access to the counter member of a struct's flexible array member
> when it has been annotated with __counted_by().
> 
> Introduce typeof_flex_counter(), can_set_flex_counter(), and
> set_flex_counter() to provide the needed _Generic() wrappers to get sane
> results out of __flex_counter().
> 
> For example, with:
> 
> 	struct foo {
> 		int counter;
> 		short array[] __counted_by(counter);
> 	} *p;
> 
> __flex_counter(p->array) will resolve to: &p->counter
> 
> typeof_flex_counter(p->array) will resolve to "int". (If p->array was not
> annotated, it would resolve to "size_t".)
> 
> can_set_flex_counter(p->array, COUNT) is the same as:
> 
> 	COUNT <= type_max(p->counter) && COUNT >= type_min(p->counter)
> 
> (If p->array was not annotated it would return true since everything
> fits in size_t.)
> 
> set_flex_counter(p->array, COUNT) is the same as:
> 
> 	p->counter = COUNT;
> 
> (It is a no-op if p->array is not annotated with __counted_by().)
> 
> Signed-off-by: Kees Cook <kees@kernel.org>

I agree that there is no suitable fallback handy, but I see counter
as integral part of the struct (in contrast to being merely annotation),
IOW, without set_flex_counter() doing the assignment, someone will
reference it later anyway, without any warning when kzalloc()'d

So, maybe BUILD_BUG() instead of no-op?

> +#define set_flex_counter(FAM, COUNT)				\
> +({								\
> +	*_Generic(__flex_counter(FAM),				\
> +		  void *:  &(size_t){ 0 },			\
> +		  default: __flex_counter(FAM)) = (COUNT);	\
> +})
> +
>   #endif /* __LINUX_OVERFLOW_H */

Re: [PATCH v4 1/2] compiler_types: Introduce __flex_counter() and family

[Przemek Kitszel]

On 3/17/25 10:26, Przemek Kitszel wrote:
> On 3/15/25 04:15, Kees Cook wrote:
>> Introduce __flex_counter() which wraps __builtin_counted_by_ref(),
>> as newly introduced by GCC[1] and Clang[2]. Use of __flex_counter()
>> allows access to the counter member of a struct's flexible array member
>> when it has been annotated with __counted_by().
>>
>> Introduce typeof_flex_counter(), can_set_flex_counter(), and
>> set_flex_counter() to provide the needed _Generic() wrappers to get sane
>> results out of __flex_counter().
>>
>> For example, with:
>>
>>     struct foo {
>>         int counter;
>>         short array[] __counted_by(counter);
>>     } *p;
>>
>> __flex_counter(p->array) will resolve to: &p->counter
>>
>> typeof_flex_counter(p->array) will resolve to "int". (If p->array was not
>> annotated, it would resolve to "size_t".)
>>
>> can_set_flex_counter(p->array, COUNT) is the same as:
>>
>>     COUNT <= type_max(p->counter) && COUNT >= type_min(p->counter)
>>
>> (If p->array was not annotated it would return true since everything
>> fits in size_t.)
>>
>> set_flex_counter(p->array, COUNT) is the same as:
>>
>>     p->counter = COUNT;
>>
>> (It is a no-op if p->array is not annotated with __counted_by().)
>>
>> Signed-off-by: Kees Cook <kees@kernel.org>
> 
> I agree that there is no suitable fallback handy, but I see counter
> as integral part of the struct (in contrast to being merely annotation),
> IOW, without set_flex_counter() doing the assignment, someone will
> reference it later anyway, without any warning when kzalloc()'d
> 
> So, maybe BUILD_BUG() instead of no-op?

I get that so far this is only used as an internal helper (in the next
patch), so for me it would be also fine to just add __ prefix: 
__set_flex_counter(), at least until the following is true:
  "manual initialization of the flexible array counter is still
required (at some point) after allocation as not all compiler versions
support the __counted_by annotation yet"

> 
>> +#define set_flex_counter(FAM, COUNT)                \
>> +({                                \
>> +    *_Generic(__flex_counter(FAM),                \
>> +          void *:  &(size_t){ 0 },            \
>> +          default: __flex_counter(FAM)) = (COUNT);    \
>> +})
>> +
>>   #endif /* __LINUX_OVERFLOW_H */
> 

Re: [PATCH v4 1/2] compiler_types: Introduce __flex_counter() and family

[Kees Cook]

On Mon, Mar 17, 2025 at 10:43:38AM +0100, Przemek Kitszel wrote:
> On 3/17/25 10:26, Przemek Kitszel wrote:
> > On 3/15/25 04:15, Kees Cook wrote:
> > > Introduce __flex_counter() which wraps __builtin_counted_by_ref(),
> > > as newly introduced by GCC[1] and Clang[2]. Use of __flex_counter()
> > > allows access to the counter member of a struct's flexible array member
> > > when it has been annotated with __counted_by().
> > > 
> > > Introduce typeof_flex_counter(), can_set_flex_counter(), and
> > > set_flex_counter() to provide the needed _Generic() wrappers to get sane
> > > results out of __flex_counter().
> > > 
> > > For example, with:
> > > 
> > >     struct foo {
> > >         int counter;
> > >         short array[] __counted_by(counter);
> > >     } *p;
> > > 
> > > __flex_counter(p->array) will resolve to: &p->counter
> > > 
> > > typeof_flex_counter(p->array) will resolve to "int". (If p->array was not
> > > annotated, it would resolve to "size_t".)
> > > 
> > > can_set_flex_counter(p->array, COUNT) is the same as:
> > > 
> > >     COUNT <= type_max(p->counter) && COUNT >= type_min(p->counter)
> > > 
> > > (If p->array was not annotated it would return true since everything
> > > fits in size_t.)
> > > 
> > > set_flex_counter(p->array, COUNT) is the same as:
> > > 
> > >     p->counter = COUNT;
> > > 
> > > (It is a no-op if p->array is not annotated with __counted_by().)
> > > 
> > > Signed-off-by: Kees Cook <kees@kernel.org>
> > 
> > I agree that there is no suitable fallback handy, but I see counter
> > as integral part of the struct (in contrast to being merely annotation),
> > IOW, without set_flex_counter() doing the assignment, someone will
> > reference it later anyway, without any warning when kzalloc()'d
> > 
> > So, maybe BUILD_BUG() instead of no-op?
> 
> I get that so far this is only used as an internal helper (in the next
> patch), so for me it would be also fine to just add __ prefix:
> __set_flex_counter(), at least until the following is true:
>  "manual initialization of the flexible array counter is still
> required (at some point) after allocation as not all compiler versions
> support the __counted_by annotation yet"

Yeah, that's fair. I will rename set_... and can_set_...

Thought FWIW I'm not sure we'll ever want a BUILD_BUG_ON() just because
there will be flex arrays with future annotations that can't have their
counter set (e.g. annotations that indicate globals, expressions, etc --
support for these cases is coming, if slowly[1]).

-Kees

[1] loooong thread https://gcc.gnu.org/pipermail/gcc-patches/2025-March/677024.html

-- 
Kees Cook

Re: [PATCH v4 2/2] slab: Introduce kmalloc_obj() and family

[Matthew Wilcox]

On Fri, Mar 14, 2025 at 08:15:45PM -0700, Kees Cook wrote:
> +Performing open-coded kmalloc()-family allocation assignments prevents
> +the kernel (and compiler) from being able to examine the type of the
> +variable being assigned, which limits any related introspection that
> +may help with alignment, wrap-around, or additional hardening. The
> +kmalloc_obj()-family of macros provide this introspection, which can be
> +used for the common code patterns for single, array, and flexible object
> +allocations. For example, these open coded assignments::
> +
> +	ptr = kmalloc(sizeof(*ptr), gfp);
> +	ptr = kmalloc(sizeof(struct the_type_of_ptr_obj), gfp);
> +	ptr = kzalloc(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);
> +
> +become, respectively::
> +
> +	kmalloc_obj(ptr, gfp);
> +	kzalloc_obj(ptr, gfp);
> +	kmalloc_objs(ptr, count, gfp);
> +	kzalloc_objs(ptr, count, gfp);
> +	kmalloc_flex(ptr, flex_member, count, gfp);

I'd like to propose a different approach for consideration.

The first two are obvious.  If we now believe that object type is the
important thing, well we have an API for that:

struct buffer_head { ... };

	bh_cache = KMEM_CACHE(buffer_head, SLAB_RECLAIM_ACCOUNT);
...
	ptr = kmem_cache_alloc(bh_cache, GFP_KERNEL);

It's a little more verbose than what you're suggesting, but it does give
us the chance to specify SLAB_ flags.  It already does the alignment
optimisation you're suggesting.  Maybe we can use some macro sugar to
simplify this.

The array ones are a little tougher.  Let's set them aside for the
moment and tackle the really hard one; kmalloc_flex.

Slab is fundamentally built on all objects in the slab being the same
size.  But maybe someone sufficiently enterprising could build a
"variable sized" slab variant?  If we're saying that object type is
more important a discriminator than object size, then perhaps grouping
objects of the same size together isn't nearly as useful as grouping
objects of the same type together, even if they're different sizes.

Might be a good GSoC/outreachy project?

Re: [PATCH v4 2/2] slab: Introduce kmalloc_obj() and family

[Kees Cook]

On Tue, Oct 07, 2025 at 03:07:33AM +0100, Matthew Wilcox wrote:
> On Fri, Mar 14, 2025 at 08:15:45PM -0700, Kees Cook wrote:
> > +Performing open-coded kmalloc()-family allocation assignments prevents
> > +the kernel (and compiler) from being able to examine the type of the
> > +variable being assigned, which limits any related introspection that
> > +may help with alignment, wrap-around, or additional hardening. The
> > +kmalloc_obj()-family of macros provide this introspection, which can be
> > +used for the common code patterns for single, array, and flexible object
> > +allocations. For example, these open coded assignments::
> > +
> > +	ptr = kmalloc(sizeof(*ptr), gfp);
> > +	ptr = kmalloc(sizeof(struct the_type_of_ptr_obj), gfp);
> > +	ptr = kzalloc(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);
> > +
> > +become, respectively::
> > +
> > +	kmalloc_obj(ptr, gfp);
> > +	kzalloc_obj(ptr, gfp);
> > +	kmalloc_objs(ptr, count, gfp);
> > +	kzalloc_objs(ptr, count, gfp);
> > +	kmalloc_flex(ptr, flex_member, count, gfp);
> 
> I'd like to propose a different approach for consideration.
> 
> The first two are obvious.  If we now believe that object type is the
> important thing, well we have an API for that:
> 
> struct buffer_head { ... };
> 
> 	bh_cache = KMEM_CACHE(buffer_head, SLAB_RECLAIM_ACCOUNT);
> ...
> 	ptr = kmem_cache_alloc(bh_cache, GFP_KERNEL);
> 
> It's a little more verbose than what you're suggesting, but it does give
> us the chance to specify SLAB_ flags.  It already does the alignment
> optimisation you're suggesting.  Maybe we can use some macro sugar to
> simplify this.

I just don't think this is remotely feasible. We have tens of thousands
of kmalloc callers in the kernel and that's just not going to work. Also
it's wildly redundant given that the type info is present already in
the proposed series -- the point is ot make this something the allocator
can use (if it wants to) and not depend on the user to do it.
kmem_cache_alloc is just too inflexible.

Doing the mechanical transformation of these callsites is also largely
done already via a Coccinelle script I mentioned in earlier threads:
https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/kmalloc_objs.cocci

https://web.git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/commit/?h=dev/v6.14-rc2/alloc_obj/v4-treewide&id=106fd376feea1699868859e82416d5b7c50866ee
7568 files changed, 16342 insertions, 18580 deletions

> The array ones are a little tougher.  Let's set them aside for the
> moment and tackle the really hard one; kmalloc_flex.
> 
> Slab is fundamentally built on all objects in the slab being the same
> size.  But maybe someone sufficiently enterprising could build a
> "variable sized" slab variant?  If we're saying that object type is
> more important a discriminator than object size, then perhaps grouping
> objects of the same size together isn't nearly as useful as grouping
> objects of the same type together, even if they're different sizes.
> 
> Might be a good GSoC/outreachy project?

I already did this (see kmem_buckets_create()) and expanded on it
with the per-call-site series I proposed:
https://lore.kernel.org/all/20240809073309.2134488-1-kees@kernel.org/
https://lore.kernel.org/all/20240809073309.2134488-5-kees@kernel.org/

But all of that is orthogonal to just _having_ the type info available.

-Kees

-- 
Kees Cook

Re: [PATCH v4 2/2] slab: Introduce kmalloc_obj() and family

[Christoph Lameter (Ampere)]

On Tue, 7 Oct 2025, Kees Cook wrote:

> But all of that is orthogonal to just _having_ the type info available.

iOS did go the path of creating basically one slab cache for each
"type" of kmalloc for security reasons.

See https://security.apple.com/blog/towards-the-next-generation-of-xnu-memory-safety/

Re: [PATCH v4 2/2] slab: Introduce kmalloc_obj() and family

[Marco Elver]

On Tue, 7 Oct 2025 at 19:47, Christoph Lameter (Ampere) <cl@gentwo.org> wrote:
>
> On Tue, 7 Oct 2025, Kees Cook wrote:
>
> > But all of that is orthogonal to just _having_ the type info available.
>
> iOS did go the path of creating basically one slab cache for each
> "type" of kmalloc for security reasons.
>
> See https://security.apple.com/blog/towards-the-next-generation-of-xnu-memory-safety/

We can get something similar to that with:
https://lore.kernel.org/all/20250825154505.1558444-1-elver@google.com/
Pending compiler support which is going to become available in a few
months (probably).
That version used the existing RANDOM_KMALLOC_CACHES choice of 16 slab
caches, but there's no fundamental limitation to go higher.
Note, this mitigation is likely not as strong as we'd like to without
SLAB_VIRTUAL (or so I'm told): https://lwn.net/Articles/944647/

Re: [PATCH v4 2/2] slab: Introduce kmalloc_obj() and family

[Kees Cook]

On Tue, Oct 07, 2025 at 08:18:28PM +0200, Marco Elver wrote:
> On Tue, 7 Oct 2025 at 19:47, Christoph Lameter (Ampere) <cl@gentwo.org> wrote:
> >
> > On Tue, 7 Oct 2025, Kees Cook wrote:
> >
> > > But all of that is orthogonal to just _having_ the type info available.
> >
> > iOS did go the path of creating basically one slab cache for each
> > "type" of kmalloc for security reasons.
> >
> > See https://security.apple.com/blog/towards-the-next-generation-of-xnu-memory-safety/
> 
> We can get something similar to that with:
> https://lore.kernel.org/all/20250825154505.1558444-1-elver@google.com/
> Pending compiler support which is going to become available in a few
> months (probably).
> That version used the existing RANDOM_KMALLOC_CACHES choice of 16 slab
> caches, but there's no fundamental limitation to go higher.

Right -- having compiler support for dealing with types at compile time
means we can create the slab caches statically (instead of any particular
fixed number, even the 16 from RANDOM_KMALLOC_CACHES). Another compiler
feature that might help here is getting a unique u32 for arbitrary type
info, which is also how KCFI works:
https://lore.kernel.org/linux-hardening/20250926030252.2387681-1-kees@kernel.org/

My main issue is that I prefer explicitly exposing the type instead of
having the compiler have to guess. We want it for more than just slab
isolation (e.g. examining alignment).

> Note, this mitigation is likely not as strong as we'd like to without
> SLAB_VIRTUAL (or so I'm told): https://lwn.net/Articles/944647/

True, but both "halves" are needed -- SLAB_VIRTUAL isn't as robust
without the type separation either.

-Kees

-- 
Kees Cook

Re: [PATCH v4 2/2] slab: Introduce kmalloc_obj() and family

[Vegard Nossum]

On 08/10/2025 06:20, Kees Cook wrote:
> On Tue, Oct 07, 2025 at 08:18:28PM +0200, Marco Elver wrote:
>> On Tue, 7 Oct 2025 at 19:47, Christoph Lameter (Ampere) <cl@gentwo.org> wrote:
>>> On Tue, 7 Oct 2025, Kees Cook wrote:
>>> iOS did go the path of creating basically one slab cache for each
>>> "type" of kmalloc for security reasons.
>>>
>>> See https://security.apple.com/blog/towards-the-next-generation-of-xnu-memory-safety/
>
>> We can get something similar to that with:
>> https://lore.kernel.org/all/20250825154505.1558444-1-elver@google.com/
>> Pending compiler support which is going to become available in a few
>> months (probably).
>> That version used the existing RANDOM_KMALLOC_CACHES choice of 16 slab
>> caches, but there's no fundamental limitation to go higher.
> 
> Right -- having compiler support for dealing with types at compile time
> means we can create the slab caches statically (instead of any particular
> fixed number, even the 16 from RANDOM_KMALLOC_CACHES).

Maybe I'm missing the point here, but I think we can already do per-
callsite static caches without specific new compiler support:

struct kmalloc_cache {
     const char *type_name;
     unsigned long caller;
     unsigned int alignment;
     unsigned int size;
     gfp_t gfp_flags;
     // ...
};

extern void *_kmalloc_cache(struct kmalloc_cache *cache);

#define kmalloc_type(type, _gfp_flags) \
     ({ \
         __label__ __here; __here: \
         static struct kmalloc_cache \
             __attribute__((__section__(".kmalloc_caches"))) \
             _cache = { \
                 .type_name = #type, \
                 .caller = (unsigned long)&&__here, \
                 .alignment = alignof(type), \
                 .size = sizeof(type), \
                 .gfp_flags = (_gfp_flags), \
             }; \
         (type *) _kmalloc_cache(&_cache); \
     })

struct device {
     int name[32];
     void *priv;
};

int foo()
{
     struct device *dev = kmalloc_type(struct device, GFP_KERNEL);
     // ...
}

// initialize all static kmalloc caches during boot if needed
// (requires linker script support)

extern struct kmalloc_cache kmalloc_caches_start[];
extern struct kmalloc_cache kmalloc_caches_end[];

void init_cache(struct kmalloc_cache *)
{
     // ...
}

void init_caches()
{
     for (struct kmalloc_cache *cache = kmalloc_caches_start;
         cache != kmalloc_caches_end; ++cache)
     {
         init_cache(cache);
     }
}

Godbolt for playing with it: https://godbolt.org/z/E1c6q9avn

If you really want just one cache per type, you can funnel all the
callers through a single function (single allocation point)?


Vegard

Re: [PATCH v4 2/2] slab: Introduce kmalloc_obj() and family

[Marco Elver]

On Wed, 8 Oct 2025 at 09:49, Vegard Nossum <vegard.nossum@oracle.com> wrote:
>
>
> On 08/10/2025 06:20, Kees Cook wrote:
> > On Tue, Oct 07, 2025 at 08:18:28PM +0200, Marco Elver wrote:
> >> On Tue, 7 Oct 2025 at 19:47, Christoph Lameter (Ampere) <cl@gentwo.org> wrote:
> >>> On Tue, 7 Oct 2025, Kees Cook wrote:
> >>> iOS did go the path of creating basically one slab cache for each
> >>> "type" of kmalloc for security reasons.
> >>>
> >>> See https://security.apple.com/blog/towards-the-next-generation-of-xnu-memory-safety/
> >
> >> We can get something similar to that with:
> >> https://lore.kernel.org/all/20250825154505.1558444-1-elver@google.com/
> >> Pending compiler support which is going to become available in a few
> >> months (probably).
> >> That version used the existing RANDOM_KMALLOC_CACHES choice of 16 slab
> >> caches, but there's no fundamental limitation to go higher.
> >
> > Right -- having compiler support for dealing with types at compile time
> > means we can create the slab caches statically (instead of any particular
> > fixed number, even the 16 from RANDOM_KMALLOC_CACHES).
>
> Maybe I'm missing the point here, but I think we can already do per-
> callsite static caches without specific new compiler support:

What we want is not per-callsite but per-type caches, possibly with
some smarter cache organization based on the properties of that type
(does type contain/is pointer), where the latter is required if we
cannot have as many caches as there are types. Per-callsite caches
could be stronger than per-type caches (with the exception where a
single callsite can allocate multiple types), but neither per-callsite
and full per-type caches are likely feasible due to performance
reasons. So we need some scheme that allows bounding the number of
caches, and letting the compiler help us out with type introspection
is probably the most reasonable approach.