Re: kalloc_objs() may not be as safe as it seems

[Kees Cook]

On Mon, Mar 16, 2026 at 07:33:34PM +0100, Alejandro Colomar wrote:
> Hi Kees,
> 
> I just learnt about kalloc_objs() et al. from
> <https://lwn.net/Articles/1062856/>.
> 
>     ptr = kmalloc_obj(*ptr);
>     ptr = kmalloc_objs(*ptr, n);
> 
> This resembles a lot the macro we have in shadow-utils, malloc_T(),
> which would be used as (to resemble the above):
> 
>     ptr = malloc_T(1, typeof(*ptr));  // But we'd really pass the type
>     ptr = malloc_T(n, typeof(*ptr));  // But we'd really pass the type
> 
> But I've noticed some design mistakes that make it not as safe as it
> seems.
> 
> Default arguments
> ~~~~~~~~~~~~~~~~~
> 
> I tend to think it's simpler to have a single API that works for both
> 1 element and multiple elements.  The special-casing of 1 element seems
> unnecessary, and having a literal 1 works just fine.

This is a reasonable opinion, but not one I wanted to try to fight for
with the Linux developer community. Linus has tended to frown on adding
any new burden when making these kinds of changes, and expecting
everyone to add a (seemingly) redundant "1" to all API calls (or an
empty comma) seems unlikely to fly.

> I think the combination of having the macros be variadic (for gfp) with
> having two very similar APIs that differ in number of arguments, and
> all those arguments being integer types, is prone to errors.  Consider
> the case where one would accidentally write
> 
> 	ptr = kmalloc_obj(*ptr, n);  // Bogus
> instead of
> 	ptr = kmalloc_objs(*ptr, n);

This loss of GFP flags wasn't part of my original design, and I would
agree that given the lack of type checking for GFP flags, this does look
like a potential foot-gun.

> The compiler wouldn't realize at all.  That's a strong argument in
> favour of having default arguments be required to be explicit, with an
> empty argument:
> 
> 	ptr = kmalloc_obj(*ptr,);
> 	ptr = kmalloc_objs(*ptr, n,);
> 
> I know you (and Linus too, FWIW) have previously claimed that it looks
> weird to the eye.  But I'm pretty sure you could get used to it.  That's
> certainly going to be safer.
> 
> With mandatory empty arguments, the compiler would easily distinguish
> mistakes like the one above.

I'd rather we get something like the __strict typedef so "gfp_t" would
be a true separate type, not just a silent alias of "int".

> Type safety
> ~~~~~~~~~~~
> 
> Apart from the issues with the above, the ability to pass a variable
> instead of a type name is also a bad choice.  In shadow-utils, we

This was a first-order requirement, or we'd never be able to refactor
the codebase to use the new API. There was already a heavy mixture of
types and variables used within sizeof(), and trying to take that part
and find types exceeded Coccinelle's abilities.

Making the refactor "trivial" was important; but see below.

> require a type name, and a variable is rejected.  We implement that with
> the typeas() macro:
> 
> 	#define typeas(T)  typeof((T){0})
> 
> This macro works exactly like typeof(), but it requires that the input
> is also a type.  Passing a variable is a syntax error.  We implement
> malloc_T() with it:
> 
> 	// malloc_T - malloc type-safe
> 	#define malloc_T_(n, T)                                       \
> 	({                                                            \
> 		(typeas(T) *){reallocarray(n, sizeof(T))};            \
> 	})
> 
> which is used as (taking some arbitrary examples from shadow-utils):
> 
> 	lp = xmalloc_T(1, struct link_name);
> 	targs = xmalloc_T(n_args + 3, char *);
> 
> Some reasons for passing a type name instead of a variable are:
> 
> -  It allows grepping for all allocations of a given type.
> -  It adds readability.  It's similar to declaring variables with some
>    explicit type, vs. using 'auto' (__auto_type) everywhere.

Sure, and that's why many places were already using type names, and
the use of "auto" was even one of Linus's driving examples for why the
new API could be very easily used.

> But there's also a safety aspect.  Consider we want to allocate an array
> of 42 ints.  And consider the programmer accidentally swaps arguments.
> 
> 	int *p = malloc_T(int, 42);  // syntax error
> 	int *p = malloc_T(42, int);
> vs
> 	int *p = kmalloc_objs(*p, 42);
> 	int *p = kmalloc_objs(42, *p);  // Bogus
> 
> The latter is dereferencing an uninitialized pointer.  If for some
> reason the pointer had a value before this call, you'd be allocating as
> many elements as *p says, which would be bogus, and since typeof(42) is
> the same as typeof(*p), the return type would be valid, so this would
> still compile.

Yeah, this is a foot-gun too. I'm open to requiring a type, but it's a
significant amount of careful refactoring needed to accomplish it. It
was a weakness of the existing API, though it was more "obvious" since
it was visually contained by "sizeof()".

-Kees

-- 
Kees Cook