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

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

[Kees Cook]

On Wed, Mar 18, 2026 at 05:33:35PM +0100, Alejandro Colomar wrote:
> How about using a struct?  That's the idiomatic way of having
> incompatible types.  Since these are used through macros, it wouldn't
> change anything to users.
> 
> The macro definitions would have to change.  For example:
> 
> -#define GFP_NOFS	(__GFP_RECLAIM | __GFP_IO)
> +#define GFP_NOFS	((void)0, (struct gfp){__GFP_RECLAIM | __GFP_IO})
> 
> We don't need any new language features.  This is backwards compatible
> up to C99.  And it might end up being simpler than __strict typedef.

Yeah, if we can convince the mm folks. (Added to CC.) It'd require some
kind of macro to build bits in the many places where values are or'ed
together.

> [...]
> After sleeping, I had some idea.
> 
> We could have coccinelle add typeof() around the first parameter when
> it's an expression (not a type).  Then, we could enforce that the first
> parameter is a type name.
> 
> That is:
> 
> 	 p = kmalloc_objs(int, 42);  // ok
> 	-q = kmalloc_objs(*p, 7);
> 	+q = kmalloc_objs(typeof(*p), 7);
> 
> I expect this would be doable with coccinelle.
> 
> Then, new code would be required to pass a type name.  And people could
> slowly replace the existing typeof() calls at their own pace.
> 
> What do you think?

Well, it'd serve as a visual indicator, but it's redundant (typeof() is
already used internally). Given it would only be a potential for
confusion on integral types, I'm less convinced this needs solving.

For completeness, though, this Coccinelle script:

// Options: --include-headers-for-types --all-includes --include-headers --keep-comments
virtual patch

@type_not_var depends on patch@
type TYPE;
TYPE *VAR;
identifier ALLOC =
{kmalloc_obj,kzalloc_obj,kmalloc_objs,kzalloc_objs,kmalloc_flex,kzalloc_flex};
@@

        VAR = ALLOC(
-                   *VAR
+                   TYPE
                        , ...)

Produces:

 6007 files changed, 12430 insertions(+), 11767 deletions(-)

Which is a lot of churn...

-- 
Kees Cook

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

[Alejandro Colomar]

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

Hi Kees,

On 2026-03-19T13:12:41-0700, Kees Cook wrote:
> On Wed, Mar 18, 2026 at 05:33:35PM +0100, Alejandro Colomar wrote:
> > [...]
> > After sleeping, I had some idea.
> > 
> > We could have coccinelle add typeof() around the first parameter when
> > it's an expression (not a type).  Then, we could enforce that the first
> > parameter is a type name.
> > 
> > That is:
> > 
> > 	 p = kmalloc_objs(int, 42);  // ok
> > 	-q = kmalloc_objs(*p, 7);
> > 	+q = kmalloc_objs(typeof(*p), 7);
> > 
> > I expect this would be doable with coccinelle.
> > 
> > Then, new code would be required to pass a type name.  And people could
> > slowly replace the existing typeof() calls at their own pace.
> > 
> > What do you think?
> 
> Well, it'd serve as a visual indicator, but it's redundant (typeof() is
> already used internally).

It is redundant now.  But my idea would be two steps:

1)  Add the redundant typeof() in the first parameter, when it's not a
    type already.

2)  Change the kmalloc_objs() implementation so that it doesn't do
    typeof() internally.  This would make step 1 non-redundant.

> Given it would only be a potential for
> confusion on integral types, I'm less convinced this needs solving.

It's directly adding safety for integral types only, yes.

But it's also improving readability.  There's no precedent of macros
that take a variable just for its type, and it feels a bit awkward to
read the current one.  By doing typeof(), people can relate better why
it takes that parameter.

If I read "q = kmalloc_objs(*q, 7)", the first thing I wonder is: why is
this macro reading *q??  If I read "q = kmalloc_objs(typeof(*p), 7)",
then all's fine in my brain.

> For completeness, though, this Coccinelle script:
> 
> // Options: --include-headers-for-types --all-includes --include-headers --keep-comments
> virtual patch
> 
> @type_not_var depends on patch@
> type TYPE;
> TYPE *VAR;
> identifier ALLOC =
> {kmalloc_obj,kzalloc_obj,kmalloc_objs,kzalloc_objs,kmalloc_flex,kzalloc_flex};
> @@
> 
>         VAR = ALLOC(
> -                   *VAR
> +                   TYPE
>                         , ...)
> 
> Produces:
> 
>  6007 files changed, 12430 insertions(+), 11767 deletions(-)
> 
> Which is a lot of churn...

Yeah, it would be a lot of churn if we were at rest.  But since the dust
has not yet settled, it might be doable.


Have a lovely night!
Alex

-- 
<https://www.alejandro-colomar.es>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]