mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 0/2] randstruct: fix container_of() false positives after __mptr removal
@ 2026-09-03 23:24 Kees Cook
  2026-09-03 23:24 ` [PATCH 1/2] " Kees Cook
  2026-09-03 23:24 ` [PATCH 2/2] randstruct: report bad casts as warnings rather than notes Kees Cook
  0 siblings, 2 replies; 6+ messages in thread
From: Kees Cook @ 2026-09-03 23:24 UTC (permalink / raw)
  To: Bill Wendling
  Cc: Kees Cook, Vincent Mailhol, Nathan Chancellor, Nick Desaulniers,
	Justin Stitt, Greg Kroah-Hartman, linux-kernel, linux-hardening

Hi,

The GCC randstruct implementation specifically keyed on the __mptr temp
variable name. With that removed, we need to check via a struct walk or
a void * cast (which is what Clang already does).

I'm hoping to get these into v7.3 before release, but I want to put this
through -next for a bit first.

-Kees

Kees Cook (2):
  randstruct: fix container_of() false positives after __mptr removal
  randstruct: report bad casts as warnings rather than notes

 scripts/gcc-plugins/randomize_layout_plugin.c | 63 +++++++++++++++++--
 1 file changed, 59 insertions(+), 4 deletions(-)

-- 
2.34.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/2] randstruct: fix container_of() false positives after __mptr removal
  2026-09-03 23:24 [PATCH 0/2] randstruct: fix container_of() false positives after __mptr removal Kees Cook
@ 2026-09-03 23:24 ` Kees Cook
  2026-09-03 23:24 ` [PATCH 2/2] randstruct: report bad casts as warnings rather than notes Kees Cook
  1 sibling, 0 replies; 6+ messages in thread
From: Kees Cook @ 2026-09-03 23:24 UTC (permalink / raw)
  To: Bill Wendling
  Cc: Kees Cook, Vincent Mailhol, Nathan Chancellor, Nick Desaulniers,
	Justin Stitt, Greg Kroah-Hartman, linux-kernel, linux-hardening

Commit f9e7a7564834 ("container_of: remove local __mptr variable") dropped
the "void *__mptr" temporary from container_of(). The randstruct GCC
plugin's find_bad_casts pass recognized the casts container_of() generates
by that variable's name:

	const_tree ssa_name_var = SSA_NAME_VAR(rhs1);
	/* skip bogus type casts introduced by container_of */
	if (ssa_name_var != NULL_TREE && DECL_NAME(ssa_name_var) &&
	    !strcmp(DECL_NAME_POINTER(ssa_name_var), "__mptr"))
		continue;

With the variable gone the suppression never fires, so every
container_of() whose container type is randomized now emits a note:

  include/linux/container_of.h:23:9: note: randstruct: casting between
    randomized structure pointer types (ssa): 'struct ocfs2_triggers' and
    'struct jbd2_buffer_trigger_type'
  fs/ocfs2/journal.c:524:16: note: in expansion of macro 'container_of'

The pass runs on GIMPLE, after folding, and for a member at offset 0 the
whole expression collapses to a bare copy that is indistinguishable from
an unsafe cast:

  to_ocfs2_trigger (struct jbd2_buffer_trigger_type * triggers)
  {
    _2 = triggers_1(D);   /* void * cast and subtraction gone */
    return _2;
  }

Match the type relationship instead. A cast from A * to B * is a
container_of() if B reaches a field of type A at offset 0 through a chain
of by-value members. The chain matters: container_of()'s member argument
may be a dotted path, as in ceph_inode(), which is container_of(inode,
struct ceph_inode_info, netfs.inode) and needs two levels. The search
is depth-bounded to 4 just in case, since real paths are generally one
or two members deep.

Requiring the cast happens at offset 0 is done because any other member
offset the subtraction survives folding and the cast's rhs is still
void *, which the pass already skips a few lines above.

A cast between two randomized types with no containment relationship is
still reported. Verified with:

	struct cred *f(struct file *f) { return (struct cred *)f; }

which is still flagged with the patch applied.

Clang's implementation is unaffected. It checks the cast as written, and
both the old and new macros cast from void *, which is always permitted;
a genuinely bad cast is rejected there as a hard error rather than a note.

Build tested ARCH=x86_64 defconfig with CONFIG_RANDSTRUCT_FULL=y and GCC
14.2.0: randstruct notes 52 before, 0 after.

Fixes: f9e7a7564834 ("container_of: remove local __mptr variable")
Assisted-by: Claude:claude-opus-5[1m]
Signed-off-by: Kees Cook <kees@kernel.org>
---
 scripts/gcc-plugins/randomize_layout_plugin.c | 61 ++++++++++++++++++-
 1 file changed, 58 insertions(+), 3 deletions(-)

diff --git a/scripts/gcc-plugins/randomize_layout_plugin.c b/scripts/gcc-plugins/randomize_layout_plugin.c
index ff65a4f87f24..e2bd9ba08089 100644
--- a/scripts/gcc-plugins/randomize_layout_plugin.c
+++ b/scripts/gcc-plugins/randomize_layout_plugin.c
@@ -698,6 +698,63 @@ static void handle_local_var_initializers(void)
 	}
 }
 
+/*
+ * Does @container reach a field of type @member_type by a chain of
+ * by-value members? That is the relationship container_of() expresses --
+ * its @member argument may be a dotted path, e.g.
+ * container_of(inode, struct ceph_inode_info, netfs.inode) -- so a cast
+ * from @member_type * to @container * is legitimate rather than a
+ * layout-confusing one.
+ *
+ * container_of() used to leave a "void *__mptr" temporary behind, and this
+ * pass recognised such casts by that name. Commit f9e7a7564834
+ * ("container_of: remove local __mptr variable") removed it to stop nested
+ * container_of() shadowing itself, and the cast now folds to a bare SSA
+ * copy when the member sits at offset 0, leaving nothing syntactic to key
+ * on. Match the type relationship instead.
+ *
+ * The depth bound keeps this cheap; container_of() paths are one or two
+ * members deep in practice.
+ */
+#define CONTAINER_OF_MAX_DEPTH 4
+
+static bool is_container_of_cast(const_tree container, const_tree member_type,
+				 int depth)
+{
+	const_tree field;
+
+	if (container == NULL_TREE || depth > CONTAINER_OF_MAX_DEPTH)
+		return false;
+
+	if (TREE_CODE(container) != RECORD_TYPE &&
+	    TREE_CODE(container) != UNION_TYPE)
+		return false;
+
+	for (field = TYPE_FIELDS(container); field; field = DECL_CHAIN(field)) {
+		const_tree field_type;
+
+		if (TREE_CODE(field) != FIELD_DECL)
+			continue;
+
+		/*
+		 * Only a member at offset 0 can reach here: for any other
+		 * offset container_of()'s subtraction survives folding, the
+		 * cast's rhs stays void *, and the caller skipped it above.
+		 */
+		if (!integer_zerop(byte_position(field)))
+			continue;
+
+		field_type = TYPE_MAIN_VARIANT(TREE_TYPE(field));
+		if (field_type == member_type)
+			return true;
+
+		if (is_container_of_cast(field_type, member_type, depth + 1))
+			return true;
+	}
+
+	return false;
+}
+
 /*
  * iterate over all statements to find "bad" casts:
  * those where the address of the start of a structure is cast
@@ -799,10 +856,8 @@ static unsigned int find_bad_casts_execute(void)
 #endif
 				MISMATCH(gimple_location(stmt), "op0", ptr_lhs_type, op0_type);
 			} else {
-				const_tree ssa_name_var = SSA_NAME_VAR(rhs1);
 				/* skip bogus type casts introduced by container_of */
-				if (ssa_name_var != NULL_TREE && DECL_NAME(ssa_name_var) && 
-				    !strcmp((const char *)DECL_NAME_POINTER(ssa_name_var), "__mptr"))
+				if (is_container_of_cast(ptr_lhs_type, ptr_rhs_type, 0))
 					continue;
 #ifndef __DEBUG_PLUGIN
 				if (lookup_attribute("randomize_performed", TYPE_ATTRIBUTES(ptr_rhs_type)))
-- 
2.34.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 2/2] randstruct: report bad casts as warnings rather than notes
  2026-09-03 23:24 [PATCH 0/2] randstruct: fix container_of() false positives after __mptr removal Kees Cook
  2026-09-03 23:24 ` [PATCH 1/2] " Kees Cook
@ 2026-09-03 23:24 ` Kees Cook
  2026-09-04 18:14   ` Bill Wendling
  1 sibling, 1 reply; 6+ messages in thread
From: Kees Cook @ 2026-09-03 23:24 UTC (permalink / raw)
  To: Bill Wendling
  Cc: Kees Cook, Vincent Mailhol, Nathan Chancellor, Nick Desaulniers,
	Justin Stitt, Greg Kroah-Hartman, linux-kernel, linux-hardening

find_bad_casts() reports a cast between two randomized structure pointer
types with inform(), which renders as a "note:". It has done so since
the plugin was originally added, while using error() freely for attribute
misuse, UAPI structs, and version mismatches.

Clang's implementation of the same check has always been stricter:
it rejects such a cast as a full error. There is no reason for the GCC
side to be effectively silent about the same problem.

Build tested ARCH=x86_64 with CONFIG_RANDSTRUCT_FULL=y and GCC 14.2.0:
allmodconfig clean, and defconfig clean under three different random
seeds. A deliberate bad cast is still reported, now as a warning, at the
correct line and column.

Signed-off-by: Kees Cook <kees@kernel.org>
---
 scripts/gcc-plugins/randomize_layout_plugin.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/gcc-plugins/randomize_layout_plugin.c b/scripts/gcc-plugins/randomize_layout_plugin.c
index e2bd9ba08089..1e66f45fe29b 100644
--- a/scripts/gcc-plugins/randomize_layout_plugin.c
+++ b/scripts/gcc-plugins/randomize_layout_plugin.c
@@ -22,7 +22,7 @@
 #define ORIG_TYPE_NAME(node) \
 	(TYPE_NAME(TYPE_MAIN_VARIANT(node)) != NULL_TREE ? ((const unsigned char *)IDENTIFIER_POINTER(TYPE_NAME(TYPE_MAIN_VARIANT(node)))) : (const unsigned char *)"anonymous")
 
-#define INFORM(loc, msg, ...)	inform(loc, "randstruct: " msg, ##__VA_ARGS__)
+#define INFORM(loc, msg, ...)	warning_at(loc, 0, "randstruct: " msg, ##__VA_ARGS__)
 #define MISMATCH(loc, how, ...)	INFORM(loc, "casting between randomized structure pointer types (" how "): %qT and %qT\n", __VA_ARGS__)
 
 __visible int plugin_is_GPL_compatible;
-- 
2.34.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] randstruct: report bad casts as warnings rather than notes
  2026-09-03 23:24 ` [PATCH 2/2] randstruct: report bad casts as warnings rather than notes Kees Cook
@ 2026-09-04 18:14   ` Bill Wendling
  2026-09-04 20:24     ` Nathan Chancellor
  2026-09-05  0:48     ` Kees Cook
  0 siblings, 2 replies; 6+ messages in thread
From: Bill Wendling @ 2026-09-04 18:14 UTC (permalink / raw)
  To: Kees Cook
  Cc: Vincent Mailhol, Nathan Chancellor, Nick Desaulniers,
	Justin Stitt, Greg Kroah-Hartman, linux-kernel, linux-hardening

Hi Kees,

On Thu, Sep 3, 2026 at 4:24 PM Kees Cook <kees@kernel.org> wrote:
>
> find_bad_casts() reports a cast between two randomized structure pointer
> types with inform(), which renders as a "note:". It has done so since
> the plugin was originally added, while using error() freely for attribute
> misuse, UAPI structs, and version mismatches.
>
> Clang's implementation of the same check has always been stricter:
> it rejects such a cast as a full error. There is no reason for the GCC
> side to be effectively silent about the same problem.
>
> Build tested ARCH=x86_64 with CONFIG_RANDSTRUCT_FULL=y and GCC 14.2.0:
> allmodconfig clean, and defconfig clean under three different random
> seeds. A deliberate bad cast is still reported, now as a warning, at the
> correct line and column.
>
> Signed-off-by: Kees Cook <kees@kernel.org>
> ---
>  scripts/gcc-plugins/randomize_layout_plugin.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/scripts/gcc-plugins/randomize_layout_plugin.c b/scripts/gcc-plugins/randomize_layout_plugin.c
> index e2bd9ba08089..1e66f45fe29b 100644
> --- a/scripts/gcc-plugins/randomize_layout_plugin.c
> +++ b/scripts/gcc-plugins/randomize_layout_plugin.c
> @@ -22,7 +22,7 @@
>  #define ORIG_TYPE_NAME(node) \
>         (TYPE_NAME(TYPE_MAIN_VARIANT(node)) != NULL_TREE ? ((const unsigned char *)IDENTIFIER_POINTER(TYPE_NAME(TYPE_MAIN_VARIANT(node)))) : (const unsigned char *)"anonymous")
>
> -#define INFORM(loc, msg, ...)  inform(loc, "randstruct: " msg, ##__VA_ARGS__)
> +#define INFORM(loc, msg, ...)  warning_at(loc, 0, "randstruct: " msg, ##__VA_ARGS__)

Why not make this an error instead of a warning?

-bw

>  #define MISMATCH(loc, how, ...)        INFORM(loc, "casting between randomized structure pointer types (" how "): %qT and %qT\n", __VA_ARGS__)
>
>  __visible int plugin_is_GPL_compatible;
> --
> 2.34.1
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] randstruct: report bad casts as warnings rather than notes
  2026-09-04 18:14   ` Bill Wendling
@ 2026-09-04 20:24     ` Nathan Chancellor
  2026-09-05  0:48     ` Kees Cook
  1 sibling, 0 replies; 6+ messages in thread
From: Nathan Chancellor @ 2026-09-04 20:24 UTC (permalink / raw)
  To: Bill Wendling
  Cc: Kees Cook, Vincent Mailhol, Nick Desaulniers, Justin Stitt,
	Greg Kroah-Hartman, linux-kernel, linux-hardening

On Fri, Sep 04, 2026 at 11:14:19AM -0700, Bill Wendling wrote:
> Hi Kees,
> 
> On Thu, Sep 3, 2026 at 4:24 PM Kees Cook <kees@kernel.org> wrote:
> >
> > find_bad_casts() reports a cast between two randomized structure pointer
> > types with inform(), which renders as a "note:". It has done so since
> > the plugin was originally added, while using error() freely for attribute
> > misuse, UAPI structs, and version mismatches.
> >
> > Clang's implementation of the same check has always been stricter:
> > it rejects such a cast as a full error. There is no reason for the GCC
> > side to be effectively silent about the same problem.
> >
> > Build tested ARCH=x86_64 with CONFIG_RANDSTRUCT_FULL=y and GCC 14.2.0:
> > allmodconfig clean, and defconfig clean under three different random
> > seeds. A deliberate bad cast is still reported, now as a warning, at the
> > correct line and column.
> >
> > Signed-off-by: Kees Cook <kees@kernel.org>
> > ---
> >  scripts/gcc-plugins/randomize_layout_plugin.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/scripts/gcc-plugins/randomize_layout_plugin.c b/scripts/gcc-plugins/randomize_layout_plugin.c
> > index e2bd9ba08089..1e66f45fe29b 100644
> > --- a/scripts/gcc-plugins/randomize_layout_plugin.c
> > +++ b/scripts/gcc-plugins/randomize_layout_plugin.c
> > @@ -22,7 +22,7 @@
> >  #define ORIG_TYPE_NAME(node) \
> >         (TYPE_NAME(TYPE_MAIN_VARIANT(node)) != NULL_TREE ? ((const unsigned char *)IDENTIFIER_POINTER(TYPE_NAME(TYPE_MAIN_VARIANT(node)))) : (const unsigned char *)"anonymous")
> >
> > -#define INFORM(loc, msg, ...)  inform(loc, "randstruct: " msg, ##__VA_ARGS__)
> > +#define INFORM(loc, msg, ...)  warning_at(loc, 0, "randstruct: " msg, ##__VA_ARGS__)
> 
> Why not make this an error instead of a warning?

Yeah, it seems odd to mention that Clang's implementation has always
treated this as an error then not match its behavior. If this cannot be
made an error, I think the commit message should call out why.

-- 
Cheers,
Nathan

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] randstruct: report bad casts as warnings rather than notes
  2026-09-04 18:14   ` Bill Wendling
  2026-09-04 20:24     ` Nathan Chancellor
@ 2026-09-05  0:48     ` Kees Cook
  1 sibling, 0 replies; 6+ messages in thread
From: Kees Cook @ 2026-09-05  0:48 UTC (permalink / raw)
  To: Bill Wendling
  Cc: Vincent Mailhol, Nathan Chancellor, Nick Desaulniers,
	Justin Stitt, Greg Kroah-Hartman, linux-kernel, linux-hardening

On Fri, Sep 04, 2026 at 11:14:19AM -0700, Bill Wendling wrote:
> Why not make this an error instead of a warning?

I guess I was thinking of moving from "invisible note" to "visible
warning", but I'd agree: better to just match Clang on this. I'll send a
v2.

-- 
Kees Cook

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-09-05  0:48 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-03 23:24 [PATCH 0/2] randstruct: fix container_of() false positives after __mptr removal Kees Cook
2026-09-03 23:24 ` [PATCH 1/2] " Kees Cook
2026-09-03 23:24 ` [PATCH 2/2] randstruct: report bad casts as warnings rather than notes Kees Cook
2026-09-04 18:14   ` Bill Wendling
2026-09-04 20:24     ` Nathan Chancellor
2026-09-05  0:48     ` Kees Cook

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®