* [PATCH] coccinelle: free: add a checker for `__cleanup(kfree)` usage @ 2026-08-31 13:43 Ella Ma 2026-08-31 15:01 ` [cocci] " Markus Elfring 2026-09-24 16:25 ` [PATCH v2] " Ella Ma 0 siblings, 2 replies; 10+ messages in thread From: Ella Ma @ 2026-08-31 13:43 UTC (permalink / raw) To: Julia.Lawall, nicolas.palix; +Cc: alansnape3058, linux-kernel, cocci 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@gmail.com> --- scripts/coccinelle/free/cleanup-free.cocci | 94 ++++++++++++++++++++++ 1 file changed, 94 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..cdf984ac6b44 --- /dev/null +++ b/scripts/coccinelle/free/cleanup-free.cocci @@ -0,0 +1,94 @@ +// 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 + + +// 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; +type T; +identifier v, n; +position p; +@@ + +( + T v __cleanup@p( +( + n +& +( + kfree \| kfree_sensitive \| kvfree \| kvfree_atomic +) +) + ); +| + T v __cleanup@p( +( + n +& +( + kfree \| kfree_sensitive \| kvfree \| kvfree_atomic +) +) + ) = ...; +) + +@script:python depends on free@ +p << free.p; +n << free.n; +@@ + +coccilib.report.print_report( + p[0], f"ERROR: found `__cleanup({n})', do you mean `__free({n})'?") + + +// 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 kvfree_sensitive, vfree, vfree_atomic; +type T; +identifier v, n; +position p; +@@ + +( + T v __cleanup@p( +( + n +& +( + kvfree_sensitive \| vfree \| vfree_atomic +) +) + ); +| + T v __cleanup@p( +( + n +& +( + kvfree_sensitive \| vfree \| vfree_atomic +) +) + ) = ...; +) + +@script:python depends on cleanup@ +p << cleanup.p; +n << cleanup.n; +@@ + +coccilib.report.print_report( + p[0], f"ERROR: `__cleanup({n})' will free the annotated variable, rather than its pointee") -- 2.34.1 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [cocci] [PATCH] coccinelle: free: add a checker for `__cleanup(kfree)` usage 2026-08-31 13:43 [PATCH] coccinelle: free: add a checker for `__cleanup(kfree)` usage Ella Ma @ 2026-08-31 15:01 ` Markus Elfring 2026-09-24 16:25 ` [PATCH v2] " Ella Ma 1 sibling, 0 replies; 10+ messages in thread From: Markus Elfring @ 2026-08-31 15:01 UTC (permalink / raw) To: Ella Ma, cocci, Julia Lawall, Nicolas Palix; +Cc: LKML, kernel-janitors > Using __cleanup(kfree) will pass the stack address of the annotated > local variable to kfree functions. I got the impression that this wording approach is improvable. This attribute probably needs a pointer to an object which was dynamically allocated. How do you think about to handle an attribute parameter? Would you like to support operation modes for coccicheck scripts in a consistent way? > This will lead to invalid > deallocation issues. Inspired by CVE-2026-45959 and similar bugs > recently detected. Thanks for such background information. … > +++ b/scripts/coccinelle/free/cleanup-free.cocci > @@ -0,0 +1,94 @@ … > +/// Find __cleanup(kfree) > +/// Using __cleanup(kfree) will pass the stack address of the annotated > +/// local variable to kfree, causing an invalid free. Would an other wording variant become more helpful? > +/// I.e., `T v __cleanup(kfree);` --> `kfree(&v);` > +/// Such usage is impossible to be correct. … > +// 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; > +type T; > +identifier v, n; > +position p; > +@@ > + > +( > + T v __cleanup@p( > +( > + n > +& > +( > + kfree \| kfree_sensitive \| kvfree \| kvfree_atomic > +) How do you think about to specify relevant function names on separate lines for such an SmPL conjunction? Can symbol lists be converted into corresponding case distinctions? Regards, Markus ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2] coccinelle: free: add a checker for `__cleanup(kfree)` usage 2026-08-31 13:43 [PATCH] coccinelle: free: add a checker for `__cleanup(kfree)` usage Ella Ma 2026-08-31 15:01 ` [cocci] " Markus Elfring @ 2026-09-24 16:25 ` Ella Ma 2026-09-27 9:43 ` Julia Lawall 1 sibling, 1 reply; 10+ messages in thread From: Ella Ma @ 2026-09-24 16:25 UTC (permalink / raw) To: Julia.Lawall, nicolas.palix; +Cc: alansnape3058, linux-kernel, cocci 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@gmail.com> --- 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` Currently, the five functions in the `free` rule are all the functions of `void (*)(const? void *)` type with a `DEFINE_FREE` definition in the code base. So I adopted all of them. Function `kfree_const` is added as it is of the kfree family and used more frequently than other kfree-family functions (even more frequently than some of those having a `DEFINE_FREE` definition). https://elixir.bootlin.com/linux/v7.3-rc3/A/ident/kfree_const (found in 64 files in v7.3-rc3). This cocci script can report the bug fixed in https://lore.kernel.org/lkml/20260617142632.3298984-1-vulab@iscas.ac.cn/ 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..63216ec5d865 --- /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; +@@ + +coccilib.report.print_report( + p[0], f"ERROR: found `__cleanup({n})', do you mean `__free({n})'?") + +@script:python depends on org@ +p << free.p; +n << free.n; +@@ + +coccilib.org.print_todo( + p[0], f"ERROR: found `__cleanup({n})', do you mean `__free({n})'?") + + +// 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; +@@ + +coccilib.report.print_report( + p[0], f"ERROR: `__cleanup({n})' will free the annotated variable, rather than its pointee") + +@script:python depends on org@ +p << cleanup.p; +n << cleanup.n; +@@ + +coccilib.org.print_todo( + p[0], f"ERROR: `__cleanup({n})' will free the annotated variable, rather than its pointee") -- 2.34.1 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] coccinelle: free: add a checker for `__cleanup(kfree)` usage 2026-09-24 16:25 ` [PATCH v2] " Ella Ma @ 2026-09-27 9:43 ` Julia Lawall 2026-10-07 16:43 ` [PATCH v3] " Ella Ma 0 siblings, 1 reply; 10+ messages in thread From: Julia Lawall @ 2026-09-27 9:43 UTC (permalink / raw) To: Ella Ma; +Cc: nicolas.palix, linux-kernel, cocci On Thu, 24 Sep 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. Thanks for the semantic patch. I think it would be good for the error reports to be more explicit about what the issue is, ie the relationship to the presence of absence of DEFINE_FREE. Otherwise, it is hard to completely understand the problem without looking at the comments in the semantic patch. julia > > Signed-off-by: Ella Ma <alansnape3058@gmail.com> > --- > > 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` > > Currently, the five functions in the `free` rule are all the functions > of `void (*)(const? void *)` type with a `DEFINE_FREE` definition in the > code base. So I adopted all of them. > Function `kfree_const` is added as it is of the kfree family and used > more frequently than other kfree-family functions (even more frequently > than some of those having a `DEFINE_FREE` definition). > https://elixir.bootlin.com/linux/v7.3-rc3/A/ident/kfree_const > (found in 64 files in v7.3-rc3). > > This cocci script can report the bug fixed in > https://lore.kernel.org/lkml/20260617142632.3298984-1-vulab@iscas.ac.cn/ > > 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..63216ec5d865 > --- /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; > +@@ > + > +coccilib.report.print_report( > + p[0], f"ERROR: found `__cleanup({n})', do you mean `__free({n})'?") > + > +@script:python depends on org@ > +p << free.p; > +n << free.n; > +@@ > + > +coccilib.org.print_todo( > + p[0], f"ERROR: found `__cleanup({n})', do you mean `__free({n})'?") > + > + > +// 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; > +@@ > + > +coccilib.report.print_report( > + p[0], f"ERROR: `__cleanup({n})' will free the annotated variable, rather than its pointee") > + > +@script:python depends on org@ > +p << cleanup.p; > +n << cleanup.n; > +@@ > + > +coccilib.org.print_todo( > + p[0], f"ERROR: `__cleanup({n})' will free the annotated variable, rather than its pointee") > -- > 2.34.1 > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v3] coccinelle: free: add a checker for `__cleanup(kfree)` usage 2026-09-27 9:43 ` Julia Lawall @ 2026-10-07 16:43 ` Ella Ma 2026-10-07 18:27 ` Julia Lawall 0 siblings, 1 reply; 10+ messages in thread From: Ella Ma @ 2026-10-07 16:43 UTC (permalink / raw) To: julia.lawall; +Cc: alansnape3058, cocci, linux-kernel, nicolas.palix 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@gmail.com> --- 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) + +@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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3] coccinelle: free: add a checker for `__cleanup(kfree)` usage 2026-10-07 16:43 ` [PATCH v3] " Ella Ma @ 2026-10-07 18:27 ` Julia Lawall 2026-10-16 13:57 ` Ella Ma 0 siblings, 1 reply; 10+ messages in thread From: Julia Lawall @ 2026-10-07 18:27 UTC (permalink / raw) To: Ella Ma; +Cc: cocci, linux-kernel, nicolas.palix 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@gmail.com> > --- > > 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" 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 > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3] coccinelle: free: add a checker for `__cleanup(kfree)` usage 2026-10-07 18:27 ` Julia Lawall @ 2026-10-16 13:57 ` Ella Ma 2026-11-01 23:46 ` Julia Lawall 0 siblings, 1 reply; 10+ messages in thread From: Ella Ma @ 2026-10-16 13:57 UTC (permalink / raw) To: Julia Lawall; +Cc: cocci, linux-kernel, nicolas.palix 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@gmail.com> > > --- > > > > 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 > > > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3] coccinelle: free: add a checker for `__cleanup(kfree)` usage 2026-10-16 13:57 ` Ella Ma @ 2026-11-01 23:46 ` Julia Lawall 2026-11-02 9:01 ` Ella Ma 0 siblings, 1 reply; 10+ messages in thread From: Julia Lawall @ 2026-11-01 23:46 UTC (permalink / raw) To: Ella Ma; +Cc: cocci, linux-kernel, nicolas.palix 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@gmail.com> > > > --- > > > > > > 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 > > > > > > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3] coccinelle: free: add a checker for `__cleanup(kfree)` usage 2026-11-01 23:46 ` Julia Lawall @ 2026-11-02 9:01 ` Ella Ma 2026-11-02 9:08 ` Julia Lawall 0 siblings, 1 reply; 10+ messages in thread From: Ella Ma @ 2026-11-02 9:01 UTC (permalink / raw) To: Julia Lawall; +Cc: alansnape3058, cocci, linux-kernel, nicolas.palix 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@gmail.com> > > > > --- > > > > > > > > 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@redhat.com> in https://lore.kernel.org/lkml/ffebbb7b-a4f9-488e-8834-199da3073b96@redhat.com/ Should I reply something to the thread about this new coccinelle report? 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 > > > > > > > > > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3] coccinelle: free: add a checker for `__cleanup(kfree)` usage 2026-11-02 9:01 ` Ella Ma @ 2026-11-02 9:08 ` Julia Lawall 0 siblings, 0 replies; 10+ messages in thread From: Julia Lawall @ 2026-11-02 9:08 UTC (permalink / raw) To: Ella Ma; +Cc: cocci, linux-kernel, nicolas.palix 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@gmail.com> > > > > > --- > > > > > > > > > > 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@redhat.com> in > https://lore.kernel.org/lkml/ffebbb7b-a4f9-488e-8834-199da3073b96@redhat.com/ > 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 > > > > > > > > > > > > > > ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2026-11-02 9:09 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2026-08-31 13:43 [PATCH] coccinelle: free: add a checker for `__cleanup(kfree)` usage Ella Ma 2026-08-31 15:01 ` [cocci] " Markus Elfring 2026-09-24 16:25 ` [PATCH v2] " Ella Ma 2026-09-27 9:43 ` Julia Lawall 2026-10-07 16:43 ` [PATCH v3] " Ella Ma 2026-10-07 18:27 ` Julia Lawall 2026-10-16 13:57 ` Ella Ma 2026-11-01 23:46 ` Julia Lawall 2026-11-02 9:01 ` Ella Ma 2026-11-02 9:08 ` Julia Lawall
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®