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

[Julia Lawall]

On Fri, 16 Oct 2026, Ella Ma wrote:

> 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

I applied this, but I changed the first error message to:

"ERROR: __cleanup({n}) will cause an invalid free: the stack address of
the annotated variable will be passed to {n}; declare the variable using
__free({n}) to cause the pointee to be freed"

The original message made it look like one should use __free when calling
kfree.

Furthermore, there is one report in the kernel for this case.  It would be
worth fixing it or checking whether it is dead code (it's immediately
under an ifdef).

julia


> > >
> > >  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
> > >
> > >
>