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