Re: [PATCH v3] coccinelle: free: add a checker for `__cleanup(kfree)` usage

[Ella Ma]

On Wed, Oct 07, 2026 at 08:27:33PM +0200, Julia Lawall wrote:
> 
> 
> On Wed, 7 Oct 2026, Ella Ma wrote:
> 
> > Using __cleanup(kfree) will pass the stack address of the annotated
> > local variable to kfree functions. This will lead to invalid
> > deallocation issues. Inspired by CVE-2026-45959 and similar bugs
> > recently detected.
> >
> > Signed-off-by: Ella Ma <alansnape3058@xxxxxxxxx>
> > ---
> >
> > Changes in v2:
> > - Add virtual rules `report` and `org` as suggested by Sashiko
> > - Remove `kvfree_sensitive` from rule `cleanup` as suggested by Sashiko
> > - Add `free_percpu` to rule `free`
> > - Add `kfree_const` to rule `cleanup`
> >
> > Changes in v3:
> > - Edit the error messages to clarify the reasons and results
> >
> >  scripts/coccinelle/free/cleanup-free.cocci | 112 +++++++++++++++++++++
> >  1 file changed, 112 insertions(+)
> >  create mode 100644 scripts/coccinelle/free/cleanup-free.cocci
> >
> > diff --git a/scripts/coccinelle/free/cleanup-free.cocci b/scripts/coccinelle/free/cleanup-free.cocci
> > new file mode 100644
> > index 000000000000..48e47ed2186a
> > --- /dev/null
> > +++ b/scripts/coccinelle/free/cleanup-free.cocci
> > @@ -0,0 +1,112 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +///
> > +/// Find __cleanup(kfree)
> > +/// Using __cleanup(kfree) will pass the stack address of the annotated
> > +/// local variable to kfree, causing an invalid free.
> > +/// I.e., `T v __cleanup(kfree);` --> `kfree(&v);`
> > +/// Such usage is impossible to be correct.
> > +///
> > +// Confidence: High
> > +// Copyright: (C) 2026 Ella Ma
> > +// Options: --no-includes --include-headers
> > +
> > +virtual report
> > +virtual org
> > +
> > +// Suggesting using __free for the functions with a DEFINE_FREE definition.
> > +// Update this list when new DEFINE_FREE definitions are added.
> > +@free@
> > +attribute name __cleanup;
> > +symbol kfree, kfree_sensitive, kvfree, kvfree_atomic, free_percpu;
> > +type T;
> > +identifier v, n;
> > +position p;
> > +@@
> > +
> > +(
> > +  T v __cleanup@p(
> > +(
> > +  n
> > +&
> > +(
> > +  kfree \| kfree_sensitive \| kvfree \| kvfree_atomic \| free_percpu
> > +)
> > +)
> > +  );
> > +|
> > +  T v __cleanup@p(
> > +(
> > +  n
> > +&
> > +(
> > +  kfree \| kfree_sensitive \| kvfree \| kvfree_atomic \| free_percpu
> > +)
> > +)
> > +  ) = ...;
> > +)
> > +
> > +@script:python depends on report@
> > +p << free.p;
> > +n << free.n;
> > +@@
> > +
> > +msg = f"ERROR: __cleanup({n}) will cause an invalid free: the stack address of the annotated variable will be passed to {n}; use __free({n}) to free the pointee"
> > +coccilib.report.print_report(p[0], msg)
> > +
> > +@script:python depends on org@
> > +p << free.p;
> > +n << free.n;
> > +@@
> > +
> > +msg = f"ERROR: __cleanup({n}) will cause an invalid free: the stack address of the annotated variable will be passed to {n}; use __free({n}) to free the pointee"
> > +coccilib.org.print_todo(p[0], msg)
> > +
> > +
> > +// Reporting __cleanup usage for the functions without a DEFINE_FREE definition.
> > +// The following list only contains frequently used functions. Supplement it if
> > +// necessary.
> > +@cleanup@
> > +attribute name __cleanup;
> > +symbol vfree, vfree_atomic, kfree_const;
> > +type T;
> > +identifier v, n;
> > +position p;
> > +@@
> > +
> > +(
> > +  T v __cleanup@p(
> > +(
> > +  n
> > +&
> > +(
> > +  vfree \| vfree_atomic \| kfree_const
> > +)
> > +)
> > +  );
> > +|
> > +  T v __cleanup@p(
> > +(
> > +  n
> > +&
> > +(
> > +  vfree \| vfree_atomic \| kfree_const
> > +)
> > +)
> > +  ) = ...;
> > +)
> > +
> > +@script:python depends on report@
> > +p << cleanup.p;
> > +n << cleanup.n;
> > +@@
> > +
> > +msg = f"ERROR: __cleanup({n}) will cause an invalid free: the stack address of the annotated variable will be passed to {n}"
> > +coccilib.report.print_report(p[0], msg)
> 
> Hello, this is more words, but is not what I was hoping for.  You explain
> the effect, but not the cause.  And to fix the bug with confidence the
> reader wants to know the cause.  Here I would have expected something like
> "__cleanup usage for a function without a DEFINE_FREE definition. This will cause an invalid free"

Do you mean only the second rule needs a refinement, or both rules?

==> __cleanup vs __free, both rules:

Using DEFINE_FREE declarer only defines a forwarding function that can
be annotated to a local variable via __free attribute. Defining such a
function with DEFINE_FREE will not automatically make __cleanup correct.
User still need to use __free instead.

FYI: __cleanup will pass the stack address to the annotated function
directly; while __free will first call the forwarding function defined
with DEFINE_FREE, and then the forwarding function will correctly pass
the pointee address to the annotated function.

==> Only the second rule:

Yes. For the functions without a forwarding function definition, one of
the root cause is lacking the definition. But I am not sure whether this
semantic patch should suggest adding one.  As these free functions are
all those frequently used, perhaps the DEFINE_FREE definition should be
added to the corresponding public header files.  Or, do you mean this
semantic rule only needs to report this missing, and the users will add
them where they think is good?

Besides, after adding a definition, the current error message will
become misleading, as there are no rules to search for existing
DEFINE_FREE definitions in the code base. Do you think it is necessary
for this semantic patch to search for the corresponding DEFINE_FREE
definitions first and then generate the error message accordingly?

I.e.,
with DEFINE_FREE:
requiring replacing __cleanup with __free.
without DEFINE_FREE:
reporting missing DEFINE_FREE and also requiring replacing __cleanup
with __free.

ella

> 
> julia
> 
> 
> > +
> > +@script:python depends on org@
> > +p << cleanup.p;
> > +n << cleanup.n;
> > +@@
> > +
> > +msg = f"ERROR: __cleanup({n}) will cause an invalid free: the stack address of the annotated variable will be passed to {n}"
> > +coccilib.org.print_todo(p[0], msg)
> > --
> > 2.34.1
> >
> >