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

[Julia Lawall]

On Mon, 2 Nov 2026, Ella Ma wrote:

> On Mon, Nov 02, 2026 at 12:46:11AM +0100, Julia Lawall wrote:
> >
> >
> > 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.
>
> OK. Thank you for the change.
>
> >
> > 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).
>
> A patch fixing it has been reviewed by Thomas Huth <thuth@xxxxxxxxxx> in
> https://lore.kernel.org/lkml/ffebbb7b-a4f9-488e-8834-199da3073b96@xxxxxxxxxx/
> Should I reply something to the thread about this new coccinelle report?

No need, I was just wondering.

julia

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