Message-ID: <CAHk-=wiF9-vf3zW8vXoFGk+bo0=dSoJMm_nmVzsTDCerZzhGaw@mail.gmail.com>
Date: Thu, 27 Nov 2025 21:35:35 -0800
From: Linus Torvalds <torvalds@...ux-foundation.org>
To: Steven Rostedt <rostedt@...dmis.org>
Cc: Kees Cook <kees@...nel.org>, LKML <linux-kernel@...r.kernel.org>, 
	linux-hardening@...r.kernel.org, 
	Linux Trace Kernel <linux-trace-kernel@...r.kernel.org>, Masami Hiramatsu <mhiramat@...nel.org>, 
	Mathieu Desnoyers <mathieu.desnoyers@...icios.com>, 
	"Gustavo A. R. Silva" <gustavoars@...nel.org>
Subject: Re: [PATCH] overflow: Introduce struct_offset() to get offset of member

On Thu, 27 Nov 2025 at 19:27, Steven Rostedt <rostedt@...dmis.org> wrote:
>
> On Thu, 27 Nov 2025 18:00:01 -0800
> Kees Cook <kees@...nel.org> wrote:
>
> > If you have an instance and not a pointer, just slap on a & :)
>
> :-p
>
> I guess that would work. Although it will kinda look a bit strange.
> At least to me.

It does look a tiny bit odd, and it's not always wonderful, but there
are advantages to conceptually passing in the pointer rather than the
actual instance.

The two main advantages are that

 (a) a pointer is *usually* what you have. Not always, no, but it's
the common thing in the kernel. We have a lot fewer cases of 'struct
on stack' than we have 'pointer to struct somewhere'.

but even more so:

 (b) unless the macro makes it very *very* obvious in the name that it
only looks at the *type* of the argument, dereferencing the pointer
can easily look absolutely horrendous.

We had an example of that (b) case recently [*] where the code looked like this:

        typeof(VAR) *__obj_ptr = NULL;
        ....__can_set_flex_counter(__obj_ptr->FAM, __count) ...

and note that combination of "NULL pointer" and "dereference that pointer".

The code was *correct*, because that '__can_set_flex_counter()' macro
in that case only looked at the *type* of the first argument, so the
dereference didn't "really" happen as an actual access to any memory
behind the pointer, but it doesn't make it really look any better. We
really don't want to have those kinds of visual patterns, it just
makes people go "WHAA?!??".

Now, with proper naming, you can make that obvious. You can make it
really clear that "this is looking just at the type of that argument,
not the value of the expression it is given".

But honestly, I think that "really clear" comes close to just having
to have that "__typeof__()" pattern.

Can people learn that a name like "struct_offset()" always just takes
the type of the first argument, and get used to patterns like that?
Sure. Absolutely. But I'm not sure we really want people to learn that
pattern if there are alternatives.

So I do think that the "struct_size()" syntax - which takes the
*pointer* to the object and then does "sizeof(*(p))" internally - is
the better syntax. Not just because we *do* typically have the pointer
as the variable we are trying to get the size of, but exactly because
we do not want it to visually look like a dereference of a pointer
that may not have a valid value yet.

Put another way: yes, it's slightly conceptually odd to pass in the
pointer in order to actually get the size (or offset of a member, in
your case) of the actual *instance*, but I think "slightly
conceptually odd" is a small price to pay to not it look more regular.

And yes, I obviously do realize that 'sizeof()' and 'offsetof' and
'__alignof__' all take the actual type, not the pointer to the type.
But they are very special operations in C, and we should not make
other operations act like them unless they have some very clear and
obvious clue.

For example, we do have 'struct_size_t()' that takes the actual type,
not the pointer to the type. It _only_ takes an explicit type, and
cannot even take an instance of an object (although we could *make* it
take an instance by adding a '__typeof__()' into that macro). But at
least the '_t' part hopefully makes that obvious, and even if we did
make it take an instance, you'd never use it on the above kind of
pointer without a valid value, because then you'd use the non-_t()
version.

So that 'xyz_t()' version then gets used for things where you
explicitly state the type, and it all looks fairly obvious, eg:

        len = struct_size_t(struct pid, numbers, level + 1);

doesn't get that "WHAA?!??!" kind of reaction.

[ And so I actually think it's good that it only takes an explicit
type - if you really have an instance, I think it's better to use just
"struct_size(&instance, ...)" even if we _could_ easily make syntax
like "struct_size_t(instance, ...)" work. ]

          Linus

[*] https://lore.kernel.org/all/CAHk-=wiNnECns4B3qxRsCykkHwzovT+3wG738fUhq5E+3Lxxbg@mail.gmail.com/

Powered by blists - more mailing lists