Re: [Cocci] [RESEND] coccinelle: api: rename kzfree to kfree_sensitive
Hi,
On 8/5/21 12:58 PM, Weizhao Ouyang wrote:
> Commit 453431a54934 ("mm, treewide: rename kzfree() to
> kfree_sensitive()") renamed kzfree() to kfree_sensitive(),
> it should be applied to coccinelle.
>
> Signed-off-by: Weizhao Ouyang
Acked-by: Denis Efremov
> ---
> scripts/coccinelle/api/kvmalloc.cocci | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/scripts/coccinelle/api/kvmalloc.cocci
> b/scripts/coccinelle/api/kvmalloc.cocci
> index c30dab718a49..5ddcb76b76b0 100644
> --- a/scripts/coccinelle/api/kvmalloc.cocci
> +++ b/scripts/coccinelle/api/kvmalloc.cocci
> @@ -79,7 +79,7 @@ position p : script:python() { relevant(p) };
>} else {
> ... when != krealloc(E, ...)
> when any
> -* \(kfree\|kzfree\)(E)
> +* \(kfree\|kfree_sensitive\)(E)
> ...
>}
>
>
Regards,
Denis
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [RESEND PATCH] coccinelle: misc: minmax: suppress patch generation for err returns
Ping?
On 4/28/21 9:03 AM, Denis Efremov wrote:
> There is a standard idiom for "if 'ret' holds an error, return it":
> return ret < 0 ? ret : 0;
>
> Developers prefer to keep the things as they are because stylistic
> change to "return min(ret, 0);" breaks readability.
>
> Let's suppress automatic generation for this type of patches.
>
> Signed-off-by: Denis Efremov
> ---
> scripts/coccinelle/misc/minmax.cocci | 18 +-
> 1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/scripts/coccinelle/misc/minmax.cocci
> b/scripts/coccinelle/misc/minmax.cocci
> index eccdd3eb3452..fcf908b34f27 100644
> --- a/scripts/coccinelle/misc/minmax.cocci
> +++ b/scripts/coccinelle/misc/minmax.cocci
> @@ -116,16 +116,32 @@ func(...)
> ...>
> }
>
> +// Don't generate patches for errcode returns.
> +@errcode depends on patch@
> +position p;
> +identifier func;
> +expression x;
> +binary operator cmp = {<, <=};
> +@@
> +
> +func(...)
> +{
> + <...
> + return ((x) cmp@p 0 ? (x) : 0);
> + ...>
> +}
> +
> @pmin depends on patch@
> identifier func;
> expression x, y;
> binary operator cmp = {<=, <};
> +position p != errcode.p;
> @@
>
> func(...)
> {
> <...
> --((x) cmp (y) ? (x) : (y))
> +-((x) cmp@p (y) ? (x) : (y))
> +min(x, y)
> ...>
> }
>
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [RESEND PATCH] coccinelle: misc: minmax: suppress patch generation for err returns
There is a standard idiom for "if 'ret' holds an error, return it":
return ret < 0 ? ret : 0;
Developers prefer to keep the things as they are because stylistic
change to "return min(ret, 0);" breaks readability.
Let's suppress automatic generation for this type of patches.
Signed-off-by: Denis Efremov
---
scripts/coccinelle/misc/minmax.cocci | 18 +-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/scripts/coccinelle/misc/minmax.cocci
b/scripts/coccinelle/misc/minmax.cocci
index eccdd3eb3452..fcf908b34f27 100644
--- a/scripts/coccinelle/misc/minmax.cocci
+++ b/scripts/coccinelle/misc/minmax.cocci
@@ -116,16 +116,32 @@ func(...)
...>
}
+// Don't generate patches for errcode returns.
+@errcode depends on patch@
+position p;
+identifier func;
+expression x;
+binary operator cmp = {<, <=};
+@@
+
+func(...)
+{
+ <...
+ return ((x) cmp@p 0 ? (x) : 0);
+ ...>
+}
+
@pmin depends on patch@
identifier func;
expression x, y;
binary operator cmp = {<=, <};
+position p != errcode.p;
@@
func(...)
{
<...
-- ((x) cmp (y) ? (x) : (y))
+- ((x) cmp@p (y) ? (x) : (y))
+ min(x, y)
...>
}
--
2.30.2
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Incorrect match with when != condition
On 3/17/21 11:32 PM, Julia Lawall wrote:
On Wed, 17 Mar 2021, Denis Efremov wrote:
Hi,
I'm trying to write the check to detect the absence of commit
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit?id=51b2ee7d006a736a9126e8111d1f24e4fd0afaa6
in kernel. The pattern can be:
@err exists@
identifier namlen, dchild, dparent, exp;
position p;
statement S;
@@
compose_entry_fh(..., int namlen, ...)
{
...
if (namlen == 2) {
* dchild =@p dget_parent(dparent);
... when != dparent == exp->ex_path.dentry
add when forall here. There does exist a path that does not contain the
dparent == exp->ex_path.dentry test. That is the path that takesthe first
goto out.
This solved the problem, thanks!
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] Incorrect match with when != condition
Hi,
I'm trying to write the check to detect the absence of commit
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit?id=51b2ee7d006a736a9126e8111d1f24e4fd0afaa6
in kernel. The pattern can be:
@err exists@
identifier namlen, dchild, dparent, exp;
position p;
statement S;
@@
compose_entry_fh(..., int namlen, ...)
{
...
if (namlen == 2) {
* dchild =@p dget_parent(dparent);
... when != dparent == exp->ex_path.dentry
} else S
...
}
But unfortunately, it matches even the fixed source. I think
that condition "... when != dparent == exp->ex_path.dentry" doesn't
work as expected. Steps to reproduce:
$ cd linux # latest master branch
$ wget
https://raw.githubusercontent.com/evdenis/cvehound/b2d109c959c299dce10274c1806406fc5653e5d0/cvehound/cve/CVE-2021-3178.cocci
$ spatch -D detect --cocci-file CVE-2021-3178.cocci fs/nfsd/nfs3xdr.c
fs/nfsd/nfs3xdr.c:935:10-11: ERROR: CVE-2021-3178
diff =
--- fs/nfsd/nfs3xdr.c
+++ /tmp/cocci-output-526900-b87df1-nfs3xdr.c
@@ -932,7 +932,6 @@ compose_entry_fh(struct nfsd3_readdirres
if (isdotent(name, namlen)) {
if (namlen == 2) {
// !!! shouldn't match because of if (dparent ==
exp->ex_path.dentry) goto out; check after
- dchild = dget_parent(dparent);
/*
* Don't return filehandle for ".." if we're at
* the filesystem or export root:
$ spatch --version
spatch version 1.1.0 compiled with OCaml version 4.11.1
Thanks,
Denis
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH v5] coccinelle: misc: add minmax script
Check for opencoded min(), max() implementations.
Signed-off-by: Denis Efremov
---
Changes in v2:
- <... ...> instead of ... when any
- org mode reports fixed
- patch rule to drop excessive ()
Changes in v3:
- "depends on patch && (pmax || pmaxif || pmin || pminif)" fixed
Changes in v4:
- refarmatting rule removed
- () brackets added to the patch rules to omit excessive ones
- org/report prints changed to cycle (for p0 in p: ...)
Changes in v5:
- parentheses droppped in pminif and pmaxif rules (max_val = x ...)
scripts/coccinelle/misc/minmax.cocci | 206 +++
1 file changed, 206 insertions(+)
create mode 100644 scripts/coccinelle/misc/minmax.cocci
diff --git a/scripts/coccinelle/misc/minmax.cocci
b/scripts/coccinelle/misc/minmax.cocci
new file mode 100644
index ..eccdd3eb3452
--- /dev/null
+++ b/scripts/coccinelle/misc/minmax.cocci
@@ -0,0 +1,206 @@
+// SPDX-License-Identifier: GPL-2.0-only
+///
+/// Check for opencoded min(), max() implementations.
+/// Generated patches sometimes require adding a cast to fix compile warning.
+/// Warnings/patches scope intentionally limited to a function body.
+///
+// Confidence: Medium
+// Copyright: (C) 2021 Denis Efremov ISPRAS
+// Options: --no-includes --include-headers
+//
+// Keywords: min, max
+//
+
+
+virtual report
+virtual org
+virtual context
+virtual patch
+
+@rmax depends on !patch@
+identifier func;
+expression x, y;
+binary operator cmp = {>, >=};
+position p;
+@@
+
+func(...)
+{
+ <...
+* ((x) cmp@p (y) ? (x) : (y))
+ ...>
+}
+
+@rmaxif depends on !patch@
+identifier func;
+expression x, y;
+expression max_val;
+binary operator cmp = {>, >=};
+position p;
+@@
+
+func(...)
+{
+ <...
+* if ((x) cmp@p (y)) {
+* max_val = (x);
+* } else {
+* max_val = (y);
+* }
+ ...>
+}
+
+@rmin depends on !patch@
+identifier func;
+expression x, y;
+binary operator cmp = {<, <=};
+position p;
+@@
+
+func(...)
+{
+ <...
+* ((x) cmp@p (y) ? (x) : (y))
+ ...>
+}
+
+@rminif depends on !patch@
+identifier func;
+expression x, y;
+expression min_val;
+binary operator cmp = {<, <=};
+position p;
+@@
+
+func(...)
+{
+ <...
+* if ((x) cmp@p (y)) {
+* min_val = (x);
+* } else {
+* min_val = (y);
+* }
+ ...>
+}
+
+@pmax depends on patch@
+identifier func;
+expression x, y;
+binary operator cmp = {>=, >};
+@@
+
+func(...)
+{
+ <...
+- ((x) cmp (y) ? (x) : (y))
++ max(x, y)
+ ...>
+}
+
+@pmaxif depends on patch@
+identifier func;
+expression x, y;
+expression max_val;
+binary operator cmp = {>=, >};
+@@
+
+func(...)
+{
+ <...
+- if ((x) cmp (y)) {
+- max_val = x;
+- } else {
+- max_val = y;
+- }
++ max_val = max(x, y);
+ ...>
+}
+
+@pmin depends on patch@
+identifier func;
+expression x, y;
+binary operator cmp = {<=, <};
+@@
+
+func(...)
+{
+ <...
+- ((x) cmp (y) ? (x) : (y))
++ min(x, y)
+ ...>
+}
+
+@pminif depends on patch@
+identifier func;
+expression x, y;
+expression min_val;
+binary operator cmp = {<=, <};
+@@
+
+func(...)
+{
+ <...
+- if ((x) cmp (y)) {
+- min_val = x;
+- } else {
+- min_val = y;
+- }
++ min_val = min(x, y);
+ ...>
+}
+
+@script:python depends on report@
+p << rmax.p;
+@@
+
+for p0 in p:
+ coccilib.report.print_report(p0, "WARNING opportunity for max()")
+
+@script:python depends on org@
+p << rmax.p;
+@@
+
+for p0 in p:
+ coccilib.org.print_todo(p0, "WARNING opportunity for max()")
+
+@script:python depends on report@
+p << rmaxif.p;
+@@
+
+for p0 in p:
+ coccilib.report.print_report(p0, "WARNING opportunity for max()")
+
+@script:python depends on org@
+p << rmaxif.p;
+@@
+
+for p0 in p:
+ coccilib.org.print_todo(p0, "WARNING opportunity for max()")
+
+@script:python depends on report@
+p << rmin.p;
+@@
+
+for p0 in p:
+ coccilib.report.print_report(p0, "WARNING opportunity for min()")
+
+@script:python depends on org@
+p << rmin.p;
+@@
+
+for p0 in p:
+ coccilib.org.print_todo(p0, "WARNING opportunity for min()")
+
+@script:python depends on report@
+p << rminif.p;
+@@
+
+for p0 in p:
+ coccilib.report.print_report(p0, "WARNING opportunity for min()")
+
+@script:python depends on org@
+p << rminif.p;
+@@
+
+for p0 in p:
+ coccilib.org.print_todo(p0, "WARNING opportunity for min()")
--
2.26.2
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH] coccinelle: misc: restrict patch mode in flexible_array.cocci
Skip patches generation for structs/unions with a single field.
Changing a zero-length array to a flexible array member in a struct
with no named members breaks the compilation. However, reporting
such cases is still valuable, e.g. commit 637464c59e0b
("ACPI: NFIT: Fix flexible_array.cocci warnings").
Signed-off-by: Denis Efremov
---
scripts/coccinelle/misc/flexible_array.cocci | 23 ++--
1 file changed, 21 insertions(+), 2 deletions(-)
diff --git a/scripts/coccinelle/misc/flexible_array.cocci
b/scripts/coccinelle/misc/flexible_array.cocci
index 947fbaff82a9..f427fd68ed2d 100644
--- a/scripts/coccinelle/misc/flexible_array.cocci
+++ b/scripts/coccinelle/misc/flexible_array.cocci
@@ -51,21 +51,40 @@ position p : script:python() { relevant(p) };
};
)
+@only_field depends on patch@
+identifier name, array;
+type T;
+position q;
+@@
+
+(
+ struct name {@q
+T array[0];
+ };
+|
+ struct {@q
+T array[0];
+ };
+)
+
@depends on patch@
identifier name, array;
type T;
position p : script:python() { relevant(p) };
+// position @q with rule "only_field" simplifies
+// handling of bitfields, arrays, etc.
+position q != only_field.q;
@@
(
- struct name {
+ struct name {@q
...
T array@p[
- 0
];
};
|
- struct {
+ struct {@q
...
T array@p[
- 0
--
2.26.2
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH] coccinelle: misc: update uninitialized_var.cocci documentation
Remove the documentation link from the warning message because commit
3942ea7a10c9 ("deprecated.rst: Remove now removed uninitialized_var")
removed the section from documentation. Update the rule documentation
accordingly.
Signed-off-by: Denis Efremov
---
scripts/coccinelle/misc/uninitialized_var.cocci | 15 ++-
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/scripts/coccinelle/misc/uninitialized_var.cocci
b/scripts/coccinelle/misc/uninitialized_var.cocci
index 8fa845cefe11..69bbaae47e73 100644
--- a/scripts/coccinelle/misc/uninitialized_var.cocci
+++ b/scripts/coccinelle/misc/uninitialized_var.cocci
@@ -1,7 +1,9 @@
// SPDX-License-Identifier: GPL-2.0-only
///
/// Please, don't reintroduce uninitialized_var().
-/// From Documentation/process/deprecated.rst:
+///
+/// From Documentation/process/deprecated.rst,
+/// commit 4b19bec97c88 ("docs: deprecated.rst: Add uninitialized_var()"):
/// For any compiler warnings about uninitialized variables, just add
/// an initializer. Using warning-silencing tricks is dangerous as it
/// papers over real bugs (or can in the future), and suppresses unrelated
@@ -11,6 +13,11 @@
/// obviously redundant, the compiler's dead-store elimination pass will make
/// sure there are no needless variable writes.
///
+/// Later, commit 3942ea7a10c9 ("deprecated.rst: Remove now removed
+/// uninitialized_var") removed this section because all initializations of
+/// this kind were cleaned-up from the kernel. This cocci rule checks that
+/// the macro is not explicitly or implicitly reintroduced.
+///
// Confidence: High
// Copyright: (C) 2020 Denis Efremov ISPRAS
// Options: --no-includes --include-headers
@@ -40,12 +47,10 @@ position p;
p << r.p;
@@
-coccilib.report.print_report(p[0],
- "WARNING this kind of initialization is deprecated
(https://www.kernel.org/doc/html/latest/process/deprecated.html#uninitialized-var)")
+coccilib.report.print_report(p[0], "WARNING this kind of initialization is
deprecated")
@script:python depends on org@
p << r.p;
@@
-coccilib.org.print_todo(p[0],
- "WARNING this kind of initialization is deprecated
(https://www.kernel.org/doc/html/latest/process/deprecated.html#uninitialized-var)")
+coccilib.org.print_todo(p[0], "WARNING this kind of initialization is
deprecated")
--
2.26.2
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH v4] coccinelle: misc: add minmax script
Check for opencoded min(), max() implementations.
Signed-off-by: Denis Efremov
---
Changes in v2:
- <... ...> instead of ... when any
- org mode reports fixed
- patch rule to drop excessive ()
Changes in v3:
- "depends on patch && (pmax || pmaxif || pmin || pminif)" fixed
Changes in v4:
- refarmatting rule removed
- () brackets added to the patch rules to omit excessive ones
- org/report prints changed to cycle (for p0 in p: ...)
scripts/coccinelle/misc/minmax.cocci | 206 +++
1 file changed, 206 insertions(+)
create mode 100644 scripts/coccinelle/misc/minmax.cocci
diff --git a/scripts/coccinelle/misc/minmax.cocci
b/scripts/coccinelle/misc/minmax.cocci
new file mode 100644
index ..63eeba1702ec
--- /dev/null
+++ b/scripts/coccinelle/misc/minmax.cocci
@@ -0,0 +1,206 @@
+// SPDX-License-Identifier: GPL-2.0-only
+///
+/// Check for opencoded min(), max() implementations.
+/// Generated patches sometimes require adding a cast to fix compile warning.
+/// Warnings/patches scope intentionally limited to a function body.
+///
+// Confidence: Medium
+// Copyright: (C) 2021 Denis Efremov ISPRAS
+// Options: --no-includes --include-headers
+//
+// Keywords: min, max
+//
+
+
+virtual report
+virtual org
+virtual context
+virtual patch
+
+@rmax depends on !patch@
+identifier func;
+expression x, y;
+binary operator cmp = {>, >=};
+position p;
+@@
+
+func(...)
+{
+ <...
+* ((x) cmp@p (y) ? (x) : (y))
+ ...>
+}
+
+@rmaxif depends on !patch@
+identifier func;
+expression x, y;
+expression max_val;
+binary operator cmp = {>, >=};
+position p;
+@@
+
+func(...)
+{
+ <...
+* if ((x) cmp@p (y)) {
+* max_val = (x);
+* } else {
+* max_val = (y);
+* }
+ ...>
+}
+
+@rmin depends on !patch@
+identifier func;
+expression x, y;
+binary operator cmp = {<, <=};
+position p;
+@@
+
+func(...)
+{
+ <...
+* ((x) cmp@p (y) ? (x) : (y))
+ ...>
+}
+
+@rminif depends on !patch@
+identifier func;
+expression x, y;
+expression min_val;
+binary operator cmp = {<, <=};
+position p;
+@@
+
+func(...)
+{
+ <...
+* if ((x) cmp@p (y)) {
+* min_val = (x);
+* } else {
+* min_val = (y);
+* }
+ ...>
+}
+
+@pmax depends on patch@
+identifier func;
+expression x, y;
+binary operator cmp = {>=, >};
+@@
+
+func(...)
+{
+ <...
+- ((x) cmp (y) ? (x) : (y))
++ max(x, y)
+ ...>
+}
+
+@pmaxif depends on patch@
+identifier func;
+expression x, y;
+expression max_val;
+binary operator cmp = {>=, >};
+@@
+
+func(...)
+{
+ <...
+- if ((x) cmp (y)) {
+- max_val = (x);
+- } else {
+- max_val = (y);
+- }
++ max_val = max(x, y);
+ ...>
+}
+
+@pmin depends on patch@
+identifier func;
+expression x, y;
+binary operator cmp = {<=, <};
+@@
+
+func(...)
+{
+ <...
+- ((x) cmp (y) ? (x) : (y))
++ min(x, y)
+ ...>
+}
+
+@pminif depends on patch@
+identifier func;
+expression x, y;
+expression min_val;
+binary operator cmp = {<=, <};
+@@
+
+func(...)
+{
+ <...
+- if ((x) cmp (y)) {
+- min_val = (x);
+- } else {
+- min_val = (y);
+- }
++ min_val = min(x, y);
+ ...>
+}
+
+@script:python depends on report@
+p << rmax.p;
+@@
+
+for p0 in p:
+ coccilib.report.print_report(p0, "WARNING opportunity for max()")
+
+@script:python depends on org@
+p << rmax.p;
+@@
+
+for p0 in p:
+ coccilib.org.print_todo(p0, "WARNING opportunity for max()")
+
+@script:python depends on report@
+p << rmaxif.p;
+@@
+
+for p0 in p:
+ coccilib.report.print_report(p0, "WARNING opportunity for max()")
+
+@script:python depends on org@
+p << rmaxif.p;
+@@
+
+for p0 in p:
+ coccilib.org.print_todo(p0, "WARNING opportunity for max()")
+
+@script:python depends on report@
+p << rmin.p;
+@@
+
+for p0 in p:
+ coccilib.report.print_report(p0, "WARNING opportunity for min()")
+
+@script:python depends on org@
+p << rmin.p;
+@@
+
+for p0 in p:
+ coccilib.org.print_todo(p0, "WARNING opportunity for min()")
+
+@script:python depends on report@
+p << rminif.p;
+@@
+
+for p0 in p:
+ coccilib.report.print_report(p0, "WARNING opportunity for min()")
+
+@script:python depends on org@
+p << rminif.p;
+@@
+
+for p0 in p:
+ coccilib.org.print_todo(p0, "WARNING opportunity for min()")
--
2.26.2
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH v3] coccinelle: misc: add minmax script
Check for opencoded min(), max() implementations.
Signed-off-by: Denis Efremov
---
Changes in v2:
- <... ...> instead of ... when any
- org mode reports fixed
- patch rule to drop excessive ()
Changes in v3:
- "depends on patch && (pmax || pmaxif || pmin || pminif)" fixed
scripts/coccinelle/misc/minmax.cocci | 224 +++
1 file changed, 224 insertions(+)
create mode 100644 scripts/coccinelle/misc/minmax.cocci
diff --git a/scripts/coccinelle/misc/minmax.cocci
b/scripts/coccinelle/misc/minmax.cocci
new file mode 100644
index ..f577f08d1e6e
--- /dev/null
+++ b/scripts/coccinelle/misc/minmax.cocci
@@ -0,0 +1,224 @@
+// SPDX-License-Identifier: GPL-2.0-only
+///
+/// Check for opencoded min(), max() implementations.
+/// Generated patches sometimes require adding a cast to fix compile warning.
+/// Warnings/patches scope intentionally limited to a function body.
+///
+// Confidence: Medium
+// Copyright: (C) 2021 Denis Efremov ISPRAS
+// Options: --no-includes --include-headers
+//
+// Keywords: min, max
+//
+
+
+virtual report
+virtual org
+virtual context
+virtual patch
+
+@rmax depends on !patch@
+identifier func;
+expression x, y;
+binary operator cmp = {>, >=};
+position p;
+@@
+
+func(...)
+{
+ <...
+* x cmp@p y ? x : y
+ ...>
+}
+
+@rmaxif depends on !patch@
+identifier func;
+expression x, y;
+expression max_val;
+binary operator cmp = {>, >=};
+position p;
+@@
+
+func(...)
+{
+ <...
+* if (x cmp@p y) {
+* max_val = x;
+* } else {
+* max_val = y;
+* }
+ ...>
+}
+
+@rmin depends on !patch@
+identifier func;
+expression x, y;
+binary operator cmp = {<, <=};
+position p;
+@@
+
+func(...)
+{
+ <...
+* x cmp@p y ? x : y
+ ...>
+}
+
+@rminif depends on !patch@
+identifier func;
+expression x, y;
+expression min_val;
+binary operator cmp = {<, <=};
+position p;
+@@
+
+func(...)
+{
+ <...
+* if (x cmp@p y) {
+* min_val = x;
+* } else {
+* min_val = y;
+* }
+ ...>
+}
+
+@pmax depends on patch@
+identifier func;
+expression x, y;
+binary operator cmp = {>=, >};
+position p;
+@@
+
+func@p(...)
+{
+ <...
+- x cmp y ? x : y
++ max(x, y)
+ ...>
+}
+
+@pmaxif depends on patch@
+identifier func;
+expression x, y;
+expression max_val;
+binary operator cmp = {>=, >};
+position p;
+@@
+
+func@p(...)
+{
+ <...
+- if (x cmp y) {
+- max_val = x;
+- } else {
+- max_val = y;
+- }
++ max_val = max(x, y);
+ ...>
+}
+
+@pmin depends on patch@
+identifier func;
+expression x, y;
+binary operator cmp = {<=, <};
+position p;
+@@
+
+func@p(...)
+{
+ <...
+- x cmp y ? x : y
++ min(x, y)
+ ...>
+}
+
+@pminif depends on patch@
+identifier func;
+expression x, y;
+expression min_val;
+binary operator cmp = {<=, <};
+position p;
+@@
+
+func@p(...)
+{
+ <...
+- if (x cmp y) {
+- min_val = x;
+- } else {
+- min_val = y;
+- }
++ min_val = min(x, y);
+ ...>
+}
+
+@depends on patch && (pmax || pmaxif || pmin || pminif)@
+identifier func;
+expression x, y;
+position p;
+// FIXME: Coccinelle consumes all available ram and
+// and timeouts on every file.
+// position p = { pmin.p, pminif.p, pmax.p, pmaxif.p };
+@@
+
+func@p(...)
+{
+ <...
+(
+- (min((x), (y)))
++ min(x, y)
+|
+- (max((x), (y)))
++ max(x, y)
+)
+ ...>
+}
+
+@script:python depends on report@
+p << rmax.p;
+@@
+
+coccilib.report.print_report(p[0], "WARNING opportunity for max()")
+
+@script:python depends on org@
+p << rmax.p;
+@@
+
+coccilib.org.print_todo(p[0], "WARNING opportunity for max()")
+
+@script:python depends on report@
+p << rmaxif.p;
+@@
+
+coccilib.report.print_report(p[0], "WARNING opportunity for max()")
+
+@script:python depends on org@
+p << rmaxif.p;
+@@
+
+coccilib.org.print_todo(p[0], "WARNING opportunity for max()")
+
+@script:python depends on report@
+p << rmin.p;
+@@
+
+coccilib.report.print_report(p[0], "WARNING opportunity for min()")
+
+@script:python depends on org@
+p << rmin.p;
+@@
+
+coccilib.org.print_todo(p[0], "WARNING opportunity for min()")
+
+@script:python depends on report@
+p << rminif.p;
+@@
+
+coccilib.report.print_report(p[0], "WARNING opportunity for min()")
+
+@script:python depends on org@
+p << rminif.p;
+@@
+
+coccilib.org.print_todo(p[0], "WARNING opportunity for min()")
--
2.26.2
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH v3] coccinelle: misc: add swap script
Check for opencoded swap() implementation.
Signed-off-by: Denis Efremov
---
Changes in v2:
- additional patch rule to drop excessive {}
- fix indentation in patch mode by anchoring ;
Changes in v3:
- Rule added for simple (without var init) swap highlighting in !patch mode
- "depends on patch && (rpvar || rp)" fixed
scripts/coccinelle/misc/swap.cocci | 122 +
1 file changed, 122 insertions(+)
create mode 100644 scripts/coccinelle/misc/swap.cocci
diff --git a/scripts/coccinelle/misc/swap.cocci
b/scripts/coccinelle/misc/swap.cocci
new file mode 100644
index ..c5e71b7ef7f5
--- /dev/null
+++ b/scripts/coccinelle/misc/swap.cocci
@@ -0,0 +1,122 @@
+// SPDX-License-Identifier: GPL-2.0-only
+///
+/// Check for opencoded swap() implementation.
+///
+// Confidence: High
+// Copyright: (C) 2021 Denis Efremov ISPRAS
+// Options: --no-includes --include-headers
+//
+// Keywords: swap
+//
+
+virtual patch
+virtual org
+virtual report
+virtual context
+
+@rvar depends on !patch@
+identifier tmp;
+expression a, b;
+type T;
+position p;
+@@
+
+(
+* T tmp;
+|
+* T tmp = 0;
+|
+* T *tmp = NULL;
+)
+... when != tmp
+* tmp = a;
+* a = b;@p
+* b = tmp;
+... when != tmp
+
+@r depends on !patch@
+identifier tmp;
+expression a, b;
+position p != rvar.p;
+@@
+
+* tmp = a;
+* a = b;@p
+* b = tmp;
+
+@rpvar depends on patch@
+identifier tmp;
+expression a, b;
+type T;
+@@
+
+(
+- T tmp;
+|
+- T tmp = 0;
+|
+- T *tmp = NULL;
+)
+... when != tmp
+- tmp = a;
+- a = b;
+- b = tmp
++ swap(a, b)
+ ;
+... when != tmp
+
+@rp depends on patch@
+identifier tmp;
+expression a, b;
+@@
+
+- tmp = a;
+- a = b;
+- b = tmp
++ swap(a, b)
+ ;
+
+@depends on patch && (rpvar || rp)@
+@@
+
+(
+ for (...;...;...)
+- {
+ swap(...);
+- }
+|
+ while (...)
+- {
+ swap(...);
+- }
+|
+ if (...)
+- {
+ swap(...);
+- }
+)
+
+
+@script:python depends on report@
+p << r.p;
+@@
+
+coccilib.report.print_report(p[0], "WARNING opportunity for swap()")
+
+@script:python depends on org@
+p << r.p;
+@@
+
+coccilib.org.print_todo(p[0], "WARNING opportunity for swap()")
+
+@script:python depends on report@
+p << rvar.p;
+@@
+
+coccilib.report.print_report(p[0], "WARNING opportunity for swap()")
+
+@script:python depends on org@
+p << rvar.p;
+@@
+
+coccilib.org.print_todo(p[0], "WARNING opportunity for swap()")
--
2.26.2
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] release of version 1.1.0
On 2/27/21 12:14 PM, Julia Lawall wrote: > > > On Sat, 27 Feb 2021, Denis Efremov wrote: > >> Hi, >> >> Great news! >> >> Could you please create release tag on GitHub? >> This will allow me to add this version to tests and add new opam package >> version. >> Currently, I use these versions for tests: >> https://github.com/evdenis/cvehound/blob/master/.github/workflows/test.yml#L25 > > There is no tag at the moment. I have asked why. > >> >> BTW, is there 1.0.9 version? I see that fedora provides 1.0.9 version, >> but there is not release tag for it and there is no opam package for it. > > No, there is no version 1.0.9. We moved on to 1.1 due to the #spatch > feature. > Seems like there is a mess. There is a commit with 1.0.9 release planned: https://github.com/coccinelle/coccinelle/commit/842075f77505386a782030ebffb1acf7f4237737#diff-652d5876e7e73c420820682a12ecdec66ecdec029223e24808e129d64493a967 Repology says that Fedora provides most recent coccinelle version: https://repology.org/project/coccinelle/versions Other distibutives use 1.0.8 version. On Fedora: $ sudo dnf info coccinelle Installed Packages Name : coccinelle Version : 1.0.9 Release : 0.14.gitd0fd4c7d.fc33.1 // <== d0fd4c7d commit reference Architecture : x86_64 Size : 26 M Source : coccinelle-1.0.9-0.14.gitd0fd4c7d.fc33.1.src.rpm $ /usr/bin/spatch --version spatch version 1.0.8-gc1dbb4f-dirty compiled with OCaml version 4.11.1 Flags passed to the configure script: --build=x86_64-redhat-linux-gnu --host=x86_64-redhat-linux-gnu --program-prefix= --disable-dependency-tracking --prefix=/usr --exec-prefix=/usr --bindir=/usr/bin --sbindir=/usr/sbin --sysconfdir=/etc --datadir=/usr/share --includedir=/usr/include --libdir=/usr/lib64 --libexecdir=/usr/libexec --localstatedir=/var --sharedstatedir=/var/lib --mandir=/usr/share/man --infodir=/usr/share/info --with-python=/usr/bin/python3 --with-menhir=/usr/bin/menhir OCaml scripting support: yes Python scripting support: yes Syntax of regular expressions: PCRE Fedora 33, 1.0.9 version points to commit: $ fedpkg clone -a coccinelle $ cd coccinelle $ git checkout f33 $ cat coccinelle.spec | grep commit %global commit d0fd4c7dfa70763870914eedee7022fa35f700e2 %global shortcommit d0fd4c7d Fedora 34 also uses d0fd4c7d as base for 1.0.9 version. Fedora 32 points to 111d328fee1303f14a5b9def835301d849e41331. Maybe release 1.0.9 wasn't planned, but as my personal opinion adding 1.0.9 tag is not a bad idea just to reference some commit (for example, d0fd4c7d) and have 1.0.9 version to run backward compatibility tests on it. https://github.com/coccinelle/coccinelle/releases Thanks, Denis >> >> On 2/27/21 11:11 AM, Julia Lawall wrote: >>> A new version 1.1.0 has been released. The most interesting change is the >>> ability to put spatch command line options in a .cocci file, for example: >>> >>> #spatch --very-quiet -j 8 --recursive-includes --include-headers-for-types >>> >>> Other specific improvements are an improved handling of attributes and a >>> more efficient treatment of header files (all thanks to Jaskaran Singh). >>> >>> Various other small issues have been addressed. >>> >>> All of these changes have already been available on github. >>> >>> julia >>> ___ >>> Cocci mailing list >>> Cocci@systeme.lip6.fr >>> https://systeme.lip6.fr/mailman/listinfo/cocci >>> >> ___ >> Cocci mailing list >> Cocci@systeme.lip6.fr >> https://systeme.lip6.fr/mailman/listinfo/cocci >> ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] release of version 1.1.0
Hi, Great news! Could you please create release tag on GitHub? This will allow me to add this version to tests and add new opam package version. Currently, I use these versions for tests: https://github.com/evdenis/cvehound/blob/master/.github/workflows/test.yml#L25 BTW, is there 1.0.9 version? I see that fedora provides 1.0.9 version, but there is not release tag for it and there is no opam package for it. Thanks, Denis On 2/27/21 11:11 AM, Julia Lawall wrote: > A new version 1.1.0 has been released. The most interesting change is the > ability to put spatch command line options in a .cocci file, for example: > > #spatch --very-quiet -j 8 --recursive-includes --include-headers-for-types > > Other specific improvements are an improved handling of attributes and a > more efficient treatment of header files (all thanks to Jaskaran Singh). > > Various other small issues have been addressed. > > All of these changes have already been available on github. > > julia > ___ > Cocci mailing list > Cocci@systeme.lip6.fr > https://systeme.lip6.fr/mailman/listinfo/cocci > ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH v2] coccinelle: misc: add swap script
Check for opencoded swap() implementation.
Signed-off-by: Denis Efremov
---
Changes in v2:
- additional patch rule to drop excessive {}
- fix indentation in patch mode by anchoring ;
scripts/coccinelle/misc/swap.cocci | 101 +
1 file changed, 101 insertions(+)
create mode 100644 scripts/coccinelle/misc/swap.cocci
diff --git a/scripts/coccinelle/misc/swap.cocci
b/scripts/coccinelle/misc/swap.cocci
new file mode 100644
index ..d5da9888c222
--- /dev/null
+++ b/scripts/coccinelle/misc/swap.cocci
@@ -0,0 +1,101 @@
+// SPDX-License-Identifier: GPL-2.0-only
+///
+/// Check for opencoded swap() implementation.
+///
+// Confidence: High
+// Copyright: (C) 2021 Denis Efremov ISPRAS
+// Options: --no-includes --include-headers
+//
+// Keywords: swap
+//
+
+virtual patch
+virtual org
+virtual report
+virtual context
+
+@r depends on !patch@
+identifier tmp;
+expression a, b;
+type T;
+position p;
+@@
+
+(
+* T tmp;
+|
+* T tmp = 0;
+|
+* T *tmp = NULL;
+)
+... when != tmp
+* tmp = a;
+* a = b;@p
+* b = tmp;
+... when != tmp
+
+@rpvar depends on patch@
+identifier tmp;
+expression a, b;
+type T;
+@@
+
+(
+- T tmp;
+|
+- T tmp = 0;
+|
+- T *tmp = NULL;
+)
+... when != tmp
+- tmp = a;
+- a = b;
+- b = tmp
++ swap(a, b)
+ ;
+... when != tmp
+
+
+@rp depends on patch@
+identifier tmp;
+expression a, b;
+@@
+
+- tmp = a;
+- a = b;
+- b = tmp
++ swap(a, b)
+ ;
+
+@depends on (rpvar || rp)@
+@@
+
+(
+ for (...;...;...)
+- {
+ swap(...);
+- }
+|
+ while (...)
+- {
+ swap(...);
+- }
+|
+ if (...)
+- {
+ swap(...);
+- }
+)
+
+
+@script:python depends on report@
+p << r.p;
+@@
+
+coccilib.report.print_report(p[0], "WARNING opportunity for swap()")
+
+@script:python depends on org@
+p << r.p;
+@@
+
+coccilib.org.print_todo(p[0], "WARNING opportunity for swap()")
--
2.26.2
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH v2] coccinelle: misc: add minmax script
Sorry for wrong thread, I'll resend v2 to the right one.
Denis
On 2/19/21 12:05 PM, Denis Efremov wrote:
> Check for opencoded min(), max() implementations.
>
> Signed-off-by: Denis Efremov
> ---
>
> Changes in v2:
> - <... ...> instead of ... when any
> - org mode reports fixed
> - patch rule to drop excessive ()
>
> scripts/coccinelle/misc/minmax.cocci | 224 +++
> 1 file changed, 224 insertions(+)
> create mode 100644 scripts/coccinelle/misc/minmax.cocci
>
> diff --git a/scripts/coccinelle/misc/minmax.cocci
> b/scripts/coccinelle/misc/minmax.cocci
> new file mode 100644
> index ..61d6b61fd82c
> --- /dev/null
> +++ b/scripts/coccinelle/misc/minmax.cocci
> @@ -0,0 +1,224 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +///
> +/// Check for opencoded min(), max() implementations.
> +/// Generated patches sometimes require adding a cast to fix compile warning.
> +/// Warnings/patches scope intentionally limited to a function body.
> +///
> +// Confidence: Medium
> +// Copyright: (C) 2021 Denis Efremov ISPRAS
> +// Options: --no-includes --include-headers
> +//
> +// Keywords: min, max
> +//
> +
> +
> +virtual report
> +virtual org
> +virtual context
> +virtual patch
> +
> +@rmax depends on !patch@
> +identifier func;
> +expression x, y;
> +binary operator cmp = {>, >=};
> +position p;
> +@@
> +
> +func(...)
> +{
> + <...
> +*x cmp@p y ? x : y
> + ...>
> +}
> +
> +@rmaxif depends on !patch@
> +identifier func;
> +expression x, y;
> +expression max_val;
> +binary operator cmp = {>, >=};
> +position p;
> +@@
> +
> +func(...)
> +{
> + <...
> +*if (x cmp@p y) {
> +*max_val = x;
> +*} else {
> +*max_val = y;
> +*}
> + ...>
> +}
> +
> +@rmin depends on !patch@
> +identifier func;
> +expression x, y;
> +binary operator cmp = {<, <=};
> +position p;
> +@@
> +
> +func(...)
> +{
> + <...
> +*x cmp@p y ? x : y
> + ...>
> +}
> +
> +@rminif depends on !patch@
> +identifier func;
> +expression x, y;
> +expression min_val;
> +binary operator cmp = {<, <=};
> +position p;
> +@@
> +
> +func(...)
> +{
> + <...
> +*if (x cmp@p y) {
> +*min_val = x;
> +*} else {
> +*min_val = y;
> +*}
> + ...>
> +}
> +
> +@pmax depends on patch@
> +identifier func;
> +expression x, y;
> +binary operator cmp = {>=, >};
> +position p;
> +@@
> +
> +func@p(...)
> +{
> + <...
> +-x cmp y ? x : y
> ++max(x, y)
> + ...>
> +}
> +
> +@pmaxif depends on patch@
> +identifier func;
> +expression x, y;
> +expression max_val;
> +binary operator cmp = {>=, >};
> +position p;
> +@@
> +
> +func@p(...)
> +{
> + <...
> +-if (x cmp y) {
> +-max_val = x;
> +-} else {
> +-max_val = y;
> +-}
> ++max_val = max(x, y);
> + ...>
> +}
> +
> +@pmin depends on patch@
> +identifier func;
> +expression x, y;
> +binary operator cmp = {<=, <};
> +position p;
> +@@
> +
> +func@p(...)
> +{
> + <...
> +-x cmp y ? x : y
> ++min(x, y)
> + ...>
> +}
> +
> +@pminif depends on patch@
> +identifier func;
> +expression x, y;
> +expression min_val;
> +binary operator cmp = {<=, <};
> +position p;
> +@@
> +
> +func@p(...)
> +{
> + <...
> +-if (x cmp y) {
> +-min_val = x;
> +-} else {
> +-min_val = y;
> +-}
> ++min_val = min(x, y);
> + ...>
> +}
> +
> +@depends on (pmax || pmaxif || pmin || pminif)@
> +identifier func;
> +expression x, y;
> +position p;
> +// FIXME: Coccinelle consumes all available ram and
> +// and timeouts on every file.
> +// position p = { pmin.p, pminif.p, pmax.p, pmaxif.p };
> +@@
> +
> +func@p(...)
> +{
> + <...
> +(
> +-(min((x), (y)))
> ++min(x, y)
> +|
> +-(max((x), (y)))
> ++max(x, y)
> +)
> + ...>
> +}
> +
> +@script:python depends on report@
> +p << rmax.p;
> +@@
> +
> +coccilib.report.print_report(p[0], "WARNING opportunity for max()")
> +
> +@script:python depends on org@
> +p << rmax.p;
> +@@
> +
> +coccilib.org.print_todo(p[0], "WARNING opportunity for max()")
> +
> +@script:python depends on report@
> +p << rmaxif.p;
> +@@
> +
> +coccilib.report.print_report(p[0], "
[Cocci] [PATCH v2 RESEND] coccinelle: misc: add minmax script
Check for opencoded min(), max() implementations.
Signed-off-by: Denis Efremov
---
Changes in v2:
- <... ...> instead of ... when any
- org mode reports fixed
- patch rule to drop excessive ()
scripts/coccinelle/misc/minmax.cocci | 224 +++
1 file changed, 224 insertions(+)
create mode 100644 scripts/coccinelle/misc/minmax.cocci
diff --git a/scripts/coccinelle/misc/minmax.cocci
b/scripts/coccinelle/misc/minmax.cocci
new file mode 100644
index ..61d6b61fd82c
--- /dev/null
+++ b/scripts/coccinelle/misc/minmax.cocci
@@ -0,0 +1,224 @@
+// SPDX-License-Identifier: GPL-2.0-only
+///
+/// Check for opencoded min(), max() implementations.
+/// Generated patches sometimes require adding a cast to fix compile warning.
+/// Warnings/patches scope intentionally limited to a function body.
+///
+// Confidence: Medium
+// Copyright: (C) 2021 Denis Efremov ISPRAS
+// Options: --no-includes --include-headers
+//
+// Keywords: min, max
+//
+
+
+virtual report
+virtual org
+virtual context
+virtual patch
+
+@rmax depends on !patch@
+identifier func;
+expression x, y;
+binary operator cmp = {>, >=};
+position p;
+@@
+
+func(...)
+{
+ <...
+* x cmp@p y ? x : y
+ ...>
+}
+
+@rmaxif depends on !patch@
+identifier func;
+expression x, y;
+expression max_val;
+binary operator cmp = {>, >=};
+position p;
+@@
+
+func(...)
+{
+ <...
+* if (x cmp@p y) {
+* max_val = x;
+* } else {
+* max_val = y;
+* }
+ ...>
+}
+
+@rmin depends on !patch@
+identifier func;
+expression x, y;
+binary operator cmp = {<, <=};
+position p;
+@@
+
+func(...)
+{
+ <...
+* x cmp@p y ? x : y
+ ...>
+}
+
+@rminif depends on !patch@
+identifier func;
+expression x, y;
+expression min_val;
+binary operator cmp = {<, <=};
+position p;
+@@
+
+func(...)
+{
+ <...
+* if (x cmp@p y) {
+* min_val = x;
+* } else {
+* min_val = y;
+* }
+ ...>
+}
+
+@pmax depends on patch@
+identifier func;
+expression x, y;
+binary operator cmp = {>=, >};
+position p;
+@@
+
+func@p(...)
+{
+ <...
+- x cmp y ? x : y
++ max(x, y)
+ ...>
+}
+
+@pmaxif depends on patch@
+identifier func;
+expression x, y;
+expression max_val;
+binary operator cmp = {>=, >};
+position p;
+@@
+
+func@p(...)
+{
+ <...
+- if (x cmp y) {
+- max_val = x;
+- } else {
+- max_val = y;
+- }
++ max_val = max(x, y);
+ ...>
+}
+
+@pmin depends on patch@
+identifier func;
+expression x, y;
+binary operator cmp = {<=, <};
+position p;
+@@
+
+func@p(...)
+{
+ <...
+- x cmp y ? x : y
++ min(x, y)
+ ...>
+}
+
+@pminif depends on patch@
+identifier func;
+expression x, y;
+expression min_val;
+binary operator cmp = {<=, <};
+position p;
+@@
+
+func@p(...)
+{
+ <...
+- if (x cmp y) {
+- min_val = x;
+- } else {
+- min_val = y;
+- }
++ min_val = min(x, y);
+ ...>
+}
+
+@depends on (pmax || pmaxif || pmin || pminif)@
+identifier func;
+expression x, y;
+position p;
+// FIXME: Coccinelle consumes all available ram and
+// and timeouts on every file.
+// position p = { pmin.p, pminif.p, pmax.p, pmaxif.p };
+@@
+
+func@p(...)
+{
+ <...
+(
+- (min((x), (y)))
++ min(x, y)
+|
+- (max((x), (y)))
++ max(x, y)
+)
+ ...>
+}
+
+@script:python depends on report@
+p << rmax.p;
+@@
+
+coccilib.report.print_report(p[0], "WARNING opportunity for max()")
+
+@script:python depends on org@
+p << rmax.p;
+@@
+
+coccilib.org.print_todo(p[0], "WARNING opportunity for max()")
+
+@script:python depends on report@
+p << rmaxif.p;
+@@
+
+coccilib.report.print_report(p[0], "WARNING opportunity for max()")
+
+@script:python depends on org@
+p << rmaxif.p;
+@@
+
+coccilib.org.print_todo(p[0], "WARNING opportunity for max()")
+
+@script:python depends on report@
+p << rmin.p;
+@@
+
+coccilib.report.print_report(p[0], "WARNING opportunity for min()")
+
+@script:python depends on org@
+p << rmin.p;
+@@
+
+coccilib.org.print_todo(p[0], "WARNING opportunity for min()")
+
+@script:python depends on report@
+p << rminif.p;
+@@
+
+coccilib.report.print_report(p[0], "WARNING opportunity for min()")
+
+@script:python depends on org@
+p << rminif.p;
+@@
+
+coccilib.org.print_todo(p[0], "WARNING opportunity for min()")
--
2.26.2
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH v2] coccinelle: misc: add minmax script
Check for opencoded min(), max() implementations.
Signed-off-by: Denis Efremov
---
Changes in v2:
- <... ...> instead of ... when any
- org mode reports fixed
- patch rule to drop excessive ()
scripts/coccinelle/misc/minmax.cocci | 224 +++
1 file changed, 224 insertions(+)
create mode 100644 scripts/coccinelle/misc/minmax.cocci
diff --git a/scripts/coccinelle/misc/minmax.cocci
b/scripts/coccinelle/misc/minmax.cocci
new file mode 100644
index ..61d6b61fd82c
--- /dev/null
+++ b/scripts/coccinelle/misc/minmax.cocci
@@ -0,0 +1,224 @@
+// SPDX-License-Identifier: GPL-2.0-only
+///
+/// Check for opencoded min(), max() implementations.
+/// Generated patches sometimes require adding a cast to fix compile warning.
+/// Warnings/patches scope intentionally limited to a function body.
+///
+// Confidence: Medium
+// Copyright: (C) 2021 Denis Efremov ISPRAS
+// Options: --no-includes --include-headers
+//
+// Keywords: min, max
+//
+
+
+virtual report
+virtual org
+virtual context
+virtual patch
+
+@rmax depends on !patch@
+identifier func;
+expression x, y;
+binary operator cmp = {>, >=};
+position p;
+@@
+
+func(...)
+{
+ <...
+* x cmp@p y ? x : y
+ ...>
+}
+
+@rmaxif depends on !patch@
+identifier func;
+expression x, y;
+expression max_val;
+binary operator cmp = {>, >=};
+position p;
+@@
+
+func(...)
+{
+ <...
+* if (x cmp@p y) {
+* max_val = x;
+* } else {
+* max_val = y;
+* }
+ ...>
+}
+
+@rmin depends on !patch@
+identifier func;
+expression x, y;
+binary operator cmp = {<, <=};
+position p;
+@@
+
+func(...)
+{
+ <...
+* x cmp@p y ? x : y
+ ...>
+}
+
+@rminif depends on !patch@
+identifier func;
+expression x, y;
+expression min_val;
+binary operator cmp = {<, <=};
+position p;
+@@
+
+func(...)
+{
+ <...
+* if (x cmp@p y) {
+* min_val = x;
+* } else {
+* min_val = y;
+* }
+ ...>
+}
+
+@pmax depends on patch@
+identifier func;
+expression x, y;
+binary operator cmp = {>=, >};
+position p;
+@@
+
+func@p(...)
+{
+ <...
+- x cmp y ? x : y
++ max(x, y)
+ ...>
+}
+
+@pmaxif depends on patch@
+identifier func;
+expression x, y;
+expression max_val;
+binary operator cmp = {>=, >};
+position p;
+@@
+
+func@p(...)
+{
+ <...
+- if (x cmp y) {
+- max_val = x;
+- } else {
+- max_val = y;
+- }
++ max_val = max(x, y);
+ ...>
+}
+
+@pmin depends on patch@
+identifier func;
+expression x, y;
+binary operator cmp = {<=, <};
+position p;
+@@
+
+func@p(...)
+{
+ <...
+- x cmp y ? x : y
++ min(x, y)
+ ...>
+}
+
+@pminif depends on patch@
+identifier func;
+expression x, y;
+expression min_val;
+binary operator cmp = {<=, <};
+position p;
+@@
+
+func@p(...)
+{
+ <...
+- if (x cmp y) {
+- min_val = x;
+- } else {
+- min_val = y;
+- }
++ min_val = min(x, y);
+ ...>
+}
+
+@depends on (pmax || pmaxif || pmin || pminif)@
+identifier func;
+expression x, y;
+position p;
+// FIXME: Coccinelle consumes all available ram and
+// and timeouts on every file.
+// position p = { pmin.p, pminif.p, pmax.p, pmaxif.p };
+@@
+
+func@p(...)
+{
+ <...
+(
+- (min((x), (y)))
++ min(x, y)
+|
+- (max((x), (y)))
++ max(x, y)
+)
+ ...>
+}
+
+@script:python depends on report@
+p << rmax.p;
+@@
+
+coccilib.report.print_report(p[0], "WARNING opportunity for max()")
+
+@script:python depends on org@
+p << rmax.p;
+@@
+
+coccilib.org.print_todo(p[0], "WARNING opportunity for max()")
+
+@script:python depends on report@
+p << rmaxif.p;
+@@
+
+coccilib.report.print_report(p[0], "WARNING opportunity for max()")
+
+@script:python depends on org@
+p << rmaxif.p;
+@@
+
+coccilib.org.print_todo(p[0], "WARNING opportunity for max()")
+
+@script:python depends on report@
+p << rmin.p;
+@@
+
+coccilib.report.print_report(p[0], "WARNING opportunity for min()")
+
+@script:python depends on org@
+p << rmin.p;
+@@
+
+coccilib.org.print_todo(p[0], "WARNING opportunity for min()")
+
+@script:python depends on report@
+p << rminif.p;
+@@
+
+coccilib.report.print_report(p[0], "WARNING opportunity for min()")
+
+@script:python depends on org@
+p << rminif.p;
+@@
+
+coccilib.org.print_todo(p[0], "WARNING opportunity for min()")
--
2.26.2
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH] coccinelle: misc: add swap script
On 2/18/21 2:29 PM, Julia Lawall wrote:
>
>
> On Thu, 18 Feb 2021, Denis Efremov wrote:
>
>>
>>
>> On 2/18/21 1:17 PM, Julia Lawall wrote:
>>>
>>>
>>> On Thu, 18 Feb 2021, Denis Efremov wrote:
>>>
>>>>
>>>>
>>>> On 2/18/21 12:31 AM, Julia Lawall wrote:
>>>>>> +@depends on patch@
>>>>>> +identifier tmp;
>>>>>> +expression a, b;
>>>>>> +type T;
>>>>>> +@@
>>>>>> +
>>>>>> +(
>>>>>> +- T tmp;
>>>>>> +|
>>>>>> +- T tmp = 0;
>>>>>> +|
>>>>>> +- T *tmp = NULL;
>>>>>> +)
>>>>>> +... when != tmp
>>>>>> +- tmp = a;
>>>>>> +- a = b;
>>>>>> +- b = tmp;
>>>>>> ++ swap(a, b);
>>>>>> +... when != tmp
>>>>>
>>>>> In this rule and the next one, if you remove the final ; from the b = tmp
>>>>> line and from the swap line, and put it into context code afterwards, them
>>>>> the generated code looks better on cases like fs/xfs/xfs_inode.c in the
>>>>> function xfs_lock_two_inodes where two successive swap calls are
>>>>> generated.
>>>>>
>>>>> There are also some cases such as drivers/net/wireless/ath/ath5k/phy.c in
>>>>> the function ath5k_hw_get_median_noise_floor where the swap code makes up
>>>>> a whole if branch.
>>>>
>>>>> In this cases it would be good to remove the {}.
>>>>
>>>> How this can be handled?
>>>>
>>>> If I use this pattern:
>>>> @depends on patch@
>>>> identifier tmp;
>>>> expression a, b;
>>>> @@
>>>>
>>>> (
>>>> if (...)
>>>> - {
>>>> - tmp = a;
>>>> - a = b;
>>>> - b = tmp
>>>> + swap(a, b)
>>>> ;
>>>> - }
>>>> |
>>>> - tmp = a;
>>>> - a = b;
>>>> - b = tmp
>>>> + swap(a, b)
>>>> ;
>>>> )
>>>>
>>>> The tool fails with error:
>>>>
>>>> EXN: Failure("rule starting on line 58: already tagged token:\nC code
>>>> context\nFile \"drivers/net/wireless/ath/ath5k/phy.c\", line 1574,
>>>> column 4, charpos = 41650\n around = 'sort',\n whole content =
>>>> \t\t\t\tsort[j - 1] = tmp;") in drivers/net/wireless/ath/ath5k/phy.c
>>>
>>> A disjunction basically says "at this node in the cfg, can I match the
>>> first patter, or can I match the second pattern, etc." Unfortunately in
>>> this case the two branches start matching at different nodes, so the short
>>> circuit aspect of a disjunction isn't used, and it matches both patterns.
>>>
>>> The solution is to just make two rules. The first for the if case and the
>>> second for everything else.
>>>
>>
>> if (...)
>> - {
>> - tmp = a;
>> - a = b;
>> - b = tmp
>> + swap(a, b)
>> ;
>> - }
>>
>>
>> This produces "single-line ifs".
>> Maybe generated patches can be improved somehow?
>> Moving -+; doesn't help in this case.
>
> There is clearly some problem with the management of newlines...
>
> The other alternative is to just have one rule for introducing swap and
> another for removing the braces around a swap, ie
>
> if (...)
> - {
> swap(...);
> - }
>
> I don't think it would be motivated to remove the newline in that case.
Yes, this works. I'll send v2.
Thanks
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH] coccinelle: misc: add swap script
Check for opencoded swap() implementation. Signed-off-by: Denis Efremov --- scripts/coccinelle/misc/swap.cocci | 77 ++ 1 file changed, 77 insertions(+) create mode 100644 scripts/coccinelle/misc/swap.cocci diff --git a/scripts/coccinelle/misc/swap.cocci b/scripts/coccinelle/misc/swap.cocci new file mode 100644 index ..38227a5d0855 --- /dev/null +++ b/scripts/coccinelle/misc/swap.cocci @@ -0,0 +1,77 @@ +// SPDX-License-Identifier: GPL-2.0-only +/// +/// Check for opencoded swap() implementation. +/// +// Confidence: High +// Copyright: (C) 2021 Denis Efremov ISPRAS +// Options: --no-includes --include-headers +// +// Keywords: swap +// + +virtual patch +virtual org +virtual report +virtual context + +@r depends on !patch@ +identifier tmp; +expression a, b; +type T; +position p; +@@ + +( +* T tmp; +| +* T tmp = 0; +| +* T *tmp = NULL; +) +... when != tmp +* tmp = a; +* a = b;@p +* b = tmp; +... when != tmp + +@depends on patch@ +identifier tmp; +expression a, b; +type T; +@@ + +( +- T tmp; +| +- T tmp = 0; +| +- T *tmp = NULL; +) +... when != tmp +- tmp = a; +- a = b; +- b = tmp; ++ swap(a, b); +... when != tmp + +@depends on patch@ +identifier tmp; +expression a, b; +@@ + +- tmp = a; +- a = b; +- b = tmp; ++ swap(a, b); + +@script:python depends on report@ +p << r.p; +@@ + +coccilib.report.print_report(p[0], "WARNING opportunity for swap()") + +@script:python depends on org@ +p << r.p; +@@ + +coccilib.org.print_todo(p[0], "WARNING opportunity for swap()") -- 2.26.2 ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Problem with partial patch generation
On 2/15/21 2:47 PM, Julia Lawall wrote: > > > On Mon, 15 Feb 2021, Denis Efremov wrote: > >> Hi, >> >> I wrote a simple rule (swap.cocci): >> >> virtual patch >> >> @depends on patch@ >> identifier tmp; >> expression a, b; >> type T; >> @@ >> >> ( >> - T tmp; >> | >> - T tmp = 0; >> | >> - T *tmp = NULL; >> ) >> ... when != tmp >> - tmp = a; >> - a = b; >> - b = tmp; >> + swap(a, b); >> ... when != tmp >> >> I would expect it to remove a local variable only if there is a match >> with swap template. >> >> However, it generates "partial" patch on 5.11 linux code: >> $ spatch --version >> spatch version 1.0.8-00201-g267f9cf8cc82 compiled with OCaml version 4.11.1 >> $ spatch -D patch --sp-file swap.cocci mm/filemap.c >> --- mm/filemap.c >> +++ /tmp/cocci-output-445786-88aa66-filemap.c >> @@ -2348,7 +2348,7 @@ static int generic_file_buffered_read_ge >> struct file_ra_state *ra = >f_ra; >> pgoff_t index = iocb->ki_pos >> PAGE_SHIFT; >> pgoff_t last_index = (iocb->ki_pos + iter->count + PAGE_SIZE-1) >> >> PAGE_SHIFT; >> - int i, j, nr_got, err = 0; >> + int i, nr_got, err = 0; >> >> nr = min_t(unsigned long, last_index - index, nr); >> find_page: >> >> How can I improve the rule? > > I don't get a match with the latest version of Coccinelle. > > If the latest version of Coccinelle were to become a release, would that > be good enough for you? Or do you need 1.0.8 to work as well? I planned to submit the swap rule to scripts/coccinelle. However, I can create more strict pattern for the patch mode. Maybe adding "when strict" is suitable here? ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] Problem with partial patch generation
Hi, I wrote a simple rule (swap.cocci): virtual patch @depends on patch@ identifier tmp; expression a, b; type T; @@ ( - T tmp; | - T tmp = 0; | - T *tmp = NULL; ) ... when != tmp - tmp = a; - a = b; - b = tmp; + swap(a, b); ... when != tmp I would expect it to remove a local variable only if there is a match with swap template. However, it generates "partial" patch on 5.11 linux code: $ spatch --version spatch version 1.0.8-00201-g267f9cf8cc82 compiled with OCaml version 4.11.1 $ spatch -D patch --sp-file swap.cocci mm/filemap.c --- mm/filemap.c +++ /tmp/cocci-output-445786-88aa66-filemap.c @@ -2348,7 +2348,7 @@ static int generic_file_buffered_read_ge struct file_ra_state *ra = >f_ra; pgoff_t index = iocb->ki_pos >> PAGE_SHIFT; pgoff_t last_index = (iocb->ki_pos + iter->count + PAGE_SIZE-1) >> PAGE_SHIFT; - int i, j, nr_got, err = 0; + int i, nr_got, err = 0; nr = min_t(unsigned long, last_index - index, nr); find_page: How can I improve the rule? Thanks, Denis ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Backward compatibility issue
On 2/12/21 5:04 PM, Julia Lawall wrote: > > > On Thu, 11 Feb 2021, Denis Efremov wrote: > >> Hi, one of my patterns started to fail tests on latest coccinelle. >> I've bisected the commit that introduces "error". It's: >> commit db60e916633d2cb3ae31140364783fdf85ed10f4 >> "make information about SmPL iterator and declarer names available to the C >> parser" >> >> To reproduce the error: >> $ cd linux >> $ git checkout 5b01014759991887b1e450c9def01e58c02ab81b >> $ wget >> https://raw.githubusercontent.com/evdenis/cvehound/master/cvehound/cve/CVE-2016-9793.cocci >> $ spatch -D detect --cocci-file CVE-2016-9793.cocci net/core/sock.c >> # spatch before db60e916633d2cb3ae31140364783fdf85ed10f4 will find the match >> net/core/sock.c:718:16-17: ERROR: CVE-2016-9793 >> net/core/sock.c:754:16-17: ERROR: CVE-2016-9793 >> ... >> # spatch >= db60e916633d2cb3ae31140364783fdf85ed10f4 will not match the same >> code > > If you change typedef u32 to symbol u32, it should be good. It is no use > to Coccinelle to know that u32 is a typedef in this code. After changing typedef to symbol, git version starts to match the code, but coccinelle 1.0.8 starts to fail the detection. Well, this doesn't solve the problem for me. What's the difference between symbol and typedef? How can I understand when to use one or another? Thanks, Denis ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] Backward compatibility issue
Hi, one of my patterns started to fail tests on latest coccinelle. I've bisected the commit that introduces "error". It's: commit db60e916633d2cb3ae31140364783fdf85ed10f4 "make information about SmPL iterator and declarer names available to the C parser" To reproduce the error: $ cd linux $ git checkout 5b01014759991887b1e450c9def01e58c02ab81b $ wget https://raw.githubusercontent.com/evdenis/cvehound/master/cvehound/cve/CVE-2016-9793.cocci $ spatch -D detect --cocci-file CVE-2016-9793.cocci net/core/sock.c # spatch before db60e916633d2cb3ae31140364783fdf85ed10f4 will find the match net/core/sock.c:718:16-17: ERROR: CVE-2016-9793 net/core/sock.c:754:16-17: ERROR: CVE-2016-9793 ... # spatch >= db60e916633d2cb3ae31140364783fdf85ed10f4 will not match the same code Thanks, Denis ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] How to match switch cases and their absence with coccinelle?
On 1/12/21 7:13 PM, Julia Lawall wrote:
>
>
> On Tue, 12 Jan 2021, Denis Efremov wrote:
>
>> Hi,
>>
>> Let's suppose I have this pattern:
>> @fix exists@
>> position p;
>> @@
>>
>> binder_release_work(...)
>> {
>> ...
>> switch (...) {
>> *case BINDER_WORK_NODE: ... break;@p
>> }
>> ...
>
> Add when any to the outer ...s
Thanks, this helped.
>
>> }
>>
>> and I want to match binder_release_work() function in
>> drivers/android/binder.c
>> file (linux kernel, master)
>>
>> Seems like the rule is not enough, it gives nothing:
>> $ spatch --cocci-file binder.cocci drivers/android/binder.c
>> init_defs_builtins: /usr/lib64/coccinelle/standard.h
>> HANDLING: drivers/android/binder.c
>>
>> 1) What can I do to reliable check that there is a special case in a switch?
>> 2) Is it possible to check that there is no case handling with something
>> like:
>> switch (...) {
>> ... when != case BINDER_WORK_NODE: ... break;
>> }
>
> I don't know if that will work. But you can do it with two rules. In the
> first rule, you could put a position variable on the switch, and then in
> the second rule, you could make a position variable that is required to be
> different than the first one, and that is also attached to a switch.
Yes, I use this method currently.
Also I faced the problem when I can't use ... in the beginning of enum, i.e.:
struct binder_work {
...
enum binder_work_type {
... // <== will not work
* BINDER_WORK_NODE,
...
} type;
...
}
This works:
struct binder_work {
...
enum binder_work_type {
// BINDER_WORK_TRANSACTION = ..., // also will not work
BINDER_WORK_TRANSACTION = 1,
BINDER_WORK_TRANSACTION_COMPLETE,
BINDER_WORK_RETURN_ERROR,
* BINDER_WORK_NODE,
...
} type;
...
}
Thanks,
Denis
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] How to match switch cases and their absence with coccinelle?
Hi,
Let's suppose I have this pattern:
@fix exists@
position p;
@@
binder_release_work(...)
{
...
switch (...) {
* case BINDER_WORK_NODE: ... break;@p
}
...
}
and I want to match binder_release_work() function in drivers/android/binder.c
file (linux kernel, master)
Seems like the rule is not enough, it gives nothing:
$ spatch --cocci-file binder.cocci drivers/android/binder.c
init_defs_builtins: /usr/lib64/coccinelle/standard.h
HANDLING: drivers/android/binder.c
1) What can I do to reliable check that there is a special case in a switch?
2) Is it possible to check that there is no case handling with something like:
switch (...) {
... when != case BINDER_WORK_NODE: ... break;
}
Thanks,
Denis
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] How to match function-like macro calls, e.g. RTA_ALIGN(rta->rta_len)?
On 1/11/21 11:59 PM, Julia Lawall wrote:
>
>
> On Mon, 11 Jan 2021, Denis Efremov wrote:
>
>>
>>
>> On 1/11/21 11:40 PM, Julia Lawall wrote:
>>>
>>>
>>> On Mon, 11 Jan 2021, Denis Efremov wrote:
>>>
>>>>
>>>>
>>>> On 1/11/21 11:23 PM, Julia Lawall wrote:
>>>>>
>>>>>
>>>>> On Mon, 11 Jan 2021, Denis Efremov wrote:
>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> Let's suppose I want to match otx_cpt_aead_cbc_aes_sha_setkey() function
>>>>>> from
>>>>>> drivers/crypto/marvell/octeontx/otx_cptvf_algs.c file (linux kernel).
>>>>>>
>>>>>> My pattern:
>>>>>> @exists@
>>>>>> identifier rta, param, key, keylen;
>>>>>> position p;
>>>>>> @@
>>>>>>
>>>>>> otx_cpt_aead_cbc_aes_sha_setkey(...,
>>>>>> unsigned char *key, unsigned int keylen)
>>>>>> {
>>>>>> ...
>>>>>> *if (RTA_PAYLOAD(rta) < sizeof(*param))@p
>>>>>> goto badkey;
>>>>>> ...
>>>>>> *key += RTA_ALIGN(rta->rta_len);
>>>>>> *keylen -= RTA_ALIGN(rta->rta_len);
>>>>>> ...
>>>>>> }
>>>>>>
>>>>>> $ spatch --no-includes --include-headers --cocci-file test.cocci
>>>>>> drivers/crypto/marvell/octeontx/otx_cptvf_algs.c
>>>>>> init_defs_builtins: /usr/lib64/coccinelle/standard.h
>>>>>> minus: parse error:
>>>>>> File "test.cocci", line 13, column 9, charpos = 219
>>>>>> around = 'RTA_ALIGN',
>>>>>> whole content = * key += RTA_ALIGN(rta->rta_len);
>>>>>>
>>>>>>
>>>>>> What can I do to match RTA_ALIGN(...) lines?
>>>>>
>>>>> I don't understand the problem. I took your rule and your command line,
>>>>> and everything was fine.
>>>>
>>>> I use version:
>>>> spatch version 1.0.8-gc1dbb4f-dirty compiled with OCaml version 4.11.1
>>>> Flags passed to the configure script: --build=x86_64-redhat-linux-gnu
>>>> --host=x86_64-redhat-linux-gnu --program-prefix=
>>>> --disable-dependency-tracking --prefix=/usr --exec-prefix=/usr
>>>> --bindir=/usr/bin --sbindir=/usr/sbin --sysconfdir=/etc
>>>> --datadir=/usr/share --includedir=/usr/include --libdir=/usr/lib64
>>>> --libexecdir=/usr/libexec --localstatedir=/var --sharedstatedir=/var/lib
>>>> --mandir=/usr/share/man --infodir=/usr/share/info
>>>> --with-python=/usr/bin/python3 --with-menhir=/usr/bin/menhir
>>>> OCaml scripting support: yes
>>>> Python scripting support: yes
>>>> Syntax of regular expressions: PCRE
>>>>
>>>> Maybe parsing error is fixed in new versions?
>>>
>>> I can't imagine what would cause a parsing error here. I don't see what
>>> could be special about RTA_ALIGN. Normally Coccinelle doesn't even know
>>> that it is a macro. Maybe you can try to simplify the semantic patch a
>>> little bit and see if there is some change that causes the problem to
>>> disappear? Does the problem disappear if you use a name other than
>>> RTA_ALIGN?
>>
>> Changing:
>> key += RTA_ALIGN(rta->rta_len);
>> to:
>> key = RTA_ALIGN(rta->rta_len);
>>
>> makes the parsing error disappear.
>>
>> Using aop instead of += leads to the same parsing error:
>> assignment operator aop;
>> key aop RTA_ALIGN(rta->rta_len);
>> init_defs_builtins: /usr/lib64/coccinelle/standard.h
>> minus: parse error:
>> File "test.cocci", line 14, column 10, charpos = 245
>> around = 'RTA_ALIGN',
>> whole content = * key aop RTA_ALIGN(rta->rta_len);
>
> I assume not, but does spatch --parse-cocci test.cocci work?
Works with "key =" pattern, with "key +=" or "key aop" doesn't.
The errors are the same:
spatch --parse-cocci test.cocci
init_defs_builtins: /usr/lib64/coccinelle/standard.h
minus: parse error:
File "test.cocci", line 13, column 9, charpos = 219
around = 'RTA_ALIGN',
whole content = * key += RTA_ALIGN(rta->rta_len);
Denis
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] How to match function-like macro calls, e.g. RTA_ALIGN(rta->rta_len)?
On 1/11/21 11:40 PM, Julia Lawall wrote:
>
>
> On Mon, 11 Jan 2021, Denis Efremov wrote:
>
>>
>>
>> On 1/11/21 11:23 PM, Julia Lawall wrote:
>>>
>>>
>>> On Mon, 11 Jan 2021, Denis Efremov wrote:
>>>
>>>> Hi,
>>>>
>>>> Let's suppose I want to match otx_cpt_aead_cbc_aes_sha_setkey() function
>>>> from
>>>> drivers/crypto/marvell/octeontx/otx_cptvf_algs.c file (linux kernel).
>>>>
>>>> My pattern:
>>>> @exists@
>>>> identifier rta, param, key, keylen;
>>>> position p;
>>>> @@
>>>>
>>>> otx_cpt_aead_cbc_aes_sha_setkey(...,
>>>>unsigned char *key, unsigned int keylen)
>>>> {
>>>>...
>>>> * if (RTA_PAYLOAD(rta) < sizeof(*param))@p
>>>>goto badkey;
>>>>...
>>>> * key += RTA_ALIGN(rta->rta_len);
>>>> * keylen -= RTA_ALIGN(rta->rta_len);
>>>>...
>>>> }
>>>>
>>>> $ spatch --no-includes --include-headers --cocci-file test.cocci
>>>> drivers/crypto/marvell/octeontx/otx_cptvf_algs.c
>>>> init_defs_builtins: /usr/lib64/coccinelle/standard.h
>>>> minus: parse error:
>>>> File "test.cocci", line 13, column 9, charpos = 219
>>>> around = 'RTA_ALIGN',
>>>> whole content = * key += RTA_ALIGN(rta->rta_len);
>>>>
>>>>
>>>> What can I do to match RTA_ALIGN(...) lines?
>>>
>>> I don't understand the problem. I took your rule and your command line,
>>> and everything was fine.
>>
>> I use version:
>> spatch version 1.0.8-gc1dbb4f-dirty compiled with OCaml version 4.11.1
>> Flags passed to the configure script: --build=x86_64-redhat-linux-gnu
>> --host=x86_64-redhat-linux-gnu --program-prefix=
>> --disable-dependency-tracking --prefix=/usr --exec-prefix=/usr
>> --bindir=/usr/bin --sbindir=/usr/sbin --sysconfdir=/etc --datadir=/usr/share
>> --includedir=/usr/include --libdir=/usr/lib64 --libexecdir=/usr/libexec
>> --localstatedir=/var --sharedstatedir=/var/lib --mandir=/usr/share/man
>> --infodir=/usr/share/info --with-python=/usr/bin/python3
>> --with-menhir=/usr/bin/menhir
>> OCaml scripting support: yes
>> Python scripting support: yes
>> Syntax of regular expressions: PCRE
>>
>> Maybe parsing error is fixed in new versions?
>
> I can't imagine what would cause a parsing error here. I don't see what
> could be special about RTA_ALIGN. Normally Coccinelle doesn't even know
> that it is a macro. Maybe you can try to simplify the semantic patch a
> little bit and see if there is some change that causes the problem to
> disappear? Does the problem disappear if you use a name other than
> RTA_ALIGN?
Changing:
key += RTA_ALIGN(rta->rta_len);
to:
key = RTA_ALIGN(rta->rta_len);
makes the parsing error disappear.
Using aop instead of += leads to the same parsing error:
assignment operator aop;
key aop RTA_ALIGN(rta->rta_len);
init_defs_builtins: /usr/lib64/coccinelle/standard.h
minus: parse error:
File "test.cocci", line 14, column 10, charpos = 245
around = 'RTA_ALIGN',
whole content = * key aop RTA_ALIGN(rta->rta_len);
Denis
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] How to match function-like macro calls, e.g. RTA_ALIGN(rta->rta_len)?
On 1/11/21 11:23 PM, Julia Lawall wrote:
>
>
> On Mon, 11 Jan 2021, Denis Efremov wrote:
>
>> Hi,
>>
>> Let's suppose I want to match otx_cpt_aead_cbc_aes_sha_setkey() function from
>> drivers/crypto/marvell/octeontx/otx_cptvf_algs.c file (linux kernel).
>>
>> My pattern:
>> @exists@
>> identifier rta, param, key, keylen;
>> position p;
>> @@
>>
>> otx_cpt_aead_cbc_aes_sha_setkey(...,
>> unsigned char *key, unsigned int keylen)
>> {
>> ...
>> *if (RTA_PAYLOAD(rta) < sizeof(*param))@p
>> goto badkey;
>> ...
>> *key += RTA_ALIGN(rta->rta_len);
>> *keylen -= RTA_ALIGN(rta->rta_len);
>> ...
>> }
>>
>> $ spatch --no-includes --include-headers --cocci-file test.cocci
>> drivers/crypto/marvell/octeontx/otx_cptvf_algs.c
>> init_defs_builtins: /usr/lib64/coccinelle/standard.h
>> minus: parse error:
>> File "test.cocci", line 13, column 9, charpos = 219
>> around = 'RTA_ALIGN',
>> whole content = * key += RTA_ALIGN(rta->rta_len);
>>
>>
>> What can I do to match RTA_ALIGN(...) lines?
>
> I don't understand the problem. I took your rule and your command line,
> and everything was fine.
I use version:
spatch version 1.0.8-gc1dbb4f-dirty compiled with OCaml version 4.11.1
Flags passed to the configure script: --build=x86_64-redhat-linux-gnu
--host=x86_64-redhat-linux-gnu --program-prefix= --disable-dependency-tracking
--prefix=/usr --exec-prefix=/usr --bindir=/usr/bin --sbindir=/usr/sbin
--sysconfdir=/etc --datadir=/usr/share --includedir=/usr/include
--libdir=/usr/lib64 --libexecdir=/usr/libexec --localstatedir=/var
--sharedstatedir=/var/lib --mandir=/usr/share/man --infodir=/usr/share/info
--with-python=/usr/bin/python3 --with-menhir=/usr/bin/menhir
OCaml scripting support: yes
Python scripting support: yes
Syntax of regular expressions: PCRE
Maybe parsing error is fixed in new versions?
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] How to match function-like macro calls, e.g. RTA_ALIGN(rta->rta_len)?
Hi,
Let's suppose I want to match otx_cpt_aead_cbc_aes_sha_setkey() function from
drivers/crypto/marvell/octeontx/otx_cptvf_algs.c file (linux kernel).
My pattern:
@exists@
identifier rta, param, key, keylen;
position p;
@@
otx_cpt_aead_cbc_aes_sha_setkey(...,
unsigned char *key, unsigned int keylen)
{
...
* if (RTA_PAYLOAD(rta) < sizeof(*param))@p
goto badkey;
...
* key += RTA_ALIGN(rta->rta_len);
* keylen -= RTA_ALIGN(rta->rta_len);
...
}
$ spatch --no-includes --include-headers --cocci-file test.cocci
drivers/crypto/marvell/octeontx/otx_cptvf_algs.c
init_defs_builtins: /usr/lib64/coccinelle/standard.h
minus: parse error:
File "test.cocci", line 13, column 9, charpos = 219
around = 'RTA_ALIGN',
whole content = * key += RTA_ALIGN(rta->rta_len);
What can I do to match RTA_ALIGN(...) lines?
Thanks,
Denis
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH v8] coccinelle: api: add kfree_mismatch script
Check that alloc and free types of functions match each other.
Signed-off-by: Denis Efremov
---
Changes in v2:
- Lines are limited to 80 characters where possible
- Confidence changed from High to Medium because of
fs/btrfs/send.c:1119 false-positive
- __vmalloc_area_node() explicitly excluded from analysis
instead of !(file in "mm/vmalloc.c") condition
Changes in v3:
- prints style in org && report modes changed for python2
Changes in v4:
- missing msg argument to print_todo fixed
Changes in v5:
- fix position p in kfree rule
- move @kok and @v positions in choice rule after the arguments
- remove kvmalloc suggestions
Changes in v6:
- more asterisks added in context mode
- second @kok added to the choice rule
Changes in v7:
- file renamed to kfree_mismatch.cocci
- python function relevant() removed
- additional rule for filtering free positions added
- btrfs false-positive fixed
- confidence level changed to high
- kvfree_switch rule added
- names for position variables changed to @a (alloc) and @f (free)
Changes in v8:
- kzfree() replaced with kfree_sensitive()
- "position f != free.fok;" simplified to "position f;" in patch
and kvfree_switch rules
scripts/coccinelle/api/kfree_mismatch.cocci | 229
1 file changed, 229 insertions(+)
create mode 100644 scripts/coccinelle/api/kfree_mismatch.cocci
diff --git a/scripts/coccinelle/api/kfree_mismatch.cocci
b/scripts/coccinelle/api/kfree_mismatch.cocci
new file mode 100644
index ..843b794fac7b
--- /dev/null
+++ b/scripts/coccinelle/api/kfree_mismatch.cocci
@@ -0,0 +1,229 @@
+// SPDX-License-Identifier: GPL-2.0-only
+///
+/// Check that kvmalloc'ed memory is freed by kfree functions,
+/// vmalloc'ed by vfree functions and kvmalloc'ed by kvfree
+/// functions.
+///
+// Confidence: High
+// Copyright: (C) 2020 Denis Efremov ISPRAS
+// Options: --no-includes --include-headers
+//
+
+virtual patch
+virtual report
+virtual org
+virtual context
+
+@alloc@
+expression E, E1;
+position kok, vok;
+@@
+
+(
+ if (...) {
+...
+E = \(kmalloc\|kzalloc\|krealloc\|kcalloc\|
+ kmalloc_node\|kzalloc_node\|kmalloc_array\|
+ kmalloc_array_node\|kcalloc_node\)(...)@kok
+...
+ } else {
+...
+E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|
+ vzalloc_node\|vmalloc_exec\|vmalloc_32\|
+ vmalloc_32_user\|__vmalloc\|__vmalloc_node_range\|
+ __vmalloc_node\)(...)@vok
+...
+ }
+|
+ E = \(kmalloc\|kzalloc\|krealloc\|kcalloc\|kmalloc_node\|kzalloc_node\|
+kmalloc_array\|kmalloc_array_node\|kcalloc_node\)(...)@kok
+ ... when != E = E1
+ when any
+ if (E == NULL) {
+...
+E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|
+ vzalloc_node\|vmalloc_exec\|vmalloc_32\|
+ vmalloc_32_user\|__vmalloc\|__vmalloc_node_range\|
+ __vmalloc_node\)(...)@vok
+...
+ }
+)
+
+@free@
+expression E;
+position fok;
+@@
+
+ E = \(kvmalloc\|kvzalloc\|kvcalloc\|kvzalloc_node\|kvmalloc_node\|
+kvmalloc_array\)(...)
+ ...
+ kvfree(E)@fok
+
+@vfree depends on !patch@
+expression E;
+position a != alloc.kok;
+position f != free.fok;
+@@
+
+* E = \(kmalloc\|kzalloc\|krealloc\|kcalloc\|kmalloc_node\|
+* kzalloc_node\|kmalloc_array\|kmalloc_array_node\|
+* kcalloc_node\)(...)@a
+ ... when != if (...) { ... E =
\(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\|vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\|__vmalloc_node_range\|__vmalloc_node\)(...);
... }
+ when != is_vmalloc_addr(E)
+ when any
+* \(vfree\|vfree_atomic\|kvfree\)(E)@f
+
+@depends on patch exists@
+expression E;
+position a != alloc.kok;
+position f != free.fok;
+@@
+
+ E = \(kmalloc\|kzalloc\|krealloc\|kcalloc\|kmalloc_node\|
+kzalloc_node\|kmalloc_array\|kmalloc_array_node\|
+kcalloc_node\)(...)@a
+ ... when != if (...) { ... E =
\(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\|vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\|__vmalloc_node_range\|__vmalloc_node\)(...);
... }
+ when != is_vmalloc_addr(E)
+ when any
+- \(vfree\|vfree_atomic\|kvfree\)(E)@f
++ kfree(E)
+
+@kfree depends on !patch@
+expression E;
+position a != alloc.vok;
+position f != free.fok;
+@@
+
+* E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\|
+* vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\|
+* __vmalloc_node_range\|__vmalloc_node\)(...)@a
+ ... when != is_vmalloc_addr(E)
+ when any
+* \(kfree\|kfree_sensitive\|kvfree\)(E)@f
+
+@depends on patch exists@
+expression E;
+position a != alloc.vok;
+position f != free.fok;
+@@
+
+ E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\|
+vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\|
+__vmalloc_node_range\|__vmalloc_node\)(...)@a
+ ... when != is_vmalloc_addr(E)
+ when any
+- \(kfree\|kvfree\)(E)@f
++ vfree(E)
+
+@k
[Cocci] [PATCH] coccinelle: api: kfree_sensitive: print memset position
Print memset() call position in addition to the kfree() position to ease issues identification. Signed-off-by: Denis Efremov --- scripts/coccinelle/api/kfree_sensitive.cocci | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/scripts/coccinelle/api/kfree_sensitive.cocci b/scripts/coccinelle/api/kfree_sensitive.cocci index e4a066a0b77d..8d980ebf3223 100644 --- a/scripts/coccinelle/api/kfree_sensitive.cocci +++ b/scripts/coccinelle/api/kfree_sensitive.cocci @@ -85,14 +85,16 @@ type T; @script:python depends on report@ p << r.p; +m << r.m; @@ -coccilib.report.print_report(p[0], - "WARNING: opportunity for kfree_sensitive/kvfree_sensitive") +msg = "WARNING opportunity for kfree_sensitive/kvfree_sensitive (memset at line %s)" +coccilib.report.print_report(p[0], msg % (m[0].line)) @script:python depends on org@ p << r.p; +m << r.m; @@ -coccilib.org.print_todo(p[0], - "WARNING: opportunity for kfree_sensitive/kvfree_sensitive") +msg = "WARNING opportunity for kfree_sensitive/kvfree_sensitive (memset at line %s)" +coccilib.org.print_todo(p[0], msg % (m[0].line)) -- 2.26.2 ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] kzfree script
On 10/2/20 5:13 PM, Julia Lawall wrote: > > > On Fri, 2 Oct 2020, Denis Efremov wrote: > >> Hi, >> >> On 10/2/20 5:01 PM, Julia Lawall wrote: >>> Denis, >>> >>> In the rule proposing kzfree_sensitive, I think it would be helpful to >>> also highlight the memset line. >> >> What do you mean? It's "highlighted" in context mode. Do you mean adding >> position argument to memset call and showing this position in the warning >> messages? > > Yes, that seems to be what I mean. 0-day generated a message from the > script, and I had to hunt around for the reason why it was doing that. So > it would be nice to have the memset highlighted. It seems that the > non-patch 0-day messages are generated from the report mode. > Ok, I will send a patch for it. Thanks, Denis ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] kzfree script
Hi, On 10/2/20 5:01 PM, Julia Lawall wrote: > Denis, > > In the rule proposing kzfree_sensitive, I think it would be helpful to > also highlight the memset line. What do you mean? It's "highlighted" in context mode. Do you mean adding position argument to memset call and showing this position in the warning messages? Thanks, Denis ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH v4] coccinelle: api: add kvmalloc script
Suggest kvmalloc, kvfree instead of opencoded patterns.
Signed-off-by: Denis Efremov
---
Changes in v2:
- binary operator cmp added
- NULL comparisions simplified
- "T x" case added to !patch mode
Changes in v3:
- kvfree rules added
Changes in v4:
- pattern updated to match only GFP_KERNEL/__GFP_NOWARN flags
to avoid possible false-positives
All patches are sent:
[1] https://lore.kernel.org/patchwork/patch/1296428/
[2] https://lore.kernel.org/patchwork/patch/1296636/
[3] https://lore.kernel.org/patchwork/patch/1282895/
[4] https://lore.kernel.org/patchwork/patch/1296631/
scripts/coccinelle/api/kvmalloc.cocci | 256 ++
1 file changed, 256 insertions(+)
create mode 100644 scripts/coccinelle/api/kvmalloc.cocci
diff --git a/scripts/coccinelle/api/kvmalloc.cocci
b/scripts/coccinelle/api/kvmalloc.cocci
new file mode 100644
index ..c30dab718a49
--- /dev/null
+++ b/scripts/coccinelle/api/kvmalloc.cocci
@@ -0,0 +1,256 @@
+// SPDX-License-Identifier: GPL-2.0-only
+///
+/// Find if/else condition with kmalloc/vmalloc calls.
+/// Suggest to use kvmalloc instead. Same for kvfree.
+///
+// Confidence: High
+// Copyright: (C) 2020 Denis Efremov ISPRAS
+// Options: --no-includes --include-headers
+//
+
+virtual patch
+virtual report
+virtual org
+virtual context
+
+@initialize:python@
+@@
+filter = frozenset(['kvfree'])
+
+def relevant(p):
+return not (filter & {el.current_element for el in p})
+
+@kvmalloc depends on !patch@
+expression E, E1, size;
+identifier flags;
+binary operator cmp = {<=, <, ==, >, >=};
+identifier x;
+type T;
+position p;
+@@
+
+(
+* if (size cmp E1 || ...)@p {
+...
+*E = \(kmalloc\|kzalloc\|kcalloc\|kmalloc_node\|kzalloc_node\|
+* kmalloc_array\|kmalloc_array_node\|kcalloc_node\)
+* (..., size,
\(flags\|GFP_KERNEL\|\(GFP_KERNEL\|flags\)|__GFP_NOWARN\), ...)
+...
+ } else {
+...
+*E = \(vmalloc\|vzalloc\|vmalloc_node\|vzalloc_node\)(..., size, ...)
+...
+ }
+|
+* E = \(kmalloc\|kzalloc\|kcalloc\|kmalloc_node\|kzalloc_node\|
+* kmalloc_array\|kmalloc_array_node\|kcalloc_node\)
+* (..., size, \(flags\|GFP_KERNEL\|\(GFP_KERNEL\|flags\)|__GFP_NOWARN\),
...)
+ ... when != E = E1
+ when != size = E1
+ when any
+* if (E == NULL)@p {
+...
+* E = \(vmalloc\|vzalloc\|vmalloc_node\|vzalloc_node\)(..., size, ...)
+...
+ }
+|
+* T x = \(kmalloc\|kzalloc\|kcalloc\|kmalloc_node\|kzalloc_node\|
+* kmalloc_array\|kmalloc_array_node\|kcalloc_node\)
+* (..., size,
\(flags\|GFP_KERNEL\|\(GFP_KERNEL\|flags\)|__GFP_NOWARN\), ...);
+ ... when != x = E1
+ when != size = E1
+ when any
+* if (x == NULL)@p {
+...
+* x = \(vmalloc\|vzalloc\|vmalloc_node\|vzalloc_node\)(..., size, ...)
+...
+ }
+)
+
+@kvfree depends on !patch@
+expression E;
+position p : script:python() { relevant(p) };
+@@
+
+* if (is_vmalloc_addr(E))@p {
+...
+* vfree(E)
+...
+ } else {
+... when != krealloc(E, ...)
+when any
+* \(kfree\|kzfree\)(E)
+...
+ }
+
+@depends on patch@
+expression E, E1, size, node;
+binary operator cmp = {<=, <, ==, >, >=};
+identifier flags, x;
+type T;
+@@
+
+(
+- if (size cmp E1)
+-E = kmalloc(size, flags);
+- else
+-E = vmalloc(size);
++ E = kvmalloc(size, flags);
+|
+- if (size cmp E1)
+-E = kmalloc(size, \(GFP_KERNEL\|GFP_KERNEL|__GFP_NOWARN\));
+- else
+-E = vmalloc(size);
++ E = kvmalloc(size, GFP_KERNEL);
+|
+- E = kmalloc(size, flags | __GFP_NOWARN);
+- if (E == NULL)
+- E = vmalloc(size);
++ E = kvmalloc(size, flags);
+|
+- E = kmalloc(size, \(GFP_KERNEL\|GFP_KERNEL|__GFP_NOWARN\));
+- if (E == NULL)
+- E = vmalloc(size);
++ E = kvmalloc(size, GFP_KERNEL);
+|
+- T x = kmalloc(size, flags | __GFP_NOWARN);
+- if (x == NULL)
+- x = vmalloc(size);
++ T x = kvmalloc(size, flags);
+|
+- T x = kmalloc(size, \(GFP_KERNEL\|GFP_KERNEL|__GFP_NOWARN\));
+- if (x == NULL)
+- x = vmalloc(size);
++ T x = kvmalloc(size, GFP_KERNEL);
+|
+- if (size cmp E1)
+-E = kzalloc(size, flags);
+- else
+-E = vzalloc(size);
++ E = kvzalloc(size, flags);
+|
+- if (size cmp E1)
+-E = kzalloc(size, \(GFP_KERNEL\|GFP_KERNEL|__GFP_NOWARN\));
+- else
+-E = vzalloc(size);
++ E = kvzalloc(size, GFP_KERNEL);
+|
+- E = kzalloc(size, flags | __GFP_NOWARN);
+- if (E == NULL)
+- E = vzalloc(size);
++ E = kvzalloc(size, flags);
+|
+- E = kzalloc(size, \(GFP_KERNEL\|GFP_KERNEL|__GFP_NOWARN\));
+- if (E == NULL)
+- E = vzalloc(size);
++ E = kvzalloc(size, GFP_KERNEL);
+|
+- T x = kzalloc(size, flags | __GFP_NOWARN);
+- if (x == NULL)
+- x = vzalloc(size);
++ T x = kvzalloc(size, flags);
+|
+- T x = kzalloc(size, \(GFP_KERNEL\|GFP_KERNEL|__GFP_NOWARN\));
+- if (x == NULL)
+- x = vzalloc(size);
++ T x = kvzalloc(size, GFP_KERNEL);
+|
+- if (size cmp E1)
+-E = kmalloc_node(size, flags, node);
+- else
+-E = vmalloc_node(size, node);
++ E = kvmalloc_node(size, flags,
[Cocci] [PATCH v3] coccinelle: misc: add flexible_array.cocci script
One-element and zero-length arrays are deprecated [1]. Kernel
code should always use "flexible array members" instead, except
for existing uapi definitions.
The script warns about one-element and zero-length arrays in structs.
[1] commit 68e4cd17e218 ("docs: deprecated.rst: Add zero-length and
one-element arrays")
Cc: Kees Cook
Cc: Gustavo A. R. Silva
Signed-off-by: Denis Efremov
---
Changes in v2:
- all uapi headers are now filtered-out. Unfortunately, coccinelle
doesn't provide structure names in Location.current_element.
For structures the field is always "something_else". Thus, there is
no easy way to create a list of existing structures in uapi headers
and suppress the warning only for them, but not for the newly added
uapi structures.
- The pattern doesn't require 2+ fields in a structure/union anymore.
Now it also checks single field structures/unions.
- The pattern simplified and now uses disjuction in array elements
(Thanks, Markus)
- Unions are removed from patch mode
- one-element arrays are removed from patch mode. Correct patch may
involve turning the array to a simple field instead of a flexible
array.
Changes in v3:
- exists removed from "depends on patch"
- position argument fixed in org mode
- link to the online documentation added to the warning message
scripts/coccinelle/misc/flexible_array.cocci | 88
1 file changed, 88 insertions(+)
create mode 100644 scripts/coccinelle/misc/flexible_array.cocci
diff --git a/scripts/coccinelle/misc/flexible_array.cocci
b/scripts/coccinelle/misc/flexible_array.cocci
new file mode 100644
index ..947fbaff82a9
--- /dev/null
+++ b/scripts/coccinelle/misc/flexible_array.cocci
@@ -0,0 +1,88 @@
+// SPDX-License-Identifier: GPL-2.0-only
+///
+/// Zero-length and one-element arrays are deprecated, see
+/// Documentation/process/deprecated.rst
+/// Flexible-array members should be used instead.
+///
+//
+// Confidence: High
+// Copyright: (C) 2020 Denis Efremov ISPRAS.
+// Comments:
+// Options: --no-includes --include-headers
+
+virtual context
+virtual report
+virtual org
+virtual patch
+
+@initialize:python@
+@@
+def relevant(positions):
+for p in positions:
+if "uapi" in p.file:
+ return False
+return True
+
+@r depends on !patch@
+identifier name, array;
+type T;
+position p : script:python() { relevant(p) };
+@@
+
+(
+ struct name {
+...
+* T array@p[\(0\|1\)];
+ };
+|
+ struct {
+...
+* T array@p[\(0\|1\)];
+ };
+|
+ union name {
+...
+* T array@p[\(0\|1\)];
+ };
+|
+ union {
+...
+* T array@p[\(0\|1\)];
+ };
+)
+
+@depends on patch@
+identifier name, array;
+type T;
+position p : script:python() { relevant(p) };
+@@
+
+(
+ struct name {
+...
+T array@p[
+- 0
+];
+ };
+|
+ struct {
+...
+T array@p[
+- 0
+];
+ };
+)
+
+@script: python depends on report@
+p << r.p;
+@@
+
+msg = "WARNING use flexible-array member instead
(https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)"
+coccilib.report.print_report(p[0], msg)
+
+@script: python depends on org@
+p << r.p;
+@@
+
+msg = "WARNING use flexible-array member instead
(https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)"
+coccilib.org.print_todo(p[0], msg)
--
2.26.2
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH v7] coccinelle: api: add kfree_mismatch script
Hi, On 8/3/20 9:34 PM, Denis Efremov wrote: > Check that alloc and free types of functions match each other. Julia, I've just send the patches to fix all the warnings emitted by the script. [1] https://lore.kernel.org/patchwork/patch/1309731/ [2] https://lore.kernel.org/patchwork/patch/1309273/ [3] https://lore.kernel.org/patchwork/patch/1309275/ Other inconsistencies and bugs detected by this script: 1e814d630fd1 drm/amd/display: Use kfree() to free rgb_user in calculate_user_regamma_ramp() 842540075974 drm/amd/display: Use kvfree() to free coeff in build_regamma() f5e383ac8b58 iommu/pamu: Use kzfree() in fsl_pamu_probe() 36b26e37 net/mlx5: Use kfree(ft->g) in arfs_create_groups() 114427b8927a drm/panfrost: Use kvfree() to free bo->sgts 742532d11d83 f2fs: use kfree() instead of kvfree() to free superblock data 47a357de2b6b net/mlx5: DR, Fix freeing in dr_create_rc_qp() a8c73c1a614f io_uring: use kvfree() in io_sqe_buffer_register() 7f89cc07d22a cxgb4: Use kfree() instead kvfree() where appropriate bb2359f4dbe9 bpf: Change kvfree to kfree in generic_map_lookup_batch() > Changes in v2: > - Lines are limited to 80 characters where possible > - Confidence changed from High to Medium because of >fs/btrfs/send.c:1119 false-positive > - __vmalloc_area_node() explicitly excluded from analysis >instead of !(file in "mm/vmalloc.c") condition > Changes in v3: > - prints style in org && report modes changed for python2 > Changes in v4: > - missing msg argument to print_todo fixed > Changes in v5: > - fix position p in kfree rule > - move @kok and @v positions in choice rule after the arguments > - remove kvmalloc suggestions > Changes in v6: > - more asterisks added in context mode > - second @kok added to the choice rule > Changes in v7: > - file renamed to kfree_mismatch.cocci > - python function relevant() removed > - additional rule for filtering free positions added > - btrfs false-positive fixed > - confidence level changed to high > - kvfree_switch rule added > - names for position variables changed to @a (alloc) and @f (free) Is there something I can improve in this cocci script to be accepted? Thanks, Denis ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH v2] coccinelle: misc: add excluded_middle.cocci script
Check for !A || A && B condition. It's equivalent to !A || B. Signed-off-by: Denis Efremov --- Changes in v2: - spelling mistake fixed - position variable moved on the && operator - patch pattern changed to - (A && B) - word "condition" removed from warning message scripts/coccinelle/misc/excluded_middle.cocci | 39 +++ 1 file changed, 39 insertions(+) create mode 100644 scripts/coccinelle/misc/excluded_middle.cocci diff --git a/scripts/coccinelle/misc/excluded_middle.cocci b/scripts/coccinelle/misc/excluded_middle.cocci new file mode 100644 index ..ab28393e4843 --- /dev/null +++ b/scripts/coccinelle/misc/excluded_middle.cocci @@ -0,0 +1,39 @@ +// SPDX-License-Identifier: GPL-2.0-only +/// +/// Condition !A || A && B is equivalent to !A || B. +/// +// Confidence: High +// Copyright: (C) 2020 Denis Efremov ISPRAS +// Options: --no-includes --include-headers + +virtual patch +virtual context +virtual org +virtual report + +@r depends on !patch@ +expression A, B; +position p; +@@ + +* !A || (A &&@p B) + +@depends on patch@ +expression A, B; +@@ + + !A || +- (A && B) ++ B + +@script:python depends on report@ +p << r.p; +@@ + +coccilib.report.print_report(p[0], "WARNING !A || A && B is equivalent to !A || B") + +@script:python depends on org@ +p << r.p; +@@ + +coccilib.org.print_todo(p[0], "WARNING !A || A && B is equivalent to !A || B") -- 2.26.2 ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH v2] coccinelle: misc: add flexible_array.cocci script
Hi,
On 9/12/20 5:43 PM, Julia Lawall wrote:
>
>
> On Mon, 10 Aug 2020, Denis Efremov wrote:
>
>> Commit 68e4cd17e218 ("docs: deprecated.rst: Add zero-length and one-element
>> arrays") marks one-element and zero-length arrays as deprecated. Kernel
>> code should always use "flexible array members" instead.
>>
>> The script warns about one-element and zero-length arrays in structs.
>>
>> Cc: Kees Cook
>> Cc: Gustavo A. R. Silva
>> Signed-off-by: Denis Efremov
>> ---
>> Changes in v2:
>> - all uapi headers are now filtered-out. Unfortunately, coccinelle
>>doesn't provide structure names in Location.current_element.
>>For structures the field is always "something_else". Thus, there is
>>no easy way to create a list of existing structures in uapi headers
>>and suppress the warning only for them, but not for the newly added
>>uapi structures.
>> - The pattern doesn't require 2+ fields in a structure/union anymore.
>>Now it also checks single field structures/unions.
>> - The pattern simplified and now uses disjuction in array elements
>>(Thanks, Markus)
>> - Unions are removed from patch mode
>> - one-element arrays are removed from patch mode. Correct patch may
>>involve turning the array to a simple field instead of a flexible
>>array.
>>
>> On the current master branch, the rule generates:
>> - context: https://gist.github.com/evdenis/e2b4323491f9eff35376372df07f723c
>> - patch: https://gist.github.com/evdenis/46081da9d68ecefd07edc3769cebcf32
>>
>> scripts/coccinelle/misc/flexible_array.cocci | 88
>> 1 file changed, 88 insertions(+)
>> create mode 100644 scripts/coccinelle/misc/flexible_array.cocci
>>
>> diff --git a/scripts/coccinelle/misc/flexible_array.cocci
>> b/scripts/coccinelle/misc/flexible_array.cocci
>> new file mode 100644
>> index ..bf6dcda1783e
>> --- /dev/null
>> +++ b/scripts/coccinelle/misc/flexible_array.cocci
>> @@ -0,0 +1,88 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +///
>> +/// Zero-length and one-element arrays are deprecated, see
>> +/// Documentation/process/deprecated.rst
>> +/// Flexible-array members should be used instead.
>> +///
>> +//
>> +// Confidence: High
>> +// Copyright: (C) 2020 Denis Efremov ISPRAS.
>> +// Comments:
>> +// Options: --no-includes --include-headers
>> +
>> +virtual context
>> +virtual report
>> +virtual org
>> +virtual patch
>> +
>> +@initialize:python@
>> +@@
>> +def relevant(positions):
>> +for p in positions:
>> +if "uapi" in p.file:
>> + return False
>> +return True
>> +
>> +@r depends on !patch@
>> +identifier name, array;
>> +type T;
>> +position p : script:python() { relevant(p) };
>> +@@
>> +
>> +(
>> + struct name {
>> +...
>> +* T array@p[\(0\|1\)];
>> + };
>> +|
>> + struct {
>> +...
>> +* T array@p[\(0\|1\)];
>> + };
>> +|
>> + union name {
>> +...
>> +* T array@p[\(0\|1\)];
>> + };
>> +|
>> + union {
>> +...
>> +* T array@p[\(0\|1\)];
>> + };
>> +)
>> +
>> +@depends on patch exists@
>
> exists is not necessary here. There are not multiple control-flow paths
> through a structure declaration.
>
>> +identifier name, array;
>> +type T;
>> +position p : script:python() { relevant(p) };
>> +@@
>> +
>> +(
>> + struct name {
>> +...
>> +T array@p[
>> +- 0
>> +];
>> + };
>> +|
>> + struct {
>> +...
>> +T array@p[
>> +- 0
>> +];
>> + };
>> +)
>> +
>> +@script: python depends on report@
>> +p << r.p;
>> +@@
>> +
>> +msg = "WARNING: use flexible-array member instead"
>> +coccilib.report.print_report(p[0], msg)
>> +
>> +@script: python depends on org@
>> +p << r.p;
>> +@@
>> +
>> +msg = "WARNING: use flexible-array member instead"
>> +coccilib.org.print_todo(p, msg)
>
> This should be coccilib.org.print_todo(p[0], msg)
>
Thanks, I will send v3 with fixes and proper links to online documentation.
Regards,
Denis
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH] scripts: kzfree.cocci: Deprecate use of kzfree
Hi,
same patch
https://lkml.org/lkml/2020/8/11/130
Julia, I've send all the patches to fix existing
kfree_sensitive/kvfree_sensitive reports.
https://lkml.org/lkml/2020/8/27/168
https://lkml.org/lkml/2020/8/27/93
Thanks,
Denis
On 9/11/20 4:49 PM, Alex Dewar wrote:
> kzfree() is effectively deprecated as of commit 453431a54934 ("mm,
> treewide: rename kzfree() to kfree_sensitive()"). It is currently just a
> legacy alias for kfree_sensitive(), which achieves the same thing.
>
> Update kzfree.cocci accordingly:
> 1) Replace instances of kzfree with kfree_sensitive
> 2) Merge different rules for memset/memset_explicit as kzfree and
>kfree_sensitive are now equivalent
> 3) Rename script to kfree_sensitive.cocci
>
> In addition:
> 4) Move the script to the free/ subfolder, where it would seem to fit
>better
>
> Signed-off-by: Alex Dewar
> ---
> .../kfree_sensitive.cocci}| 38 +--
> 1 file changed, 10 insertions(+), 28 deletions(-)
> rename scripts/coccinelle/{api/kzfree.cocci => free/kfree_sensitive.cocci}
> (59%)
>
> diff --git a/scripts/coccinelle/api/kzfree.cocci
> b/scripts/coccinelle/free/kfree_sensitive.cocci
> similarity index 59%
> rename from scripts/coccinelle/api/kzfree.cocci
> rename to scripts/coccinelle/free/kfree_sensitive.cocci
> index 33625bd7cec9..a87f93f2ed5c 100644
> --- a/scripts/coccinelle/api/kzfree.cocci
> +++ b/scripts/coccinelle/free/kfree_sensitive.cocci
> @@ -1,13 +1,13 @@
> // SPDX-License-Identifier: GPL-2.0-only
> ///
> -/// Use kzfree, kvfree_sensitive rather than memset or
> -/// memzero_explicit followed by kfree
> +/// Use k{,v}free_sensitive rather than memset or memzero_explicit followed
> by
> +/// k{,v}free
> ///
> // Confidence: High
> // Copyright: (C) 2020 Denis Efremov ISPRAS
> // Options: --no-includes --include-headers
> //
> -// Keywords: kzfree, kvfree_sensitive
> +// Keywords: kfree_sensitive, kvfree_sensitive
> //
>
> virtual context
> @@ -18,7 +18,7 @@ virtual report
> @initialize:python@
> @@
> # kmalloc_oob_in_memset uses memset to explicitly trigger out-of-bounds
> access
> -filter = frozenset(['kmalloc_oob_in_memset', 'kzfree', 'kvfree_sensitive'])
> +filter = frozenset(['kmalloc_oob_in_memset', 'kfree_sensitive',
> 'kvfree_sensitive'])
>
> def relevant(p):
> return not (filter & {el.current_element for el in p})
> @@ -53,34 +53,16 @@ position m != cond.ok;
> type T;
> @@
>
> +(
> - memzero_explicit@m((T)E, size);
> - ... when != E
> - when strict
> -// TODO: uncomment when kfree_sensitive will be merged.
> -// Only this case is commented out because developers
> -// may not like patches like this since kzfree uses memset
> -// internally (not memzero_explicit).
> -//(
> -//- kfree(E)@p;
> -//+ kfree_sensitive(E);
> -//|
> -- \(vfree\|kvfree\)(E)@p;
> -+ kvfree_sensitive(E, size);
> -//)
> -
> -@rp_memset depends on patch@
> -expression E, size;
> -position p : script:python() { relevant(p) };
> -position m != cond.ok;
> -type T;
> -@@
> -
> +|
> - memset@m((T)E, 0, size);
> +)
>... when != E
>when strict
> (
> - kfree(E)@p;
> -+ kzfree(E);
> ++ kfree_sensitive(E);
> |
> - \(vfree\|kvfree\)(E)@p;
> + kvfree_sensitive(E, size);
> @@ -91,11 +73,11 @@ p << r.p;
> @@
>
> coccilib.report.print_report(p[0],
> - "WARNING: opportunity for kzfree/kvfree_sensitive")
> + "WARNING: opportunity for k{,v}free_sensitive")
>
> @script:python depends on org@
> p << r.p;
> @@
>
> coccilib.org.print_todo(p[0],
> - "WARNING: opportunity for kzfree/kvfree_sensitive")
> + "WARNING: opportunity for k{,v}free_sensitive")
>
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH] coccinelle: misc: add excluded_middle.cocci script
Check for "!A || A && B" condition. It's equivalent to "!A || B" condition. Signed-off-by: Denis Efremov --- scripts/coccinelle/misc/excluded_middle.cocci | 40 +++ 1 file changed, 40 insertions(+) create mode 100644 scripts/coccinelle/misc/excluded_middle.cocci diff --git a/scripts/coccinelle/misc/excluded_middle.cocci b/scripts/coccinelle/misc/excluded_middle.cocci new file mode 100644 index ..1b8c20f13966 --- /dev/null +++ b/scripts/coccinelle/misc/excluded_middle.cocci @@ -0,0 +1,40 @@ +// SPDX-License-Identifier: GPL-2.0-only +/// +/// Condition "!A || A && B" is equalent to "!A || B". +/// +// Confidence: High +// Copyright: (C) 2020 Denis Efremov ISPRAS +// Options: --no-includes --include-headers + +virtual patch +virtual context +virtual org +virtual report + +@r depends on !patch@ +expression A, B; +position p; +@@ + +* !A || (A && B)@p + +@depends on patch@ +expression A, B; +@@ + + !A || +- (A && + B +- ) + +@script:python depends on report@ +p << r.p; +@@ + +coccilib.report.print_report(p[0], "WARNING condition !A || A && B is equivalent to !A || B") + +@script:python depends on org@ +p << r.p; +@@ + +coccilib.org.print_todo(p[0], "WARNING condition !A || A && B is equivalent to !A || B") -- 2.26.2 ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] expression without side effects
On 9/2/20 3:32 PM, Julia Lawall wrote: > There is an isomorphism that you can disable: ptr_to_array Thanks! Anyway, even with ptr_to_array enabled equalizing cmd->dmap[0] and cmd->dmap[1] looks incorrect to me. Denis ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] expression without side effects
Hi, I'm trying to write a pattern to match expression without side-effects, i.e expression E but not i++, --i, function call. While trying to write it I faced that this expression matches different array indices. Actually, it's quite unexpected for me: @@ expression E; identifier A; @@ * E->A || E->A Actually, I would expect it not to match the file at all: $ spatch --cocci-file ./test.cocci ./drivers/md/dm-clone-metadata.c init_defs_builtins: /usr/lib64/coccinelle/standard.h HANDLING: ./drivers/md/dm-clone-metadata.c diff = --- ./drivers/md/dm-clone-metadata.c +++ /tmp/cocci-output-11041-66b4fb-dm-clone-metadata.c @@ -947,7 +947,6 @@ bool dm_clone_changed_this_transaction(s unsigned long flags; spin_lock_irqsave(>bitmap_lock, flags); - r = cmd->dmap[0].changed || cmd->dmap[1].changed; spin_unlock_irqrestore(>bitmap_lock, flags); return r; Thanks, Denis ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] checkpatch? (was: Re: [PATCH v3] coccinelle: misc: add uninitialized_var.cocci script)
On 9/1/20 5:37 PM, Joe Perches wrote:
> On Tue, 2020-09-01 at 12:48 +0300, Denis Efremov wrote:
>> uninitialized_var() macro was removed from the sources [1] and
>> other warning-silencing tricks were deprecated [2]. The purpose of this
>> cocci script is to prevent new occurrences of uninitialized_var()
>> open-coded variants.
>
>> +(
>> +* T var =@p var;
>> +|
>> +* T var =@p *(&(var));
>> +|
>> +* var =@p var
>> +|
>> +* var =@p *(&(var))
>> +)
>
> Adding a checkpatch test might be a good thing too.
>
> ---
> scripts/checkpatch.pl | 11 +++
> 1 file changed, 11 insertions(+)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 149518d2a6a7..300b2659aab3 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -3901,6 +3901,17 @@ sub process {
> #ignore lines not being added
> next if ($line =~ /^[^\+]/);
>
> +# check for self assigments used to avoid compiler warnings
> +# e.g.: int foo = foo, *bar = NULL;
> +#struct foo bar = *(&(bar));
> + if ($line =~ /^\+\s*(?:$Declare)?([A-Za-z_][A-Za-z\d_]*)\s*=/) {
> + my $var = $1;
> + if ($line =~
> /^\+\s*(?:$Declare)?$var\s*=\s*(?:$var|\*\s*\(?\s*&\s*\(?\s*$var\s*\)?\s*\)?)\s*[;,]/)
> {
> + WARN("SELF_ASSIGNMENT",
> + "Do not use self-assignments to avoid
> compiler warnings\n" . $herecurr);
> + }
> + }
> +
> # check for dereferences that span multiple lines
> if ($prevline =~ /^\+.*$Lval\s*(?:\.|->)\s*$/ &&
> $line =~ /^\+\s*(?!\#\s*(?!define\s+|if))\s*$Lval/) {
Looks good. I also faced this kind of assignments after declarations.
https://lkml.org/lkml/2020/8/31/85
I'm not sure if they are used to suppress compiler warnings, through.
Denis
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH] coccinelle: ifnullfree: add vfree(), kvfree*() functions
Extend the list of free functions with kvfree(), kvfree_sensitive(), vfree(). Signed-off-by: Denis Efremov --- scripts/coccinelle/free/ifnullfree.cocci | 11 +-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/scripts/coccinelle/free/ifnullfree.cocci b/scripts/coccinelle/free/ifnullfree.cocci index 2045391e36a0..285b92d5c665 100644 --- a/scripts/coccinelle/free/ifnullfree.cocci +++ b/scripts/coccinelle/free/ifnullfree.cocci @@ -20,8 +20,14 @@ expression E; - if (E != NULL) ( kfree(E); +| + kvfree(E); | kfree_sensitive(E); +| + kvfree_sensitive(E, ...); +| + vfree(E); | debugfs_remove(E); | @@ -42,9 +48,10 @@ position p; @@ * if (E != NULL) -* \(kfree@p\|kfree_sensitive@p\|debugfs_remove@p\|debugfs_remove_recursive@p\| +* \(kfree@p\|kvfree@p\|kfree_sensitive@p\|kvfree_sensitive@p\|vfree@p\| +* debugfs_remove@p\|debugfs_remove_recursive@p\| * usb_free_urb@p\|kmem_cache_destroy@p\|mempool_destroy@p\| -* dma_pool_destroy@p\)(E); +* dma_pool_destroy@p\)(E, ...); @script:python depends on org@ p << r.p; -- 2.26.2 ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH v3] coccinelle: misc: add uninitialized_var.cocci script
uninitialized_var() macro was removed from the sources [1] and
other warning-silencing tricks were deprecated [2]. The purpose of this
cocci script is to prevent new occurrences of uninitialized_var()
open-coded variants.
[1] commit 63a0895d960a ("compiler: Remove uninitialized_var() macro")
[2] commit 4b19bec97c88 ("docs: deprecated.rst: Add uninitialized_var()")
Cc: Kees Cook
Cc: Gustavo A. R. Silva
Signed-off-by: Denis Efremov
---
Changes in v2:
- Documentation cited in the script's description
- kernel.org link added to the diagnostics messages
- "T *var = " pattern removed
- "var =@p var", "var =@p *(&(var))" patterns added
Changes in v3:
- commit's description changed
.../coccinelle/misc/uninitialized_var.cocci | 51 +++
1 file changed, 51 insertions(+)
create mode 100644 scripts/coccinelle/misc/uninitialized_var.cocci
diff --git a/scripts/coccinelle/misc/uninitialized_var.cocci
b/scripts/coccinelle/misc/uninitialized_var.cocci
new file mode 100644
index ..8fa845cefe11
--- /dev/null
+++ b/scripts/coccinelle/misc/uninitialized_var.cocci
@@ -0,0 +1,51 @@
+// SPDX-License-Identifier: GPL-2.0-only
+///
+/// Please, don't reintroduce uninitialized_var().
+/// From Documentation/process/deprecated.rst:
+/// For any compiler warnings about uninitialized variables, just add
+/// an initializer. Using warning-silencing tricks is dangerous as it
+/// papers over real bugs (or can in the future), and suppresses unrelated
+/// compiler warnings (e.g. "unused variable"). If the compiler thinks it
+/// is uninitialized, either simply initialize the variable or make compiler
+/// changes. Keep in mind that in most cases, if an initialization is
+/// obviously redundant, the compiler's dead-store elimination pass will make
+/// sure there are no needless variable writes.
+///
+// Confidence: High
+// Copyright: (C) 2020 Denis Efremov ISPRAS
+// Options: --no-includes --include-headers
+//
+
+virtual context
+virtual report
+virtual org
+
+@r@
+identifier var;
+type T;
+position p;
+@@
+
+(
+* T var =@p var;
+|
+* T var =@p *(&(var));
+|
+* var =@p var
+|
+* var =@p *(&(var))
+)
+
+@script:python depends on report@
+p << r.p;
+@@
+
+coccilib.report.print_report(p[0],
+ "WARNING this kind of initialization is deprecated
(https://www.kernel.org/doc/html/latest/process/deprecated.html#uninitialized-var)")
+
+@script:python depends on org@
+p << r.p;
+@@
+
+coccilib.org.print_todo(p[0],
+ "WARNING this kind of initialization is deprecated
(https://www.kernel.org/doc/html/latest/process/deprecated.html#uninitialized-var)")
--
2.26.2
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH] coccinelle: api: kobj_to_dev: don't warn about kobj_to_dev()
Exclude kobj_to_dev() definition from warnings.
Signed-off-by: Denis Efremov
---
No changes in performance. This patch can be squashed to the
original patch with kobj_to_dev.cocci script.
scripts/coccinelle/api/kobj_to_dev.cocci | 13 +++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/scripts/coccinelle/api/kobj_to_dev.cocci
b/scripts/coccinelle/api/kobj_to_dev.cocci
index cd5d31c6fe76..d0b3b9647c19 100644
--- a/scripts/coccinelle/api/kobj_to_dev.cocci
+++ b/scripts/coccinelle/api/kobj_to_dev.cocci
@@ -15,10 +15,18 @@ virtual org
virtual patch
+@initialize:python@
+@@
+filter = frozenset(['kobj_to_dev'])
+
+def relevant(p):
+return not (filter & {el.current_element for el in p})
+
+
@r depends on !patch@
expression ptr;
symbol kobj;
-position p;
+position p : script:python() { relevant(p) };
@@
* container_of(ptr, struct device, kobj)@p
@@ -26,9 +34,10 @@ position p;
@depends on patch@
expression ptr;
+position p : script:python() { relevant(p) };
@@
-- container_of(ptr, struct device, kobj)
+- container_of(ptr, struct device, kobj)@p
+ kobj_to_dev(ptr)
--
2.26.2
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH v2] coccinelle: misc: add uninitialized_var.cocci script
Commit 63a0895d960a ("compiler: Remove uninitialized_var() macro") and
commit 4b19bec97c88 ("docs: deprecated.rst: Add uninitialized_var()")
removed uninitialized_var() and deprecated it.
The purpose of this script is to prevent new occurrences of open-coded
variants of uninitialized_var().
Cc: Kees Cook
Cc: Gustavo A. R. Silva
Signed-off-by: Denis Efremov
---
Changes in v2:
- Documentation cited in the script's description
- kernel.org link added to the diagnostics messages
- "T *var = " pattern removed
- "var =@p var", "var =@p *(&(var))" patterns added
.../coccinelle/misc/uninitialized_var.cocci | 51 +++
1 file changed, 51 insertions(+)
create mode 100644 scripts/coccinelle/misc/uninitialized_var.cocci
diff --git a/scripts/coccinelle/misc/uninitialized_var.cocci
b/scripts/coccinelle/misc/uninitialized_var.cocci
new file mode 100644
index ..8fa845cefe11
--- /dev/null
+++ b/scripts/coccinelle/misc/uninitialized_var.cocci
@@ -0,0 +1,51 @@
+// SPDX-License-Identifier: GPL-2.0-only
+///
+/// Please, don't reintroduce uninitialized_var().
+/// From Documentation/process/deprecated.rst:
+/// For any compiler warnings about uninitialized variables, just add
+/// an initializer. Using warning-silencing tricks is dangerous as it
+/// papers over real bugs (or can in the future), and suppresses unrelated
+/// compiler warnings (e.g. "unused variable"). If the compiler thinks it
+/// is uninitialized, either simply initialize the variable or make compiler
+/// changes. Keep in mind that in most cases, if an initialization is
+/// obviously redundant, the compiler's dead-store elimination pass will make
+/// sure there are no needless variable writes.
+///
+// Confidence: High
+// Copyright: (C) 2020 Denis Efremov ISPRAS
+// Options: --no-includes --include-headers
+//
+
+virtual context
+virtual report
+virtual org
+
+@r@
+identifier var;
+type T;
+position p;
+@@
+
+(
+* T var =@p var;
+|
+* T var =@p *(&(var));
+|
+* var =@p var
+|
+* var =@p *(&(var))
+)
+
+@script:python depends on report@
+p << r.p;
+@@
+
+coccilib.report.print_report(p[0],
+ "WARNING this kind of initialization is deprecated
(https://www.kernel.org/doc/html/latest/process/deprecated.html#uninitialized-var)")
+
+@script:python depends on org@
+p << r.p;
+@@
+
+coccilib.org.print_todo(p[0],
+ "WARNING this kind of initialization is deprecated
(https://www.kernel.org/doc/html/latest/process/deprecated.html#uninitialized-var)")
--
2.26.2
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [RFC PATCH] coccinelle: misc: add uninitialized_var.cocci script
On 8/29/20 10:48 PM, Julia Lawall wrote:
>
>
> On Sat, 29 Aug 2020, Joe Perches wrote:
>
>> On Sat, 2020-08-29 at 21:36 +0200, Julia Lawall wrote:
>>>
>>> On Wed, 12 Aug 2020, Denis Efremov wrote:
>>>
>>>> Commit 63a0895d960a ("compiler: Remove uninitialized_var() macro") and
>>>> commit 4b19bec97c88 ("docs: deprecated.rst: Add uninitialized_var()")
>>>> removed uninitialized_var() and deprecated it.
>>>>
>>>> The purpose of this script is to prevent new occurrences of open-coded
>>>> variants of uninitialized_var().
>>
>>>> Cc: Kees Cook
>>>> Cc: Gustavo A. R. Silva
>>>> Signed-off-by: Denis Efremov
>>>
>>> Applied, without the commented out part.
>>>
>>> I only got three warnings, though. Perhaps the others have been fixed?
>>
>> uninitialized_var does not exist in -next
Yes, and this rule checks for not introducing these initializations once again.
i.e, checks for:
int a = a;
int a = *();
>
> OK, if it seems better, I can remove it. Out of the threee reported, one
> was a completely unnecessary initialization.
>
I would like send v2 with better description and link to the documentation
because it's
now available online:
https://www.kernel.org/doc/html/latest/process/deprecated.html#uninitialized-var
Thanks,
Denis
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [RFC PATCH] coccinelle: api: add flex_array_size.cocci script
Suggest flex_array_size() wrapper to compute the size of a
flexible array member in a structure. The macro additionally
checks for integer overflows.
The cocci script intentionally skips cases where count argument
is not a member of a structure because this introduce false
positives.
Cc: Gustavo A. R. Silva
Cc: Kees Cook
Signed-off-by: Denis Efremov
---
Kees, Gustavo, may I have your acks if you find this script useful?
Currently, it emits following warnings:
./fs/select.c:994:25-26: WARNING opportunity for flex_array_size
./include/linux/avf/virtchnl.h:711:34-35: WARNING opportunity for
flex_array_size
./include/linux/avf/virtchnl.h:722:43-44: WARNING opportunity for
flex_array_size
./include/linux/avf/virtchnl.h:738:40-41: WARNING opportunity for
flex_array_size
./include/linux/avf/virtchnl.h:749:46-47: WARNING opportunity for
flex_array_size
./drivers/dma/qcom/bam_dma.c:1055:35-36: WARNING opportunity for flex_array_size
./drivers/md/dm-crypt.c:2895:45-46: WARNING opportunity for flex_array_size
./drivers/md/dm-crypt.c:3381:47-48: WARNING opportunity for flex_array_size
./drivers/md/dm-crypt.c:2484:45-46: WARNING opportunity for flex_array_size
./drivers/md/dm-crypt.c:2484:45-46: WARNING opportunity for flex_array_size
./net/sched/em_canid.c:198:48-49: WARNING opportunity for flex_array_size
./include/linux/filter.h:741:42-43: WARNING opportunity for flex_array_size
./fs/aio.c:677:42-43: WARNING opportunity for flex_array_size
./include/rdma/rdmavt_qp.h:537:31-32: WARNING opportunity for flex_array_size
./include/rdma/rdmavt_qp.h:537:31-32: WARNING opportunity for flex_array_size
./lib/ts_fsm.c:311:49-50: WARNING opportunity for flex_array_size
./mm/slab.c:3407:59-60: WARNING opportunity for flex_array_size
./mm/slab.c:2139:55-56: WARNING opportunity for flex_array_size
./mm/slab.c:3407:59-60: WARNING opportunity for flex_array_size
./mm/slab.c:2139:55-56: WARNING opportunity for flex_array_size
scripts/coccinelle/api/flex_array_size.cocci | 180 +++
1 file changed, 180 insertions(+)
create mode 100644 scripts/coccinelle/api/flex_array_size.cocci
diff --git a/scripts/coccinelle/api/flex_array_size.cocci
b/scripts/coccinelle/api/flex_array_size.cocci
new file mode 100644
index ..b5264a826c29
--- /dev/null
+++ b/scripts/coccinelle/api/flex_array_size.cocci
@@ -0,0 +1,180 @@
+// SPDX-License-Identifier: GPL-2.0-only
+///
+/// Suggest flex_array_size() wrapper to compute the size of a
+/// flexible array member in a structure. The macro additionally
+/// checks for integer overflows.
+///
+// Confidence: High
+// Copyright: (C) 2020 Denis Efremov ISPRAS
+// Options: --no-includes --include-headers
+//
+// Keywords: flex_array_size
+//
+
+
+virtual context
+virtual report
+virtual org
+virtual patch
+
+@decl_flex@
+identifier name, array, size;
+type TA, TS;
+@@
+
+ struct name {
+...
+TS size;
+...
+(
+TA array[];
+|
+TA array[\(0\|1\)];
+)
+ };
+
+@ptr_flex@
+identifier decl_flex.name;
+identifier instance;
+@@
+
+ struct name *instance;
+
+@struct_flex@
+identifier decl_flex.name;
+identifier instance;
+@@
+
+ struct name instance;
+
+@ptr_flex_size depends on !patch@
+identifier decl_flex.array, decl_flex.size;
+identifier ptr_flex.instance;
+type decl_flex.TA;
+position p;
+@@
+
+(
+* instance->size * sizeof(TA)@p
+|
+* instance->size * sizeof(*instance->array)@p
+)
+
+@depends on patch exists@
+identifier decl_flex.array, decl_flex.size;
+identifier ptr_flex.instance;
+type decl_flex.TA;
+@@
+
+(
+- instance->size * sizeof(TA)
++ flex_array_size(instance, array, instance->size)
+|
+- instance->size * sizeof(*instance->array)
++ flex_array_size(instance, array, instance->size)
+)
+
+@struct_flex_size depends on !patch@
+identifier decl_flex.array, decl_flex.size;
+identifier struct_flex.instance;
+type decl_flex.TA;
+position p;
+@@
+
+(
+* instance.size * sizeof(TA)@p
+|
+* instance.size * sizeof(*instance->array)@p
+)
+
+@depends on patch exists@
+identifier decl_flex.array, decl_flex.size;
+identifier struct_flex.instance;
+type decl_flex.TA;
+@@
+
+(
+- instance.size * sizeof(TA)
++ flex_array_size(instance, array, instance.size)
+|
+- instance.size * sizeof(*instance->array)
++ flex_array_size(instance, array, instance.size)
+)
+
+@func_arg_flex_size depends on !patch@
+identifier decl_flex.name, decl_flex.array, decl_flex.size;
+identifier func, instance;
+type decl_flex.TA;
+position p;
+@@
+
+ func(..., struct name *instance, ...) {
+... when any
+(
+* instance->size * sizeof(TA)@p
+|
+* instance->size * sizeof(*instance->array)@p
+)
+...
+ }
+
+@depends on patch exists@
+identifier decl_flex.name, decl_flex.array, decl_flex.size;
+identifier func, instance;
+type decl_flex.TA;
+@@
+
+ func(..., struct name *instance, ...) {
+... when any
+(
+- instance->size * sizeof(TA)
++ flex_array_size(instance, array, instance->size)
+|
+- instance->size * sizeof
Re: [Cocci] [PATCH] usb: atm: don't use snprintf() for sysfs attrs
>
> I tried:
> @@
> identifier f_show =~ "^.*_show$";
This will miss this kind of functions:
./drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:1953:static
DEVICE_ATTR(vbios_version, 0444, amdgpu_atombios_get_vbios_version,
./drivers/gpu/drm/amd/amdgpu/df_v3_6.c:266:static DEVICE_ATTR(df_cntr_avail,
S_IRUGO, df_v3_6_get_df_cntr_avail, NULL);
./drivers/input/touchscreen/melfas_mip4.c:1348:static DEVICE_ATTR(fw_version,
S_IRUGO, mip4_sysfs_read_fw_version, NULL);
./drivers/input/touchscreen/melfas_mip4.c:1373:static DEVICE_ATTR(hw_version,
S_IRUGO, mip4_sysfs_read_hw_version, NULL);
./drivers/input/touchscreen/melfas_mip4.c:1392:static DEVICE_ATTR(product_id,
S_IRUGO, mip4_sysfs_read_product_id, NULL);
...
> identifier dev, attr, buf;
> const char *chr;
> @@
> ssize_t f_show(struct device *dev, struct device_attribute *attr, char
> *buf)
> {
> <...
> (
> - sprintf
> + sysfs_sprintf
> (...);
> |
> - snprintf(buf, PAGE_SIZE,
> + sysfs_sprintf(buf,
> ...);
> |
> - scnprintf(buf, PAGE_SIZE,
> + sysfs_sprintf(buf,
> ...);
> |
> strcpy(buf, chr);
> sysfs_strcpy(buf, chr);
> )
> ...>
> }
>
> which finds direct statements without an assign
> but that doesn't find
>
> arch/arm/common/dmabounce.c:static ssize_t dmabounce_show(struct device *dev,
> struct device_attribute *attr, char *buf)
> arch/arm/common/dmabounce.c-{
> arch/arm/common/dmabounce.c-struct dmabounce_device_info *device_info =
> dev->archdata.dmabounce;
> arch/arm/common/dmabounce.c-return sprintf(buf, "%lu %lu %lu %lu %lu
> %lu\n",
> arch/arm/common/dmabounce.c-device_info->small.allocs,
> arch/arm/common/dmabounce.c-device_info->large.allocs,
> arch/arm/common/dmabounce.c-device_info->total_allocs -
> device_info->small.allocs -
> arch/arm/common/dmabounce.c-device_info->large.allocs,
> arch/arm/common/dmabounce.c-device_info->total_allocs,
> arch/arm/common/dmabounce.c-device_info->map_op_count,
> arch/arm/common/dmabounce.c-device_info->bounce_count);
> arch/arm/common/dmabounce.c-}
>
This will match it (the difference is in the ';'):
@@
identifier f_show =~ "^.*_show$";
identifier dev, attr, buf;
@@
ssize_t f_show(struct device *dev, struct device_attribute *attr, char *buf)
{
<...
- sprintf
+ sysfs_sprintf
(...)
...>
}
Regards,
Denis
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH] usb: atm: don't use snprintf() for sysfs attrs
Hi all,
On 8/27/20 10:42 PM, Julia Lawall wrote:
>
>
> On Thu, 27 Aug 2020, Joe Perches wrote:
>
>> On Thu, 2020-08-27 at 15:48 +0100, Alex Dewar wrote:
>>> On Thu, Aug 27, 2020 at 03:41:06PM +0200, Rasmus Villemoes wrote:
On 27/08/2020 15.18, Alex Dewar wrote:
> On Thu, Aug 27, 2020 at 09:15:37AM +0200, Greg Kroah-Hartman wrote:
>> On Thu, Aug 27, 2020 at 08:42:06AM +0200, Rasmus Villemoes wrote:
>>> On 25/08/2020 00.23, Alex Dewar wrote:
kernel/cpu.c: don't use snprintf() for sysfs attrs
As per the documentation (Documentation/filesystems/sysfs.rst),
snprintf() should not be used for formatting values returned by sysfs.
Just FYI, I've send an addition to the device_attr_show.cocci script[1] to turn
simple cases of snprintf (e.g. "%i") to sprintf. Looks like many developers
would
like it more than changing snprintf to scnprintf. As for me, I don't like the
idea
of automated altering of the original logic from bounded snprint to unbouded one
with sprintf.
[1] https://lkml.org/lkml/2020/8/13/786
Regarding current device_attr_show.cocci implementation, it detects the
functions
by declaration:
ssize_t any_name(struct device *dev, struct device_attribute *attr, char *buf)
and I limited the check to:
"return snprintf"
pattern because there are already too many warnings.
Actually, it looks more correct to check for:
ssize_t show(struct device *dev, struct device_attribute *attr, char *buf)
{
<...
* snprintf@p(...);
...>
}
This pattern should also highlight the snprintf calls there we save returned
value in a var, e.g.:
ret += snprintf(...);
...
ret += snprintf(...);
...
ret += snprintf(...);
return ret;
>
> Something like
>
> identifier f;
> fresh identifier = "sysfs" ## f;
>
> may be useful. Let me know if further help is needed.
Initially, I wrote the rule to search for DEVICE_ATTR(..., ..., func_name, ...)
functions. However, it looks like matching function prototype is enough. At
least,
I failed to find false positives. I rejected the initial DEVICE_ATTR() searching
because I thought that it's impossible to handle
DEVICE_ATTR_RO()/DEVICE_ATTR_RW()
macroses with coccinelle as they "generate" function names internally with
"##". "fresh identifier" should really help here, but now I doubt it's required
in
device_attr_show.cocci, function prototype is enough.
Thanks,
Denis
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH] coccinelle: api: update kzfree script to kfree_sensitive
Ping?
On 8/11/20 10:49 AM, Denis Efremov wrote:
> Commit 453431a54934 ("mm, treewide: rename kzfree() to kfree_sensitive()")
> renames kzfree to kfree_sensitive and uses memzero_explicit(...) instead of
> memset(..., 0, ...) internally. Update cocci script to reflect these
> changes.
>
> Signed-off-by: Denis Efremov
> ---
> Julia, I think you can squash this commit with original script, or I can
> resend the whole script since it's not merged to the mainline.
>
> .../{kzfree.cocci => kfree_sensitive.cocci} | 29 +--
> 1 file changed, 13 insertions(+), 16 deletions(-)
> rename scripts/coccinelle/api/{kzfree.cocci => kfree_sensitive.cocci} (70%)
>
> diff --git a/scripts/coccinelle/api/kzfree.cocci
> b/scripts/coccinelle/api/kfree_sensitive.cocci
> similarity index 70%
> rename from scripts/coccinelle/api/kzfree.cocci
> rename to scripts/coccinelle/api/kfree_sensitive.cocci
> index 33625bd7cec9..e4a066a0b77d 100644
> --- a/scripts/coccinelle/api/kzfree.cocci
> +++ b/scripts/coccinelle/api/kfree_sensitive.cocci
> @@ -1,13 +1,13 @@
> // SPDX-License-Identifier: GPL-2.0-only
> ///
> -/// Use kzfree, kvfree_sensitive rather than memset or
> -/// memzero_explicit followed by kfree
> +/// Use kfree_sensitive, kvfree_sensitive rather than memset or
> +/// memzero_explicit followed by kfree.
> ///
> // Confidence: High
> // Copyright: (C) 2020 Denis Efremov ISPRAS
> // Options: --no-includes --include-headers
> //
> -// Keywords: kzfree, kvfree_sensitive
> +// Keywords: kfree_sensitive, kvfree_sensitive
> //
>
> virtual context
> @@ -18,7 +18,8 @@ virtual report
> @initialize:python@
> @@
> # kmalloc_oob_in_memset uses memset to explicitly trigger out-of-bounds
> access
> -filter = frozenset(['kmalloc_oob_in_memset', 'kzfree', 'kvfree_sensitive'])
> +filter = frozenset(['kmalloc_oob_in_memset',
> + 'kfree_sensitive', 'kvfree_sensitive'])
>
> def relevant(p):
> return not (filter & {el.current_element for el in p})
> @@ -56,17 +57,13 @@ type T;
> - memzero_explicit@m((T)E, size);
>... when != E
>when strict
> -// TODO: uncomment when kfree_sensitive will be merged.
> -// Only this case is commented out because developers
> -// may not like patches like this since kzfree uses memset
> -// internally (not memzero_explicit).
> -//(
> -//- kfree(E)@p;
> -//+ kfree_sensitive(E);
> -//|
> +(
> +- kfree(E)@p;
> ++ kfree_sensitive(E);
> +|
> - \(vfree\|kvfree\)(E)@p;
> + kvfree_sensitive(E, size);
> -//)
> +)
>
> @rp_memset depends on patch@
> expression E, size;
> @@ -80,7 +77,7 @@ type T;
>when strict
> (
> - kfree(E)@p;
> -+ kzfree(E);
> ++ kfree_sensitive(E);
> |
> - \(vfree\|kvfree\)(E)@p;
> + kvfree_sensitive(E, size);
> @@ -91,11 +88,11 @@ p << r.p;
> @@
>
> coccilib.report.print_report(p[0],
> - "WARNING: opportunity for kzfree/kvfree_sensitive")
> + "WARNING: opportunity for kfree_sensitive/kvfree_sensitive")
>
> @script:python depends on org@
> p << r.p;
> @@
>
> coccilib.org.print_todo(p[0],
> - "WARNING: opportunity for kzfree/kvfree_sensitive")
> + "WARNING: opportunity for kfree_sensitive/kvfree_sensitive")
>
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH v2] coccinelle: api: add kobj_to_dev.cocci script
Use kobj_to_dev() instead of container_of(). Signed-off-by: Denis Efremov --- Changes in v2: - "symbol kobj;" added to the rule r scripts/coccinelle/api/kobj_to_dev.cocci | 45 1 file changed, 45 insertions(+) create mode 100644 scripts/coccinelle/api/kobj_to_dev.cocci diff --git a/scripts/coccinelle/api/kobj_to_dev.cocci b/scripts/coccinelle/api/kobj_to_dev.cocci new file mode 100644 index ..cd5d31c6fe76 --- /dev/null +++ b/scripts/coccinelle/api/kobj_to_dev.cocci @@ -0,0 +1,45 @@ +// SPDX-License-Identifier: GPL-2.0-only +/// +/// Use kobj_to_dev() instead of container_of() +/// +// Confidence: High +// Copyright: (C) 2020 Denis Efremov ISPRAS +// Options: --no-includes --include-headers +// +// Keywords: kobj_to_dev, container_of +// + +virtual context +virtual report +virtual org +virtual patch + + +@r depends on !patch@ +expression ptr; +symbol kobj; +position p; +@@ + +* container_of(ptr, struct device, kobj)@p + + +@depends on patch@ +expression ptr; +@@ + +- container_of(ptr, struct device, kobj) ++ kobj_to_dev(ptr) + + +@script:python depends on report@ +p << r.p; +@@ + +coccilib.report.print_report(p[0], "WARNING opportunity for kobj_to_dev()") + +@script:python depends on org@ +p << r.p; +@@ + +coccilib.org.print_todo(p[0], "WARNING opportunity for kobj_to_dev()") -- 2.26.2 ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH] coccinelle: api: add kobj_to_dev.cocci script
Use kobj_to_dev() instead of container_of(). Signed-off-by: Denis Efremov --- Examples of such patches: 893c3d82b425 watchdog: Use kobj_to_dev() API 23fd63a44460 hwmon: (nct6683) Replace container_of() with kobj_to_dev() 224941c9424f power: supply: use kobj_to_dev a9b9b2af40c7 backlight: lm3533_bl: Use kobj_to_dev() instead 0acb47a3a093 qlcnic: Use kobj_to_dev() instead 97cd738c44c8 gpiolib: sysfs: use kobj_to_dev d06f9e6c8960 hwmon: (nct7802) Replace container_of() API 036855a4c3b3 hwmon : (nct6775) Use kobj_to_dev() API baf1d9c18293 driver/base/soc: Use kobj_to_dev() API ae243ef0afbc rtc: sysfs: use kobj_to_dev 6b060d8a09e9 i2c: use kobj_to_dev() API 9e7bd945b9a9 scsi: core: use kobj_to_dev 0d730b57b95f s390/cio: use kobj_to_dev() API 0616ca73fd35 usb: use kobj_to_dev() API 8c9b839c0b80 alpha: use kobj_to_dev() 016c0bbae1d1 netxen: Use kobj_to_dev() 6908b45eafc4 GenWQE: use kobj_to_dev() 85f4f39c80e9 pch_phub: use kobj_to_dev() 47679cde604d misc: c2port: use kobj_to_dev() 85016ff33f35 misc: cxl: use kobj_to_dev() 092462c2b522 misc: eeprom: use kobj_to_dev() a9c9d9aca4e7 zorro: Use kobj_to_dev() a253f1eee6c4 rapidio: use kobj_to_dev() e3837b00b6bb drm/radeon: use kobj_to_dev() cc29ec874b37 drm/amdgpu: use kobj_to_dev() d122cbf1a310 drm/sysfs: use kobj_to_dev() 657fb5fbadb3 drm/i915: use kobj_to_dev() 554a60379aaa PCI: Use kobj_to_dev() instead of open-coding it 2cf83833fc9c HID: use kobj_to_dev() aeb7ed14fe5d bridge: use kobj_to_dev instead of to_dev 8e3829c61b48 staging:iio:adis16220: Use kobj_to_dev instead of open-coding it b0d1f807f340 driver-core: Use kobj_to_dev instead of re-implementing it scripts/coccinelle/api/kobj_to_dev.cocci | 44 1 file changed, 44 insertions(+) create mode 100644 scripts/coccinelle/api/kobj_to_dev.cocci diff --git a/scripts/coccinelle/api/kobj_to_dev.cocci b/scripts/coccinelle/api/kobj_to_dev.cocci new file mode 100644 index ..e2cdd424aeca --- /dev/null +++ b/scripts/coccinelle/api/kobj_to_dev.cocci @@ -0,0 +1,44 @@ +// SPDX-License-Identifier: GPL-2.0-only +/// +/// Use kobj_to_dev() instead of container_of() +/// +// Confidence: High +// Copyright: (C) 2020 Denis Efremov ISPRAS +// Options: --no-includes --include-headers +// +// Keywords: kobj_to_dev, container_of +// + +virtual context +virtual report +virtual org +virtual patch + + +@r depends on !patch@ +expression ptr; +position p; +@@ + +* container_of(ptr, struct device, kobj)@p + + +@depends on patch@ +expression ptr; +@@ + +- container_of(ptr, struct device, kobj) ++ kobj_to_dev(ptr) + + +@script:python depends on report@ +p << r.p; +@@ + +coccilib.report.print_report(p[0], "WARNING opportunity for kobj_to_dev()") + +@script:python depends on org@ +p << r.p; +@@ + +coccilib.org.print_todo(p[0], "WARNING opportunity for kobj_to_dev()") -- 2.26.2 ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH] coccinelle: api: add sprintf() support to device_attr_show
On 8/14/20 3:30 PM, Markus Elfring wrote:
>>> You propose to use a nested SmPL disjunction for desired adjustments.
>>> I suggest to start a corresponding case distinction behind
>>> the key word “return” instead of repeating it three times.
>>
>> It doesn't work.
>
> How do you think about to apply a SmPL rule variant like the following?
>
> @rp depends on patch@
> identifier show, dev, attr, buf;
> constant str;
> @@
>
> ssize_t show(struct device *dev, struct device_attribute *attr, char *buf)
> {
> <...
> return
> (
> - snprintf
> + sprintf
> (
> buf,
> - \(PAGE_SIZE\|PAGE_SIZE - 1\),
> ( str
> |
> ( "%i"\|"%i\n"\|"%li"\|"%li\n"\|"%lli"\|"%lli\n"\|
> "%d"\|"%d\n"\|"%ld"\|"%ld\n"\|"%lld"\|"%lld\n"\|
> "%u"\|"%u\n"\|"%lu"\|"%lu\n"\|"%llu"\|"%llu\n"\|
> "%x"\|"%x\n"\|"%lx"\|"%lx\n"\|"%llx"\|"%llx\n"\|
> "%X"\|"%X\n"\|"%lX"\|"%lX\n"\|"%llX"\|"%llX\n"\|
>
> "0x%x"\|"0x%x\n"\|"0x%lx"\|"0x%lx\n"\|"0x%llx"\|"0x%llx\n"\|
>
> "0x%X"\|"0x%X\n"\|"0x%lX"\|"0x%lX\n"\|"0x%llX"\|"0x%llX\n"\|
> "%02x\n"\|"%03x\n"\|"%04x\n"\|"%08x\n"\|
> "%02X\n"\|"%03X\n"\|"%04X\n"\|"%08X\n"\|
> "0x%02x\n"\|"0x%03x\n"\|"0x%04x\n"\|"0x%08x\n"\|
> "0x%02X\n"\|"0x%03X\n"\|"0x%04X\n"\|"0x%08X\n"\|
> "%zd"\|"%zd\n"\|"%zu"\|"%zu\n"\|"%zx"\|"%zx\n"\|
> "%c"\|"%c\n"\|"%p"\|"%p\n"\|"%pU\n"\|"%pUl\n"\|"%hu\n"
> ) ,
> ...
> )
> )
> |
> - snprintf
> + scnprintf
> (...)
> );
> ...>
> }
>
3 levels of nested disjunctions makes this pattern completely unreadable
and gives no comparable benefits. I don't think we should care much about
number of characters in the kernel sources, gzip will do a better job
anyway.
Thanks,
Denis
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH] coccinelle: api: add sprintf() support to device_attr_show
Hi,
Markus, I think that CCing new people and spam them with mails they
are obviously not interested in doesn't bring an additional value to
the discussion. linux-kernel and cocci mailing lists are enough
in my opinion. This also will allow us to keep "threaded" mail
order.
On 8/14/20 11:30 AM, Markus Elfring wrote:
>> Interesting enough that with this patch coccinelle starts to skip
>> patch generation in some cases. For example, it skips patch for
>> drivers/base/core.c This is an unexpected result for me.
>
> Would you like to point questionable differences for such patch hunks out?
Without this patch the script generates:
$ spatch -D patch --no-includes --include-headers --cocci-file
scripts/coccinelle/api/device_attr_show.cocci drivers/base/core.c
--- drivers/base/core.c
+++ /tmp/cocci-output-63510-2f17ff-core.c
@@ -1713,7 +1713,7 @@ ssize_t device_show_ulong(struct device
char *buf)
{
struct dev_ext_attribute *ea = to_ext_attr(attr);
- return snprintf(buf, PAGE_SIZE, "%lx\n", *(unsigned long *)(ea->var));
+ return scnprintf(buf, PAGE_SIZE, "%lx\n", *(unsigned long *)(ea->var));
}
EXPORT_SYMBOL_GPL(device_show_ulong);
@@ -1743,7 +1743,7 @@ ssize_t device_show_int(struct device *d
{
struct dev_ext_attribute *ea = to_ext_attr(attr);
- return snprintf(buf, PAGE_SIZE, "%d\n", *(int *)(ea->var));
+ return scnprintf(buf, PAGE_SIZE, "%d\n", *(int *)(ea->var));
}
EXPORT_SYMBOL_GPL(device_show_int);
@@ -1764,7 +1764,7 @@ ssize_t device_show_bool(struct device *
{
struct dev_ext_attribute *ea = to_ext_attr(attr);
- return snprintf(buf, PAGE_SIZE, "%d\n", *(bool *)(ea->var));
+ return scnprintf(buf, PAGE_SIZE, "%d\n", *(bool *)(ea->var));
}
EXPORT_SYMBOL_GPL(device_show_bool);
With this patch it generates nothing. I would expect spatch to generate
a different patch with sprintf instead of scnprintf, because I think
... is enough to match "*(int *)(ea->var)". Even if it can't match sprintf
pattern it should fallback to scnprintf pattern.
> You propose to use a nested SmPL disjunction for desired adjustments.
> I suggest to start a corresponding case distinction behind
> the key word “return” instead of repeating it three times.
It doesn't work.
Thanks,
Denis
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH] coccinelle: api: add sprintf() support to device_attr_show
It's safe to use sprintf() for simple cases in device_attr_show
type of functions. Add support for sprintf() in patch mode to
the device_attr_show.cocci script to print numbers and pointers.
Signed-off-by: Denis Efremov
---
Interesting enough that with this patch coccinelle starts to skip
patch generation in some cases. For example, it skips patch for
drivers/base/core.c This is an unexpected result for me.
scripts/coccinelle/api/device_attr_show.cocci | 30 +++
1 file changed, 30 insertions(+)
diff --git a/scripts/coccinelle/api/device_attr_show.cocci
b/scripts/coccinelle/api/device_attr_show.cocci
index d8ec4bb8ac41..1248b8c76cfe 100644
--- a/scripts/coccinelle/api/device_attr_show.cocci
+++ b/scripts/coccinelle/api/device_attr_show.cocci
@@ -30,15 +30,45 @@ ssize_t show(struct device *dev, struct device_attribute
*attr, char *buf)
@rp depends on patch@
identifier show, dev, attr, buf;
+constant str;
@@
ssize_t show(struct device *dev, struct device_attribute *attr, char *buf)
{
<...
+(
+ return
+- snprintf
++ sprintf
+ (buf,
+- \(PAGE_SIZE\|PAGE_SIZE - 1\),
+ str);
+|
+ return
+- snprintf
++ sprintf
+ (buf,
+- \(PAGE_SIZE\|PAGE_SIZE - 1\),
+ \("%i"\|"%i\n"\|"%li"\|"%li\n"\|"%lli"\|"%lli\n"\|
+ "%d"\|"%d\n"\|"%ld"\|"%ld\n"\|"%lld"\|"%lld\n"\|
+ "%u"\|"%u\n"\|"%lu"\|"%lu\n"\|"%llu"\|"%llu\n"\|
+ "%x"\|"%x\n"\|"%lx"\|"%lx\n"\|"%llx"\|"%llx\n"\|
+ "%X"\|"%X\n"\|"%lX"\|"%lX\n"\|"%llX"\|"%llX\n"\|
+
"0x%x"\|"0x%x\n"\|"0x%lx"\|"0x%lx\n"\|"0x%llx"\|"0x%llx\n"\|
+
"0x%X"\|"0x%X\n"\|"0x%lX"\|"0x%lX\n"\|"0x%llX"\|"0x%llX\n"\|
+ "%02x\n"\|"%03x\n"\|"%04x\n"\|"%08x\n"\|
+ "%02X\n"\|"%03X\n"\|"%04X\n"\|"%08X\n"\|
+ "0x%02x\n"\|"0x%03x\n"\|"0x%04x\n"\|"0x%08x\n"\|
+ "0x%02X\n"\|"0x%03X\n"\|"0x%04X\n"\|"0x%08X\n"\|
+ "%zd"\|"%zd\n"\|"%zu"\|"%zu\n"\|"%zx"\|"%zx\n"\|
+
"%c"\|"%c\n"\|"%p"\|"%p\n"\|"%pU\n"\|"%pUl\n"\|"%hu\n"\),
+ ...);
+|
return
- snprintf
+ scnprintf
(...);
+)
...>
}
--
2.26.2
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [RFC PATCH] coccinelle: misc: add uninitialized_var.cocci script
Commit 63a0895d960a ("compiler: Remove uninitialized_var() macro") and
commit 4b19bec97c88 ("docs: deprecated.rst: Add uninitialized_var()")
removed uninitialized_var() and deprecated it.
The purpose of this script is to prevent new occurrences of open-coded
variants of uninitialized_var().
Cc: Kees Cook
Cc: Gustavo A. R. Silva
Signed-off-by: Denis Efremov
---
List of warnings:
./lib/glob.c:48:31-39: WARNING: this kind of initialization is deprecated
./tools/testing/selftests/vm/userfaultfd.c:349:15-22: WARNING: this kind of
initialization is deprecated
./drivers/block/drbd/drbd_vli.h:330:5-9: WARNING: this kind of initialization
is deprecated
./drivers/char/hw_random/intel-rng.c:333:15-18: WARNING: this kind of
initialization is deprecated
./drivers/gpu/drm/nouveau/nvkm/subdev/instmem/nv50.c:316:7-10: WARNING: this
kind of initialization is deprecated
./arch/x86/include/asm/paravirt_types.h:455:15-20: WARNING: this kind of
initialization is deprecated
./arch/x86/include/asm/paravirt_types.h:455:30-35: WARNING: this kind of
initialization is deprecated
./arch/x86/include/asm/paravirt_types.h:455:45-50: WARNING: this kind of
initialization is deprecated
./arch/x86/include/asm/paravirt_types.h:475:15-20: WARNING: this kind of
initialization is deprecated
./arch/x86/include/asm/paravirt_types.h:475:30-35: WARNING: this kind of
initialization is deprecated
./arch/x86/include/asm/paravirt_types.h:476:2-7: WARNING: this kind of
initialization isdeprecated
./arch/x86/include/asm/paravirt_types.h:476:17-22: WARNING: this kind of
initialization is deprecated
./arch/x86/include/asm/paravirt_types.h:476:32-37: WARNING: this kind of
initialization is deprecated
.../coccinelle/misc/uninitialized_var.cocci | 51 +++
1 file changed, 51 insertions(+)
create mode 100644 scripts/coccinelle/misc/uninitialized_var.cocci
diff --git a/scripts/coccinelle/misc/uninitialized_var.cocci
b/scripts/coccinelle/misc/uninitialized_var.cocci
new file mode 100644
index ..e4787bc6ab9c
--- /dev/null
+++ b/scripts/coccinelle/misc/uninitialized_var.cocci
@@ -0,0 +1,51 @@
+// SPDX-License-Identifier: GPL-2.0-only
+///
+/// uninitialized_var() and its open-coded variations are
+/// deprecated. For details, see:
+/// Documentation/process/deprecated.rst
+///
+// Confidence: High
+// Copyright: (C) 2020 Denis Efremov ISPRAS
+// Options: --no-includes --include-headers
+//
+
+virtual context
+virtual report
+virtual org
+
+@r@
+identifier var;
+type T;
+position p;
+@@
+
+(
+* T var@p = var;
+|
+* T var@p = *(&(var));
+//|
+// TODO: Actually, I'm not sure about this pattern.
+// Looks like it's used in wireless drivers to determine
+// whether data belongs to the driver or not.
+// Here are all matches:
+// https://elixir.bootlin.com/linux/latest/source/net/mac802154/util.c#L14
+//
https://elixir.bootlin.com/linux/latest/source/drivers/staging/wlan-ng/cfg80211.c#L48
+//
https://elixir.bootlin.com/linux/latest/source/drivers/net/wireless/intersil/orinoco/cfg.c#L21
+// https://elixir.bootlin.com/linux/latest/source/net/mac80211/util.c#L37
+//
https://elixir.bootlin.com/linux/latest/source/drivers/net/wireless/rndis_wlan.c#L544
+// * T *var@p =
+)
+
+@script:python depends on report@
+p << r.p;
+@@
+
+coccilib.report.print_report(p[0],
+ "WARNING: this kind of initialization is deprecated")
+
+@script:python depends on org@
+p << r.p;
+@@
+
+coccilib.org.print_todo(p[0],
+ "WARNING: this kind of initialization is deprecated")
--
2.26.2
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH] coccinelle: api: update kzfree script to kfree_sensitive
Commit 453431a54934 ("mm, treewide: rename kzfree() to kfree_sensitive()")
renames kzfree to kfree_sensitive and uses memzero_explicit(...) instead of
memset(..., 0, ...) internally. Update cocci script to reflect these
changes.
Signed-off-by: Denis Efremov
---
Julia, I think you can squash this commit with original script, or I can
resend the whole script since it's not merged to the mainline.
.../{kzfree.cocci => kfree_sensitive.cocci} | 29 +--
1 file changed, 13 insertions(+), 16 deletions(-)
rename scripts/coccinelle/api/{kzfree.cocci => kfree_sensitive.cocci} (70%)
diff --git a/scripts/coccinelle/api/kzfree.cocci
b/scripts/coccinelle/api/kfree_sensitive.cocci
similarity index 70%
rename from scripts/coccinelle/api/kzfree.cocci
rename to scripts/coccinelle/api/kfree_sensitive.cocci
index 33625bd7cec9..e4a066a0b77d 100644
--- a/scripts/coccinelle/api/kzfree.cocci
+++ b/scripts/coccinelle/api/kfree_sensitive.cocci
@@ -1,13 +1,13 @@
// SPDX-License-Identifier: GPL-2.0-only
///
-/// Use kzfree, kvfree_sensitive rather than memset or
-/// memzero_explicit followed by kfree
+/// Use kfree_sensitive, kvfree_sensitive rather than memset or
+/// memzero_explicit followed by kfree.
///
// Confidence: High
// Copyright: (C) 2020 Denis Efremov ISPRAS
// Options: --no-includes --include-headers
//
-// Keywords: kzfree, kvfree_sensitive
+// Keywords: kfree_sensitive, kvfree_sensitive
//
virtual context
@@ -18,7 +18,8 @@ virtual report
@initialize:python@
@@
# kmalloc_oob_in_memset uses memset to explicitly trigger out-of-bounds access
-filter = frozenset(['kmalloc_oob_in_memset', 'kzfree', 'kvfree_sensitive'])
+filter = frozenset(['kmalloc_oob_in_memset',
+ 'kfree_sensitive', 'kvfree_sensitive'])
def relevant(p):
return not (filter & {el.current_element for el in p})
@@ -56,17 +57,13 @@ type T;
- memzero_explicit@m((T)E, size);
... when != E
when strict
-// TODO: uncomment when kfree_sensitive will be merged.
-// Only this case is commented out because developers
-// may not like patches like this since kzfree uses memset
-// internally (not memzero_explicit).
-//(
-//- kfree(E)@p;
-//+ kfree_sensitive(E);
-//|
+(
+- kfree(E)@p;
++ kfree_sensitive(E);
+|
- \(vfree\|kvfree\)(E)@p;
+ kvfree_sensitive(E, size);
-//)
+)
@rp_memset depends on patch@
expression E, size;
@@ -80,7 +77,7 @@ type T;
when strict
(
- kfree(E)@p;
-+ kzfree(E);
++ kfree_sensitive(E);
|
- \(vfree\|kvfree\)(E)@p;
+ kvfree_sensitive(E, size);
@@ -91,11 +88,11 @@ p << r.p;
@@
coccilib.report.print_report(p[0],
- "WARNING: opportunity for kzfree/kvfree_sensitive")
+ "WARNING: opportunity for kfree_sensitive/kvfree_sensitive")
@script:python depends on org@
p << r.p;
@@
coccilib.org.print_todo(p[0],
- "WARNING: opportunity for kzfree/kvfree_sensitive")
+ "WARNING: opportunity for kfree_sensitive/kvfree_sensitive")
--
2.26.2
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH v4] coccinelle: api: add kzfree script
On 8/11/20 2:45 AM, Eric Biggers wrote: > On Fri, Jul 17, 2020 at 10:39:20PM +0200, Julia Lawall wrote: >> >> >> On Fri, 17 Jul 2020, Denis Efremov wrote: >> >>> Check for memset()/memzero_explicit() followed by kfree()/vfree()/kvfree(). >>> >>> Signed-off-by: Denis Efremov >> >> Applied. > > FYI, this new script is already outdated, since kzfree() has been renamed to > kfree_sensitive(). > Ok, I will send an update. Thanks,Denis ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH v2] coccinelle: misc: add flexible_array.cocci script
Commit 68e4cd17e218 ("docs: deprecated.rst: Add zero-length and one-element
arrays") marks one-element and zero-length arrays as deprecated. Kernel
code should always use "flexible array members" instead.
The script warns about one-element and zero-length arrays in structs.
Cc: Kees Cook
Cc: Gustavo A. R. Silva
Signed-off-by: Denis Efremov
---
Changes in v2:
- all uapi headers are now filtered-out. Unfortunately, coccinelle
doesn't provide structure names in Location.current_element.
For structures the field is always "something_else". Thus, there is
no easy way to create a list of existing structures in uapi headers
and suppress the warning only for them, but not for the newly added
uapi structures.
- The pattern doesn't require 2+ fields in a structure/union anymore.
Now it also checks single field structures/unions.
- The pattern simplified and now uses disjuction in array elements
(Thanks, Markus)
- Unions are removed from patch mode
- one-element arrays are removed from patch mode. Correct patch may
involve turning the array to a simple field instead of a flexible
array.
On the current master branch, the rule generates:
- context: https://gist.github.com/evdenis/e2b4323491f9eff35376372df07f723c
- patch: https://gist.github.com/evdenis/46081da9d68ecefd07edc3769cebcf32
scripts/coccinelle/misc/flexible_array.cocci | 88
1 file changed, 88 insertions(+)
create mode 100644 scripts/coccinelle/misc/flexible_array.cocci
diff --git a/scripts/coccinelle/misc/flexible_array.cocci
b/scripts/coccinelle/misc/flexible_array.cocci
new file mode 100644
index ..bf6dcda1783e
--- /dev/null
+++ b/scripts/coccinelle/misc/flexible_array.cocci
@@ -0,0 +1,88 @@
+// SPDX-License-Identifier: GPL-2.0-only
+///
+/// Zero-length and one-element arrays are deprecated, see
+/// Documentation/process/deprecated.rst
+/// Flexible-array members should be used instead.
+///
+//
+// Confidence: High
+// Copyright: (C) 2020 Denis Efremov ISPRAS.
+// Comments:
+// Options: --no-includes --include-headers
+
+virtual context
+virtual report
+virtual org
+virtual patch
+
+@initialize:python@
+@@
+def relevant(positions):
+for p in positions:
+if "uapi" in p.file:
+ return False
+return True
+
+@r depends on !patch@
+identifier name, array;
+type T;
+position p : script:python() { relevant(p) };
+@@
+
+(
+ struct name {
+...
+* T array@p[\(0\|1\)];
+ };
+|
+ struct {
+...
+* T array@p[\(0\|1\)];
+ };
+|
+ union name {
+...
+* T array@p[\(0\|1\)];
+ };
+|
+ union {
+...
+* T array@p[\(0\|1\)];
+ };
+)
+
+@depends on patch exists@
+identifier name, array;
+type T;
+position p : script:python() { relevant(p) };
+@@
+
+(
+ struct name {
+...
+T array@p[
+- 0
+];
+ };
+|
+ struct {
+...
+T array@p[
+- 0
+];
+ };
+)
+
+@script: python depends on report@
+p << r.p;
+@@
+
+msg = "WARNING: use flexible-array member instead"
+coccilib.report.print_report(p[0], msg)
+
+@script: python depends on org@
+p << r.p;
+@@
+
+msg = "WARNING: use flexible-array member instead"
+coccilib.org.print_todo(p, msg)
--
2.26.2
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [RFC PATCH] coccinelle: misc: add flexible_array.cocci script
Commit 68e4cd17e218 ("docs: deprecated.rst: Add zero-length and one-element
arrays") marks one-element and zero-length arrays as deprecated. Kernel
code should always use "flexible array members" instead.
The script warns about one-element and zero-length arrays in structs.
Cc: Kees Cook
Cc: Gustavo A. R. Silva
Signed-off-by: Denis Efremov
---
Currently, it's just a draft. I've placed a number of questions in the
script and marked them as TODO. Kees, Gustavo, if you could help me with
my questions I think that this rule will be enough to close:
https://github.com/KSPP/linux/issues/76
BTW, I it's possible to not warn about files in uapi folder if
this is relevant. Do I need to do it in the script?
scripts/coccinelle/misc/flexible_array.cocci | 158 +++
1 file changed, 158 insertions(+)
create mode 100644 scripts/coccinelle/misc/flexible_array.cocci
diff --git a/scripts/coccinelle/misc/flexible_array.cocci
b/scripts/coccinelle/misc/flexible_array.cocci
new file mode 100644
index ..1e7165c79e60
--- /dev/null
+++ b/scripts/coccinelle/misc/flexible_array.cocci
@@ -0,0 +1,158 @@
+// SPDX-License-Identifier: GPL-2.0-only
+///
+/// Zero-length and one-element arrays are deprecated, see
+/// Documentation/process/deprecated.rst
+/// Flexible-array members should be used instead.
+///
+//
+// Confidence: High
+// Copyright: (C) 2020 Denis Efremov ISPRAS.
+// Comments:
+// Options: --no-includes --include-headers
+
+virtual context
+virtual report
+virtual org
+virtual patch
+
+@r depends on !patch@
+identifier name, size, array;
+// TODO: We can additionally restrict size and array to:
+// identifier size =~ ".*(num|len|count|size|ncpus).*";
+// identifier array !~ ".*(pad|reserved).*";
+// Do we need it?
+type TS, TA;
+position p;
+@@
+
+(
+ // This will also match: typedef struct name { ...
+ // However nested structs are not matched, i.e.:
+ // struct name1 { struct name2 { int s; int a[0]; } st; int i; }
+ // will not be matched. Do we need to handle it?
+ struct name {
+... // TODO: Maybe simple ... is enough? It will match structs with a
+TS size; // single field, e.g.
+... //
https://elixir.bootlin.com/linux/v5.8/source/arch/arm/include/uapi/asm/setup.h#L127
+(
+*TA array@p[0];
+|
+ // TODO: It seems that there are exception cases for array[1], e.g.
+ //
https://elixir.bootlin.com/linux/v5.8/source/arch/powerpc/boot/rs6000.h#L152
+ //
https://elixir.bootlin.com/linux/v5.8/source/include/uapi/linux/cdrom.h#L292
+ //
https://elixir.bootlin.com/linux/v5.8/source/drivers/net/wireless/ath/ath6kl/usb.c#L108
+ // We could either drop array[1] checking from this rule or
+ // restrict array name with regexp and add, for example, an "allowlist"
+ // with struct names where we allow this code pattern.
+ // TODO: How to handle: u8 data[1][MAXLEN_PSTR6]; ?
+*TA array@p[1];
+)
+ };
+|
+ struct {
+...
+TS size;
+...
+(
+*TA array@p[0];
+|
+*TA array@p[1];
+)
+ };
+|
+ // TODO: do we need to handle unions?
+ union name {
+...
+TS size;
+...
+(
+*TA array@p[0];
+|
+*TA array@p[1];
+)
+ };
+|
+ union {
+...
+TS size;
+...
+(
+*TA array@p[0];
+|
+*TA array@p[1];
+)
+ };
+)
+
+// FIXME: Patch mode doesn't work as expected.
+// Coccinelle handles formatting incorrectly.
+// Patch mode in this rule should be disabled until
+// proper formatting will be supported.
+@depends on patch exists@
+identifier name, size, array;
+type TS, TA;
+@@
+
+(
+ struct name {
+...
+TS size;
+...
+(
+-TA array[0];
+|
+-TA array[1];
+)
++TA array[];
+ };
+|
+ struct {
+...
+TS size;
+...
+(
+-TA array[0];
+|
+-TA array[1];
+)
++TA array[];
+ };
+|
+ union name {
+...
+TS size;
+...
+(
+-TA array[0];
+|
+-TA array[1];
+)
++TA array[];
+ };
+|
+ union {
+...
+TS size;
+...
+(
+-TA array[0];
+|
+-TA array[1];
+)
++TA array[];
+ };
+)
+
+@script: python depends on report@
+p << r.p;
+@@
+
+msg = "WARNING: use flexible-array member instead"
+coccilib.report.print_report(p[0], msg)
+
+@script: python depends on org@
+p << r.p;
+@@
+
+msg = "WARNING: use flexible-array member instead"
+coccilib.org.print_todo(p, msg)
--
2.26.2
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH v3] coccinelle: api: add kvmalloc script
Suggest kvmalloc, kvfree instead of opencoded patterns.
Signed-off-by: Denis Efremov
---
Changes in v2:
- binary operator cmp added
- NULL comparisions simplified
- "T x" case added to !patch mode
Changes in v3:
- kvfree rules added
scripts/coccinelle/api/kvmalloc.cocci | 188 ++
1 file changed, 188 insertions(+)
create mode 100644 scripts/coccinelle/api/kvmalloc.cocci
diff --git a/scripts/coccinelle/api/kvmalloc.cocci
b/scripts/coccinelle/api/kvmalloc.cocci
new file mode 100644
index ..2cb9281cc092
--- /dev/null
+++ b/scripts/coccinelle/api/kvmalloc.cocci
@@ -0,0 +1,188 @@
+// SPDX-License-Identifier: GPL-2.0-only
+///
+/// Find if/else condition with kmalloc/vmalloc calls.
+/// Suggest to use kvmalloc instead. Same for kvfree.
+///
+// Confidence: High
+// Copyright: (C) 2020 Denis Efremov ISPRAS
+// Options: --no-includes --include-headers
+//
+
+virtual patch
+virtual report
+virtual org
+virtual context
+
+@initialize:python@
+@@
+filter = frozenset(['kvfree'])
+
+def relevant(p):
+return not (filter & {el.current_element for el in p})
+
+@kvmalloc depends on !patch@
+expression E, E1, size;
+binary operator cmp = {<=, <, ==, >, >=};
+identifier x;
+type T;
+position p;
+@@
+
+(
+* if (size cmp E1 || ...)@p {
+...
+*E = \(kmalloc\|kzalloc\|kcalloc\|kmalloc_node\|kzalloc_node\|
+* kmalloc_array\|kmalloc_array_node\|kcalloc_node\)(..., size, ...)
+...
+ } else {
+...
+*E = \(vmalloc\|vzalloc\|vmalloc_node\|vzalloc_node\)(..., size, ...)
+...
+ }
+|
+* E = \(kmalloc\|kzalloc\|kcalloc\|kmalloc_node\|kzalloc_node\|
+* kmalloc_array\|kmalloc_array_node\|kcalloc_node\)(..., size, ...)
+ ... when != E = E1
+ when != size = E1
+ when any
+* if (E == NULL)@p {
+...
+* E = \(vmalloc\|vzalloc\|vmalloc_node\|vzalloc_node\)(..., size, ...)
+...
+ }
+|
+* T x = \(kmalloc\|kzalloc\|kcalloc\|kmalloc_node\|kzalloc_node\|
+* kmalloc_array\|kmalloc_array_node\|kcalloc_node\)(..., size, ...);
+ ... when != x = E1
+ when != size = E1
+ when any
+* if (x == NULL)@p {
+...
+* x = \(vmalloc\|vzalloc\|vmalloc_node\|vzalloc_node\)(..., size, ...)
+...
+ }
+)
+
+@kvfree depends on !patch@
+expression E;
+position p : script:python() { relevant(p) };
+@@
+
+* if (is_vmalloc_addr(E))@p {
+...
+* vfree(E)
+...
+ } else {
+... when != krealloc(E, ...)
+when any
+* \(kfree\|kzfree\)(E)
+...
+ }
+
+@depends on patch@
+expression E, E1, flags, size, node;
+binary operator cmp = {<=, <, ==, >, >=};
+identifier x;
+type T;
+@@
+
+(
+- if (size cmp E1)
+-E = kmalloc(size, flags);
+- else
+-E = vmalloc(size);
++ E = kvmalloc(size, flags);
+|
+- E = kmalloc(size, flags | __GFP_NOWARN);
+- if (E == NULL)
+- E = vmalloc(size);
++ E = kvmalloc(size, flags);
+|
+- T x = kmalloc(size, flags | __GFP_NOWARN);
+- if (x == NULL)
+- x = vmalloc(size);
++ T x = kvmalloc(size, flags);
+|
+- if (size cmp E1)
+-E = kzalloc(size, flags);
+- else
+-E = vzalloc(size);
++ E = kvzalloc(size, flags);
+|
+- E = kzalloc(size, flags | __GFP_NOWARN);
+- if (E == NULL)
+- E = vzalloc(size);
++ E = kvzalloc(size, flags);
+|
+- T x = kzalloc(size, flags | __GFP_NOWARN);
+- if (x == NULL)
+- x = vzalloc(size);
++ T x = kvzalloc(size, flags);
+|
+- if (size cmp E1)
+-E = kmalloc_node(size, flags, node);
+- else
+-E = vmalloc_node(size, node);
++ E = kvmalloc_node(size, flags, node);
+|
+- E = kmalloc_node(size, flags | __GFP_NOWARN, node);
+- if (E == NULL)
+- E = vmalloc_node(size, node);
++ E = kvmalloc_node(size, flags, node);
+|
+- T x = kmalloc_node(size, flags | __GFP_NOWARN, node);
+- if (x == NULL)
+- x = vmalloc_node(size, node);
++ T x = kvmalloc_node(size, flags, node);
+|
+- if (size cmp E1)
+-E = kvzalloc_node(size, flags, node);
+- else
+-E = vzalloc_node(size, node);
++ E = kvzalloc_node(size, flags, node);
+|
+- E = kvzalloc_node(size, flags | __GFP_NOWARN, node);
+- if (E == NULL)
+- E = vzalloc_node(size, node);
++ E = kvzalloc_node(size, flags, node);
+|
+- T x = kvzalloc_node(size, flags | __GFP_NOWARN, node);
+- if (x == NULL)
+- x = vzalloc_node(size, node);
++ T x = kvzalloc_node(size, flags, node);
+)
+
+@depends on patch@
+expression E;
+position p : script:python() { relevant(p) };
+@@
+
+- if (is_vmalloc_addr(E))@p
+- vfree(E);
+- else
+- kfree(E);
++ kvfree(E);
+
+@script: python depends on report@
+p << kvmalloc.p;
+@@
+
+coccilib.report.print_report(p[0], "WARNING: opportunity for kvmalloc")
+
+@script: python depends on org@
+p << kvmalloc.p;
+@@
+
+coccilib.org.print_todo(p[0], "WARNING: opportunity for kvmalloc")
+
+@script: python depends on report@
+p << kvfree.p;
+@@
+
+coccilib.report.print_report(p[0], "WARNING: opportunity for kvfree")
+
+@script: python depends on org@
+p <<
[Cocci] [PATCH v2] coccinelle: api: add kvmalloc script
Suggest kvmalloc instead of opencoded kmalloc && vmalloc condition.
Signed-off-by: Denis Efremov
---
Changes in v2:
- binary operator cmp added
- NULL comparisions simplified
- "T x" case added to !patch mode
scripts/coccinelle/api/kvmalloc.cocci | 142 ++
1 file changed, 142 insertions(+)
create mode 100644 scripts/coccinelle/api/kvmalloc.cocci
diff --git a/scripts/coccinelle/api/kvmalloc.cocci
b/scripts/coccinelle/api/kvmalloc.cocci
new file mode 100644
index ..20b22e3d0f74
--- /dev/null
+++ b/scripts/coccinelle/api/kvmalloc.cocci
@@ -0,0 +1,142 @@
+// SPDX-License-Identifier: GPL-2.0-only
+///
+/// Find if/else condition with kmalloc/vmalloc calls.
+/// Suggest to use kvmalloc instead.
+///
+// Confidence: High
+// Copyright: (C) 2020 Denis Efremov ISPRAS
+// Options: --no-includes --include-headers
+//
+
+virtual patch
+virtual report
+virtual org
+virtual context
+
+@opportunity depends on !patch@
+expression E, E1, size;
+binary operator cmp = {<=, <, ==, >, >=};
+identifier x;
+type T;
+position p;
+@@
+
+(
+* if (size cmp E1 || ...)@p {
+...
+*E = \(kmalloc\|kzalloc\|kcalloc\|kmalloc_node\|kzalloc_node\|
+* kmalloc_array\|kmalloc_array_node\|kcalloc_node\)(..., size, ...)
+...
+ } else {
+...
+*E = \(vmalloc\|vzalloc\|vmalloc_node\|vzalloc_node\)(..., size, ...)
+...
+ }
+|
+* E = \(kmalloc\|kzalloc\|kcalloc\|kmalloc_node\|kzalloc_node\|
+* kmalloc_array\|kmalloc_array_node\|kcalloc_node\)(..., size, ...)
+ ... when != E = E1
+ when != size = E1
+ when any
+* if (E == NULL)@p {
+...
+* E = \(vmalloc\|vzalloc\|vmalloc_node\|vzalloc_node\)(..., size, ...)
+...
+ }
+|
+* T x = \(kmalloc\|kzalloc\|kcalloc\|kmalloc_node\|kzalloc_node\|
+* kmalloc_array\|kmalloc_array_node\|kcalloc_node\)(..., size, ...);
+ ... when != x = E1
+ when != size = E1
+ when any
+* if (x == NULL)@p {
+...
+* x = \(vmalloc\|vzalloc\|vmalloc_node\|vzalloc_node\)(..., size, ...)
+...
+ }
+)
+
+@depends on patch@
+expression E, E1, flags, size, node;
+binary operator cmp = {<=, <, ==, >, >=};
+identifier x;
+type T;
+@@
+
+(
+- if (size cmp E1)
+-E = kmalloc(size, flags);
+- else
+-E = vmalloc(size);
++ E = kvmalloc(size, flags);
+|
+- E = kmalloc(size, flags | __GFP_NOWARN);
+- if (E == NULL)
+- E = vmalloc(size);
++ E = kvmalloc(size, flags);
+|
+- T x = kmalloc(size, flags | __GFP_NOWARN);
+- if (x == NULL)
+- x = vmalloc(size);
++ T x = kvmalloc(size, flags);
+|
+- if (size cmp E1)
+-E = kzalloc(size, flags);
+- else
+-E = vzalloc(size);
++ E = kvzalloc(size, flags);
+|
+- E = kzalloc(size, flags | __GFP_NOWARN);
+- if (E == NULL)
+- E = vzalloc(size);
++ E = kvzalloc(size, flags);
+|
+- T x = kzalloc(size, flags | __GFP_NOWARN);
+- if (x == NULL)
+- x = vzalloc(size);
++ T x = kvzalloc(size, flags);
+|
+- if (size cmp E1)
+-E = kmalloc_node(size, flags, node);
+- else
+-E = vmalloc_node(size, node);
++ E = kvmalloc_node(size, flags, node);
+|
+- E = kmalloc_node(size, flags | __GFP_NOWARN, node);
+- if (E == NULL)
+- E = vmalloc_node(size, node);
++ E = kvmalloc_node(size, flags, node);
+|
+- T x = kmalloc_node(size, flags | __GFP_NOWARN, node);
+- if (x == NULL)
+- x = vmalloc_node(size, node);
++ T x = kvmalloc_node(size, flags, node);
+|
+- if (size cmp E1)
+-E = kvzalloc_node(size, flags, node);
+- else
+-E = vzalloc_node(size, node);
++ E = kvzalloc_node(size, flags, node);
+|
+- E = kvzalloc_node(size, flags | __GFP_NOWARN, node);
+- if (E == NULL)
+- E = vzalloc_node(size, node);
++ E = kvzalloc_node(size, flags, node);
+|
+- T x = kvzalloc_node(size, flags | __GFP_NOWARN, node);
+- if (x == NULL)
+- x = vzalloc_node(size, node);
++ T x = kvzalloc_node(size, flags, node);
+)
+
+@script: python depends on report@
+p << opportunity.p;
+@@
+
+coccilib.report.print_report(p[0], "WARNING: opportunity for kvmalloc")
+
+@script: python depends on org@
+p << opportunity.p;
+@@
+
+coccilib.org.print_todo(p[0], "WARNING: opportunity for kvmalloc")
--
2.26.2
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH v7] coccinelle: api: add kfree_mismatch script
Check that alloc and free types of functions match each other.
Signed-off-by: Denis Efremov
---
Changes in v2:
- Lines are limited to 80 characters where possible
- Confidence changed from High to Medium because of
fs/btrfs/send.c:1119 false-positive
- __vmalloc_area_node() explicitly excluded from analysis
instead of !(file in "mm/vmalloc.c") condition
Changes in v3:
- prints style in org && report modes changed for python2
Changes in v4:
- missing msg argument to print_todo fixed
Changes in v5:
- fix position p in kfree rule
- move @kok and @v positions in choice rule after the arguments
- remove kvmalloc suggestions
Changes in v6:
- more asterisks added in context mode
- second @kok added to the choice rule
Changes in v7:
- file renamed to kfree_mismatch.cocci
- python function relevant() removed
- additional rule for filtering free positions added
- btrfs false-positive fixed
- confidence level changed to high
- kvfree_switch rule added
- names for position variables changed to @a (alloc) and @f (free)
scripts/coccinelle/api/kfree_mismatch.cocci | 229
1 file changed, 229 insertions(+)
create mode 100644 scripts/coccinelle/api/kfree_mismatch.cocci
diff --git a/scripts/coccinelle/api/kfree_mismatch.cocci
b/scripts/coccinelle/api/kfree_mismatch.cocci
new file mode 100644
index ..9e9ef9fd7a25
--- /dev/null
+++ b/scripts/coccinelle/api/kfree_mismatch.cocci
@@ -0,0 +1,229 @@
+// SPDX-License-Identifier: GPL-2.0-only
+///
+/// Check that kvmalloc'ed memory is freed by kfree functions,
+/// vmalloc'ed by vfree functions and kvmalloc'ed by kvfree
+/// functions.
+///
+// Confidence: High
+// Copyright: (C) 2020 Denis Efremov ISPRAS
+// Options: --no-includes --include-headers
+//
+
+virtual patch
+virtual report
+virtual org
+virtual context
+
+@alloc@
+expression E, E1;
+position kok, vok;
+@@
+
+(
+ if (...) {
+...
+E = \(kmalloc\|kzalloc\|krealloc\|kcalloc\|
+ kmalloc_node\|kzalloc_node\|kmalloc_array\|
+ kmalloc_array_node\|kcalloc_node\)(...)@kok
+...
+ } else {
+...
+E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|
+ vzalloc_node\|vmalloc_exec\|vmalloc_32\|
+ vmalloc_32_user\|__vmalloc\|__vmalloc_node_range\|
+ __vmalloc_node\)(...)@vok
+...
+ }
+|
+ E = \(kmalloc\|kzalloc\|krealloc\|kcalloc\|kmalloc_node\|kzalloc_node\|
+kmalloc_array\|kmalloc_array_node\|kcalloc_node\)(...)@kok
+ ... when != E = E1
+ when any
+ if (E == NULL) {
+...
+E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|
+ vzalloc_node\|vmalloc_exec\|vmalloc_32\|
+ vmalloc_32_user\|__vmalloc\|__vmalloc_node_range\|
+ __vmalloc_node\)(...)@vok
+...
+ }
+)
+
+@free@
+expression E;
+position fok;
+@@
+
+ E = \(kvmalloc\|kvzalloc\|kvcalloc\|kvzalloc_node\|kvmalloc_node\|
+kvmalloc_array\)(...)
+ ...
+ kvfree(E)@fok
+
+@vfree depends on !patch@
+expression E;
+position a != alloc.kok;
+position f != free.fok;
+@@
+
+* E = \(kmalloc\|kzalloc\|krealloc\|kcalloc\|kmalloc_node\|
+* kzalloc_node\|kmalloc_array\|kmalloc_array_node\|
+* kcalloc_node\)(...)@a
+ ... when != if (...) { ... E =
\(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\|vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\|__vmalloc_node_range\|__vmalloc_node\)(...);
... }
+ when != is_vmalloc_addr(E)
+ when any
+* \(vfree\|vfree_atomic\|kvfree\)(E)@f
+
+@depends on patch exists@
+expression E;
+position a != alloc.kok;
+position f != free.fok;
+@@
+
+ E = \(kmalloc\|kzalloc\|krealloc\|kcalloc\|kmalloc_node\|
+kzalloc_node\|kmalloc_array\|kmalloc_array_node\|
+kcalloc_node\)(...)@a
+ ... when != if (...) { ... E =
\(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\|vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\|__vmalloc_node_range\|__vmalloc_node\)(...);
... }
+ when != is_vmalloc_addr(E)
+ when any
+- \(vfree\|vfree_atomic\|kvfree\)(E)@f
++ kfree(E)
+
+@kfree depends on !patch@
+expression E;
+position a != alloc.vok;
+position f != free.fok;
+@@
+
+* E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\|
+* vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\|
+* __vmalloc_node_range\|__vmalloc_node\)(...)@a
+ ... when != is_vmalloc_addr(E)
+ when any
+* \(kfree\|kzfree\|kvfree\)(E)@f
+
+@depends on patch exists@
+expression E;
+position a != alloc.vok;
+position f != free.fok;
+@@
+
+ E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\|
+vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\|
+__vmalloc_node_range\|__vmalloc_node\)(...)@a
+ ... when != is_vmalloc_addr(E)
+ when any
+- \(kfree\|kvfree\)(E)@f
++ vfree(E)
+
+@kvfree depends on !patch@
+expression E;
+position a, f;
+@@
+
+* E = \(kvmalloc\|kvzalloc\|kvcalloc\|kvzalloc_node\|kvmalloc_node\|
+* kvmalloc_array\)(...)@a
+ ... when !=
Re: [Cocci] [PATCH v6] coccinelle: api: add kvfree script
Is there a difference from cocci point of view between: ... when != !is_vmalloc_addr(E) and ... when != is_vmalloc_addr(E) Should the latter one be used in most cases? Thanks, Denis ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH v6] coccinelle: api: add kvfree script
On 8/2/20 11:24 PM, Julia Lawall wrote:
>> +@initialize:python@
>> +@@
>> +# low-level memory api
>> +filter = frozenset(['__vmalloc_area_node'])
>> +
>> +def relevant(p):
>> +return not (filter & {el.current_element for el in p})
>
> Is this used?
I'll remove it in v8. Or do you want me to add iterate_dir_item() in the list?
>
> Otherwise, I think it would be good to not warn about a use of kvfree
> if that use is reachable from a kvmalloc. There seems to be such a false
> positive in fs/btrfs/send.c, on line 1118.
I don't know how to handle this case without position filter.
It's too complex. In iterate_dir_item() there is:
buf = kmalloc(buf_len, GFP_KERNEL);
while(...) {
if (...) {
if (is_vmalloc_addr(buf)) {
vfree(buf);
...
} else {
char *tmp = krealloc(buf, ...);
if (!tmp)
kfree(buf);
...
}
if (!buf) {
buf = kvmalloc(buf_len, GFP_KERNEL);
...
}
}
}
kvfree(buf);
Adding "when != kvfree(E)" is not enough:
* E = \(kvmalloc\|kvzalloc\|kvcalloc\|kvzalloc_node\|kvmalloc_node\|
* kvmalloc_array\)(...)@k
... when != is_vmalloc_addr(E)
+ when != kvfree(E)
when any
* \(kfree\|kzfree\|vfree\|vfree_atomic\)(E)@p
>
> It also seems that when there are both a kmalloc and a vmalloc, there is
> no warning if kfree or vfree is used. Is that intentional?
>
No, I will try to address it in v8.
Regards,
Denis
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH] coccinelle: api: add kvmalloc script
Suggest kvmalloc instead of opencoded kmalloc && vmalloc condition.
Signed-off-by: Denis Efremov
---
If coccinelle fails with "Segmentation fault" during analysis, then
one needs to increase stack limit, e.g. ulimit -s 32767.
Current, I've sent only one patch for this rule and will send the rest
after the merge window. https://lkml.org/lkml/2020/7/31/986
scripts/coccinelle/api/kvmalloc.cocci | 127 ++
1 file changed, 127 insertions(+)
create mode 100644 scripts/coccinelle/api/kvmalloc.cocci
diff --git a/scripts/coccinelle/api/kvmalloc.cocci
b/scripts/coccinelle/api/kvmalloc.cocci
new file mode 100644
index ..76d6aeab7c09
--- /dev/null
+++ b/scripts/coccinelle/api/kvmalloc.cocci
@@ -0,0 +1,127 @@
+// SPDX-License-Identifier: GPL-2.0-only
+///
+/// Find conditions in code for kmalloc/vmalloc calls.
+/// Suggest to use kvmalloc instead.
+///
+// Confidence: High
+// Copyright: (C) 2020 Denis Efremov ISPRAS
+// Options: --no-includes --include-headers
+//
+
+virtual patch
+virtual report
+virtual org
+virtual context
+
+@opportunity depends on !patch@
+expression E, E1, size;
+position p;
+@@
+
+(
+* if (\(size <= E1\|size < E1\|size == E1\|size > E1\) || ...)@p {
+...
+*E = \(kmalloc\|kzalloc\|kcalloc\|kmalloc_node\|kzalloc_node\|
+* kmalloc_array\|kmalloc_array_node\|kcalloc_node\)(..., size, ...)
+...
+ } else {
+...
+*E = \(vmalloc\|vzalloc\|vmalloc_node\|vzalloc_node\)(..., size, ...)
+...
+ }
+|
+* E = \(kmalloc\|kzalloc\|kcalloc\|kmalloc_node\|kzalloc_node\|
+* kmalloc_array\|kmalloc_array_node\|kcalloc_node\)(..., size, ...)
+ ... when != E = E1
+ when != size = E1
+ when any
+* if (\(!E\|E == NULL\))@p {
+...
+* E = \(vmalloc\|vzalloc\|vmalloc_node\|vzalloc_node\)(..., size, ...)
+...
+ }
+)
+
+@depends on patch@
+expression E, E1, flags, size, node;
+identifier x;
+type T;
+@@
+
+(
+- if (\(size <= E1\|size < E1\|size == E1\|size > E1\))
+-E = kmalloc(size, flags);
+- else
+-E = vmalloc(size);
++ E = kvmalloc(size, flags);
+|
+- E = kmalloc(size, flags | __GFP_NOWARN);
+- if (\(!E\|E == NULL\))
+- E = vmalloc(size);
++ E = kvmalloc(size, flags);
+|
+- T x = kmalloc(size, flags | __GFP_NOWARN);
+- if (\(!x\|x == NULL\))
+- x = vmalloc(size);
++ T x = kvmalloc(size, flags);
+|
+- if (\(size <= E1\|size < E1\|size == E1\|size > E1\))
+-E = kzalloc(size, flags);
+- else
+-E = vzalloc(size);
++ E = kvzalloc(size, flags);
+|
+- E = kzalloc(size, flags | __GFP_NOWARN);
+- if (\(!E\|E == NULL\))
+- E = vzalloc(size);
++ E = kvzalloc(size, flags);
+|
+- T x = kzalloc(size, flags | __GFP_NOWARN);
+- if (\(!x\|x == NULL\))
+- x = vzalloc(size);
++ T x = kvzalloc(size, flags);
+|
+- if (\(size <= E1\|size < E1\|size == E1\|size > E1\))
+-E = kmalloc_node(size, flags, node);
+- else
+-E = vmalloc_node(size, node);
++ E = kvmalloc_node(size, flags, node);
+|
+- E = kmalloc_node(size, flags | __GFP_NOWARN, node);
+- if (\(!E\|E == NULL\))
+- E = vmalloc_node(size, node);
++ E = kvmalloc_node(size, flags, node);
+|
+- T x = kmalloc_node(size, flags | __GFP_NOWARN, node);
+- if (\(!x\|x == NULL\))
+- x = vmalloc_node(size, node);
++ T x = kvmalloc_node(size, flags, node);
+|
+- if (\(size <= E1\|size < E1\|size == E1\|size > E1\))
+-E = kvzalloc_node(size, flags, node);
+- else
+-E = vzalloc_node(size, node);
++ E = kvzalloc_node(size, flags, node);
+|
+- E = kvzalloc_node(size, flags | __GFP_NOWARN, node);
+- if (\(!E\|E == NULL\))
+- E = vzalloc_node(size, node);
++ E = kvzalloc_node(size, flags, node);
+|
+- T x = kvzalloc_node(size, flags | __GFP_NOWARN, node);
+- if (\(!x\|x == NULL\))
+- x = vzalloc_node(size, node);
++ T x = kvzalloc_node(size, flags, node);
+)
+
+@script: python depends on report@
+p << opportunity.p;
+@@
+
+coccilib.report.print_report(p[0], "WARNING: opportunity for kvmalloc")
+
+@script: python depends on org@
+p << opportunity.p;
+@@
+
+coccilib.org.print_todo(p[0], "WARNING: opportunity for kvmalloc")
--
2.26.2
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH v6] coccinelle: api: add kvfree script
Check that alloc and free types of functions match each other.
Signed-off-by: Denis Efremov
---
Changes in v2:
- Lines are limited to 80 characters where possible
- Confidence changed from High to Medium because of
fs/btrfs/send.c:1119 false-positive
- __vmalloc_area_node() explicitly excluded from analysis
instead of !(file in "mm/vmalloc.c") condition
Changes in v3:
- prints style in org && report modes changed for python2
Changes in v4:
- missing msg argument to print_todo fixed
Changes in v5:
- fix position p in kfree rule
- move @kok and @v positions in choice rule after the arguments
- remove kvmalloc suggestions
Changes in v6:
- more asterisks added in context mode
- second @kok added to the choice rule
scripts/coccinelle/api/kvfree.cocci | 182
1 file changed, 182 insertions(+)
create mode 100644 scripts/coccinelle/api/kvfree.cocci
diff --git a/scripts/coccinelle/api/kvfree.cocci
b/scripts/coccinelle/api/kvfree.cocci
new file mode 100644
index ..f43cda5b0d5b
--- /dev/null
+++ b/scripts/coccinelle/api/kvfree.cocci
@@ -0,0 +1,182 @@
+// SPDX-License-Identifier: GPL-2.0-only
+///
+/// Check that kvmalloc'ed memory is freed by kfree functions,
+/// vmalloc'ed by vfree functions and kvmalloc'ed by kvfree
+/// functions.
+///
+// Confidence: Medium
+// Copyright: (C) 2020 Denis Efremov ISPRAS
+// Options: --no-includes --include-headers
+//
+
+virtual patch
+virtual report
+virtual org
+virtual context
+
+@initialize:python@
+@@
+# low-level memory api
+filter = frozenset(['__vmalloc_area_node'])
+
+def relevant(p):
+return not (filter & {el.current_element for el in p})
+
+@choice@
+expression E, E1;
+position kok, vok;
+@@
+
+(
+ if (...) {
+...
+E = \(kmalloc\|kzalloc\|krealloc\|kcalloc\|
+ kmalloc_node\|kzalloc_node\|kmalloc_array\|
+ kmalloc_array_node\|kcalloc_node\)(...)@kok
+...
+ } else {
+...
+E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|
+ vzalloc_node\|vmalloc_exec\|vmalloc_32\|
+ vmalloc_32_user\|__vmalloc\|__vmalloc_node_range\|
+ __vmalloc_node\)(...)@vok
+...
+ }
+|
+ E = \(kmalloc\|kzalloc\|krealloc\|kcalloc\|kmalloc_node\|kzalloc_node\|
+kmalloc_array\|kmalloc_array_node\|kcalloc_node\)(...)@kok
+ ... when != E = E1
+ when any
+ if (\(!E\|E == NULL\)) {
+...
+E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|
+ vzalloc_node\|vmalloc_exec\|vmalloc_32\|
+ vmalloc_32_user\|__vmalloc\|__vmalloc_node_range\|
+ __vmalloc_node\)(...)@vok
+...
+ }
+)
+
+@vfree depends on !patch@
+expression E;
+position k != choice.kok;
+position p;
+@@
+
+* E = \(kmalloc\|kzalloc\|krealloc\|kcalloc\|kmalloc_node\|
+* kzalloc_node\|kmalloc_array\|kmalloc_array_node\|
+* kcalloc_node\)(...)@k
+ ... when != if (...) { ... E =
\(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\|vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\|__vmalloc_node_range\|__vmalloc_node\)(...);
... }
+ when != is_vmalloc_addr(E)
+ when any
+* \(vfree\|vfree_atomic\|kvfree\)(E)@p
+
+@pvfree depends on patch exists@
+expression E;
+position k != choice.kok;
+@@
+
+ E = \(kmalloc\|kzalloc\|krealloc\|kcalloc\|kmalloc_node\|
+kzalloc_node\|kmalloc_array\|kmalloc_array_node\|
+kcalloc_node\)(...)@k
+ ... when != if (...) { ... E =
\(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\|vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\|__vmalloc_node_range\|__vmalloc_node\)(...);
... }
+ when != is_vmalloc_addr(E)
+ when any
+- \(vfree\|vfree_atomic\|kvfree\)(E)
++ kfree(E)
+
+@kfree depends on !patch@
+expression E;
+position v != choice.vok;
+position p;
+@@
+
+* E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\|
+* vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\|
+* __vmalloc_node_range\|__vmalloc_node\)(...)@v
+ ... when != !is_vmalloc_addr(E)
+ when any
+* \(kfree\|kzfree\|kvfree\)(E)@p
+
+@pkfree depends on patch exists@
+expression E;
+position v != choice.vok;
+@@
+
+ E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\|
+vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\|
+__vmalloc_node_range\|__vmalloc_node\)(...)@v
+ ... when != !is_vmalloc_addr(E)
+ when any
+- \(kfree\|kvfree\)(E)
++ vfree(E)
+
+@kvfree depends on !patch@
+expression E;
+position p, k;
+@@
+
+* E = \(kvmalloc\|kvzalloc\|kvcalloc\|kvzalloc_node\|kvmalloc_node\|
+* kvmalloc_array\)(...)@k
+ ... when != is_vmalloc_addr(E)
+ when any
+* \(kfree\|kzfree\|vfree\|vfree_atomic\)(E)@p
+
+@pkvfree depends on patch exists@
+expression E;
+@@
+
+ E = \(kvmalloc\|kvzalloc\|kvcalloc\|kvzalloc_node\|kvmalloc_node\|
+kvmalloc_array\)(...)
+ ... when != is_vmalloc_addr(E)
+ when any
+- \(kfree\|vfree\)(E)
++ kvfree(E)
+
+@script: python depends on report@
+k << vfree.
[Cocci] Match variable declaration with init expression
Hi, This pattern: - E = kzalloc(size, flags | __GFP_NOWARN); - if (\(!E\|E == null\))@p - E = vzalloc(size); + E = kvzalloc(size, flags); matches this code: void *p; p = kzalloc(size, gfp | __GFP_NOWARN); if (!p) p = vzalloc(size); But not this: void *p = kzalloc(size, gfp | __GFP_NOWARN); if (!p) p = vzalloc(size); What can I do to match them both? Thanks, Denis ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH v5] coccinelle: api: add kvfree script
Check that alloc and free types of functions match each other.
Signed-off-by: Denis Efremov
---
Changes in v2:
- Lines are limited to 80 characters where possible
- Confidence changed from High to Medium because of
fs/btrfs/send.c:1119 false-positive
- __vmalloc_area_node() explicitly excluded from analysis
instead of !(file in "mm/vmalloc.c") condition
Changes in v3:
- prints style in org && report modes changed for python2
Changes in v4:
- missing msg argument to print_todo fixed
Changes in v5:
- fix position p in kfree rule
- move @kok and @v positions in choice rule after the arguments
- remove kvmalloc suggestions
scripts/coccinelle/api/kvfree.cocci | 182
1 file changed, 182 insertions(+)
create mode 100644 scripts/coccinelle/api/kvfree.cocci
diff --git a/scripts/coccinelle/api/kvfree.cocci
b/scripts/coccinelle/api/kvfree.cocci
new file mode 100644
index ..e83b1ebbad7e
--- /dev/null
+++ b/scripts/coccinelle/api/kvfree.cocci
@@ -0,0 +1,182 @@
+// SPDX-License-Identifier: GPL-2.0-only
+///
+/// Check that kvmalloc'ed memory is freed by kfree functions,
+/// vmalloc'ed by vfree functions and kvmalloc'ed by kvfree
+/// functions.
+///
+// Confidence: Medium
+// Copyright: (C) 2020 Denis Efremov ISPRAS
+// Options: --no-includes --include-headers
+//
+
+virtual patch
+virtual report
+virtual org
+virtual context
+
+@initialize:python@
+@@
+# low-level memory api
+filter = frozenset(['__vmalloc_area_node'])
+
+def relevant(p):
+return not (filter & {el.current_element for el in p})
+
+@choice@
+expression E, E1;
+position kok, vok;
+@@
+
+(
+ if (...) {
+...
+E = \(kmalloc\|kzalloc\|krealloc\|kcalloc\|
+ kmalloc_node\|kzalloc_node\|kmalloc_array\|
+ kmalloc_array_node\|kcalloc_node\)(...)@kok
+...
+ } else {
+...
+E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|
+ vzalloc_node\|vmalloc_exec\|vmalloc_32\|
+ vmalloc_32_user\|__vmalloc\|__vmalloc_node_range\|
+ __vmalloc_node\)(...)@vok
+...
+ }
+|
+ E = \(kmalloc\|kzalloc\|krealloc\|kcalloc\|kmalloc_node\|kzalloc_node\|
+kmalloc_array\|kmalloc_array_node\|kcalloc_node\)(...)
+ ... when != E = E1
+ when any
+ if (\(!E\|E == NULL\)) {
+...
+E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|
+ vzalloc_node\|vmalloc_exec\|vmalloc_32\|
+ vmalloc_32_user\|__vmalloc\|__vmalloc_node_range\|
+ __vmalloc_node\)(...)@vok
+...
+ }
+)
+
+@vfree depends on !patch@
+expression E;
+position k != choice.kok;
+position p;
+@@
+
+* E = \(kmalloc\|kzalloc\|krealloc\|kcalloc\|kmalloc_node\|
+kzalloc_node\|kmalloc_array\|kmalloc_array_node\|
+kcalloc_node\)(...)@k
+ ... when != if (...) { ... E =
\(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\|vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\|__vmalloc_node_range\|__vmalloc_node\)(...);
... }
+ when != is_vmalloc_addr(E)
+ when any
+* \(vfree\|vfree_atomic\|kvfree\)(E)@p
+
+@pvfree depends on patch exists@
+expression E;
+position k != choice.kok;
+@@
+
+ E = \(kmalloc\|kzalloc\|krealloc\|kcalloc\|kmalloc_node\|
+kzalloc_node\|kmalloc_array\|kmalloc_array_node\|
+kcalloc_node\)(...)@k
+ ... when != if (...) { ... E =
\(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\|vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\|__vmalloc_node_range\|__vmalloc_node\)(...);
... }
+ when != is_vmalloc_addr(E)
+ when any
+- \(vfree\|vfree_atomic\|kvfree\)(E)
++ kfree(E)
+
+@kfree depends on !patch@
+expression E;
+position v != choice.vok;
+position p;
+@@
+
+* E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\|
+vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\|
+__vmalloc_node_range\|__vmalloc_node\)(...)@v
+ ... when != !is_vmalloc_addr(E)
+ when any
+* \(kfree\|kzfree\|kvfree\)(E)@p
+
+@pkfree depends on patch exists@
+expression E;
+position v != choice.vok;
+@@
+
+ E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\|
+vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\|
+__vmalloc_node_range\|__vmalloc_node\)(...)@v
+ ... when != !is_vmalloc_addr(E)
+ when any
+- \(kfree\|kvfree\)(E)
++ vfree(E)
+
+@kvfree depends on !patch@
+expression E;
+position p, k;
+@@
+
+* E = \(kvmalloc\|kvzalloc\|kvcalloc\|kvzalloc_node\|kvmalloc_node\|
+kvmalloc_array\)(...)@k
+ ... when != is_vmalloc_addr(E)
+ when any
+* \(kfree\|kzfree\|vfree\|vfree_atomic\)(E)@p
+
+@pkvfree depends on patch exists@
+expression E;
+@@
+
+ E = \(kvmalloc\|kvzalloc\|kvcalloc\|kvzalloc_node\|kvmalloc_node\|
+kvmalloc_array\)(...)
+ ... when != is_vmalloc_addr(E)
+ when any
+- \(kfree\|vfree\)(E)
++ kvfree(E)
+
+@script: python depends on report@
+k << vfree.k;
+p << vfree.p;
+@@
+
+msg = "WARNING: kmalloc is used to allocat
Re: [Cocci] [PATCH v4] coccinelle: api: add kvfree script
> With the current patch mode, I got some changes in a recent linux-next.
> Have you sent patches for these issues?
For mellanox, I've sent these patches:
https://lkml.org/lkml/2020/6/5/901
https://lkml.org/lkml/2020/6/1/713
They were accepted.
I see two new places in mellanox driver in linux-next. It looks like this
is new code that is not yet merged to the linux master branch.
diff -u -p a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c
b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c
@@ -228,8 +228,8 @@ static int rx_fs_create(struct mlx5e_pri
fs_prot->miss_rule = miss_rule;
out:
- kfree(flow_group_in);
- kfree(spec);
+ kvfree(flow_group_in);
+ kvfree(spec);
return err;
}
diff -u -p a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/fs_tcp.c
b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/fs_tcp.c
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/fs_tcp.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/fs_tcp.c
@@ -191,7 +191,7 @@ static int accel_fs_tcp_create_groups(st
ft->g = kcalloc(MLX5E_ACCEL_FS_TCP_NUM_GROUPS, sizeof(*ft->g), GFP_KERNEL);
in = kvzalloc(inlen, GFP_KERNEL);
if (!in || !ft->g) {
- kvfree(ft->g);
+ kfree(ft->g);
kvfree(in);
return -ENOMEM;
}
I will send the fixes when the code will be merged to the linux master branch.
Maybe it will be fixed already in net-next at that time.
>
> Do the checks for the opportunities for kvmalloc really belong in this
> rule? That issue is not mentioned in the commit log or the description of
> the semantic patch.
I added this at the last moment. It was easy enough to add it based on existing
patterns. I will add description for this warnings. Or do you want me to single
out this warning to a separate rule?
Regards,
Denis
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH v4] coccinelle: api: add kvfree script
Check that alloc and free types of functions match each other.
Signed-off-by: Denis Efremov
---
Changes in v2:
- Lines are limited to 80 characters where possible
- Confidence changed from High to Medium because of
fs/btrfs/send.c:1119 false-positive
- __vmalloc_area_node() explicitly excluded from analysis
instead of !(file in "mm/vmalloc.c") condition
Changes in v3:
- prints style in org && report modes changed for python2
Changes in v4:
- missing msg argument to print_todo fixed
scripts/coccinelle/api/kvfree.cocci | 227
1 file changed, 227 insertions(+)
create mode 100644 scripts/coccinelle/api/kvfree.cocci
diff --git a/scripts/coccinelle/api/kvfree.cocci
b/scripts/coccinelle/api/kvfree.cocci
new file mode 100644
index ..d73578c5549c
--- /dev/null
+++ b/scripts/coccinelle/api/kvfree.cocci
@@ -0,0 +1,227 @@
+// SPDX-License-Identifier: GPL-2.0-only
+///
+/// Check that kvmalloc'ed memory is freed by kfree functions,
+/// vmalloc'ed by vfree functions and kvmalloc'ed by kvfree
+/// functions.
+///
+// Confidence: Medium
+// Copyright: (C) 2020 Denis Efremov ISPRAS
+// Options: --no-includes --include-headers
+//
+
+virtual patch
+virtual report
+virtual org
+virtual context
+
+@initialize:python@
+@@
+# low-level memory api
+filter = frozenset(['__vmalloc_area_node'])
+
+def relevant(p):
+return not (filter & {el.current_element for el in p})
+
+@choice@
+expression E, E1;
+position kok, vok;
+@@
+
+(
+ if (...) {
+...
+E = \(kmalloc@kok\|kzalloc@kok\|krealloc@kok\|kcalloc@kok\|
+ kmalloc_node@kok\|kzalloc_node@kok\|kmalloc_array@kok\|
+ kmalloc_array_node@kok\|kcalloc_node@kok\)(...)
+...
+ } else {
+...
+E = \(vmalloc@vok\|vzalloc@vok\|vmalloc_user@vok\|vmalloc_node@vok\|
+ vzalloc_node@vok\|vmalloc_exec@vok\|vmalloc_32@vok\|
+ vmalloc_32_user@vok\|__vmalloc@vok\|__vmalloc_node_range@vok\|
+ __vmalloc_node@vok\)(...)
+...
+ }
+|
+ E = \(kmalloc\|kzalloc\|krealloc\|kcalloc\|kmalloc_node\|kzalloc_node\|
+kmalloc_array\|kmalloc_array_node\|kcalloc_node\)(...)
+ ... when != E = E1
+ when any
+ if (\(!E\|E == NULL\)) {
+...
+E = \(vmalloc@vok\|vzalloc@vok\|vmalloc_user@vok\|vmalloc_node@vok\|
+ vzalloc_node@vok\|vmalloc_exec@vok\|vmalloc_32@vok\|
+ vmalloc_32_user@vok\|__vmalloc@vok\|__vmalloc_node_range@vok\|
+ __vmalloc_node@vok\)(...)
+...
+ }
+)
+
+@opportunity depends on !patch@
+expression E, E1, size;
+position p : script:python() { relevant(p) };
+@@
+
+(
+* if (\(size <= E1\|size < E1\|size = E1\|size > E1\) || ...)@p {
+...
+E = \(kmalloc\|kzalloc\|krealloc\|kcalloc\|kmalloc_node\|kzalloc_node\|
+ kmalloc_array\|kmalloc_array_node\|kcalloc_node\)(..., size, ...)
+...
+ } else {
+...
+E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\|
+ vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\|
+ __vmalloc_node_range\|__vmalloc_node\)(..., size, ...)
+...
+ }
+|
+ E = \(kmalloc\|kzalloc\|krealloc\|kcalloc\|kmalloc_node\|kzalloc_node\|
+kmalloc_array\|kmalloc_array_node\|kcalloc_node\)(..., size, ...)
+ ... when != E = E1
+ when != size = E1
+ when any
+* if (\(!E\|E == NULL\))@p {
+...
+E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\|
+ vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\|
+ __vmalloc_node_range\|__vmalloc_node\)(..., size, ...)
+...
+ }
+)
+
+@vfree depends on !patch@
+expression E;
+position k != choice.kok;
+position p;
+@@
+
+* E = \(kmalloc@k\|kzalloc@k\|krealloc@k\|kcalloc@k\|kmalloc_node@k\|
+kzalloc_node@k\|kmalloc_array@k\|kmalloc_array_node@k\|
+kcalloc_node@k\)(...)
+ ... when != if (...) { ... E =
\(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\|vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\|__vmalloc_node_range\|__vmalloc_node\)(...);
... }
+ when != is_vmalloc_addr(E)
+ when any
+* \(vfree\|vfree_atomic\|kvfree\)(E)@p
+
+@pvfree depends on patch exists@
+expression E;
+position k != choice.kok;
+@@
+
+ E = \(kmalloc@k\|kzalloc@k\|krealloc@k\|kcalloc@k\|kmalloc_node@k\|
+kzalloc_node@k\|kmalloc_array@k\|kmalloc_array_node@k\|
+kcalloc_node@k\)(...)
+ ... when != if (...) { ... E =
\(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\|vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\|__vmalloc_node_range\|__vmalloc_node\)(...);
... }
+ when != is_vmalloc_addr(E)
+ when any
+- \(vfree\|vfree_atomic\|kvfree\)(E)
++ kfree(E)
+
+@kfree depends on !patch@
+expression E;
+position v != choice.vok;
+position p;
+@@
+
+* E = \(vmalloc@v\|vzalloc@v\|vmalloc_user@v\|vmalloc_node@v\|vzalloc_node@v\|
+vmalloc_exec@v\|vmalloc_32@v\|vmalloc_32_user@v\|__vmalloc@v\|
+__vmalloc_node_range@v\|__vmalloc_node@v\)(...)
+
Re: [Cocci] [PATCH v3] coccinelle: api: add kvfree script
> + > +@script: python depends on org@ > +v << kfree.v; > +p << kfree.p; > +@@ > + > +msg = "WARNING: vmalloc is used to allocate this memory at line %s" % > (v[0].line) > +coccilib.org.print_todo(p[0], Just noticed this error. I will resend the patch in 5mins. Regards, Denis ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH v3] coccinelle: api: add kvfree script
Check that alloc and free types of functions match each other.
Signed-off-by: Denis Efremov
---
Changes in v2:
- Lines are limited to 80 characters where possible
- Confidence changed from High to Medium because of
fs/btrfs/send.c:1119 false-positive
- __vmalloc_area_node() explicitly excluded from analysis
instead of !(file in "mm/vmalloc.c") condition
Changes in v3:
- prints style in org && report modes changed for python2
scripts/coccinelle/api/kvfree.cocci | 227
1 file changed, 227 insertions(+)
create mode 100644 scripts/coccinelle/api/kvfree.cocci
diff --git a/scripts/coccinelle/api/kvfree.cocci
b/scripts/coccinelle/api/kvfree.cocci
new file mode 100644
index ..7c396daeacad
--- /dev/null
+++ b/scripts/coccinelle/api/kvfree.cocci
@@ -0,0 +1,227 @@
+// SPDX-License-Identifier: GPL-2.0-only
+///
+/// Check that kvmalloc'ed memory is freed by kfree functions,
+/// vmalloc'ed by vfree functions and kvmalloc'ed by kvfree
+/// functions.
+///
+// Confidence: Medium
+// Copyright: (C) 2020 Denis Efremov ISPRAS
+// Options: --no-includes --include-headers
+//
+
+virtual patch
+virtual report
+virtual org
+virtual context
+
+@initialize:python@
+@@
+# low-level memory api
+filter = frozenset(['__vmalloc_area_node'])
+
+def relevant(p):
+return not (filter & {el.current_element for el in p})
+
+@choice@
+expression E, E1;
+position kok, vok;
+@@
+
+(
+ if (...) {
+...
+E = \(kmalloc@kok\|kzalloc@kok\|krealloc@kok\|kcalloc@kok\|
+ kmalloc_node@kok\|kzalloc_node@kok\|kmalloc_array@kok\|
+ kmalloc_array_node@kok\|kcalloc_node@kok\)(...)
+...
+ } else {
+...
+E = \(vmalloc@vok\|vzalloc@vok\|vmalloc_user@vok\|vmalloc_node@vok\|
+ vzalloc_node@vok\|vmalloc_exec@vok\|vmalloc_32@vok\|
+ vmalloc_32_user@vok\|__vmalloc@vok\|__vmalloc_node_range@vok\|
+ __vmalloc_node@vok\)(...)
+...
+ }
+|
+ E = \(kmalloc\|kzalloc\|krealloc\|kcalloc\|kmalloc_node\|kzalloc_node\|
+kmalloc_array\|kmalloc_array_node\|kcalloc_node\)(...)
+ ... when != E = E1
+ when any
+ if (\(!E\|E == NULL\)) {
+...
+E = \(vmalloc@vok\|vzalloc@vok\|vmalloc_user@vok\|vmalloc_node@vok\|
+ vzalloc_node@vok\|vmalloc_exec@vok\|vmalloc_32@vok\|
+ vmalloc_32_user@vok\|__vmalloc@vok\|__vmalloc_node_range@vok\|
+ __vmalloc_node@vok\)(...)
+...
+ }
+)
+
+@opportunity depends on !patch@
+expression E, E1, size;
+position p : script:python() { relevant(p) };
+@@
+
+(
+* if (\(size <= E1\|size < E1\|size = E1\|size > E1\) || ...)@p {
+...
+E = \(kmalloc\|kzalloc\|krealloc\|kcalloc\|kmalloc_node\|kzalloc_node\|
+ kmalloc_array\|kmalloc_array_node\|kcalloc_node\)(..., size, ...)
+...
+ } else {
+...
+E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\|
+ vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\|
+ __vmalloc_node_range\|__vmalloc_node\)(..., size, ...)
+...
+ }
+|
+ E = \(kmalloc\|kzalloc\|krealloc\|kcalloc\|kmalloc_node\|kzalloc_node\|
+kmalloc_array\|kmalloc_array_node\|kcalloc_node\)(..., size, ...)
+ ... when != E = E1
+ when != size = E1
+ when any
+* if (\(!E\|E == NULL\))@p {
+...
+E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\|
+ vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\|
+ __vmalloc_node_range\|__vmalloc_node\)(..., size, ...)
+...
+ }
+)
+
+@vfree depends on !patch@
+expression E;
+position k != choice.kok;
+position p;
+@@
+
+* E = \(kmalloc@k\|kzalloc@k\|krealloc@k\|kcalloc@k\|kmalloc_node@k\|
+kzalloc_node@k\|kmalloc_array@k\|kmalloc_array_node@k\|
+kcalloc_node@k\)(...)
+ ... when != if (...) { ... E =
\(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\|vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\|__vmalloc_node_range\|__vmalloc_node\)(...);
... }
+ when != is_vmalloc_addr(E)
+ when any
+* \(vfree\|vfree_atomic\|kvfree\)(E)@p
+
+@pvfree depends on patch exists@
+expression E;
+position k != choice.kok;
+@@
+
+ E = \(kmalloc@k\|kzalloc@k\|krealloc@k\|kcalloc@k\|kmalloc_node@k\|
+kzalloc_node@k\|kmalloc_array@k\|kmalloc_array_node@k\|
+kcalloc_node@k\)(...)
+ ... when != if (...) { ... E =
\(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\|vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\|__vmalloc_node_range\|__vmalloc_node\)(...);
... }
+ when != is_vmalloc_addr(E)
+ when any
+- \(vfree\|vfree_atomic\|kvfree\)(E)
++ kfree(E)
+
+@kfree depends on !patch@
+expression E;
+position v != choice.vok;
+position p;
+@@
+
+* E = \(vmalloc@v\|vzalloc@v\|vmalloc_user@v\|vmalloc_node@v\|vzalloc_node@v\|
+vmalloc_exec@v\|vmalloc_32@v\|vmalloc_32_user@v\|__vmalloc@v\|
+__vmalloc_node_range@v\|__vmalloc_node@v\)(...)
+ ... when != !is_vmalloc_addr(E)
+ when any
+* \(kfree\|kzfree\
[Cocci] [PATCH v3 1/3] coccinelle: api: extend memdup_user transformation with GFP_USER
Match GFP_USER and optional __GFP_NOWARN allocations with
memdup_user.cocci rule.
Commit 6c2c97a24f09 ("memdup_user(): switch to GFP_USER") switched
memdup_user() from GFP_KERNEL to GFP_USER. In almost all cases it
is still a good idea to recommend memdup_user() for GFP_KERNEL
allocations. The motivation behind altering memdup_user() to GFP_USER:
https://lkml.org/lkml/2018/1/6/333
Signed-off-by: Denis Efremov
---
scripts/coccinelle/api/memdup_user.cocci | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/scripts/coccinelle/api/memdup_user.cocci
b/scripts/coccinelle/api/memdup_user.cocci
index c809ab10bbce..0e29d41ecab8 100644
--- a/scripts/coccinelle/api/memdup_user.cocci
+++ b/scripts/coccinelle/api/memdup_user.cocci
@@ -20,7 +20,9 @@ expression from,to,size;
identifier l1,l2;
@@
-- to = \(kmalloc\|kzalloc\)(size,GFP_KERNEL);
+- to = \(kmalloc\|kzalloc\)
+- (size,\(GFP_KERNEL\|GFP_USER\|
+-\(GFP_KERNEL\|GFP_USER\)|__GFP_NOWARN\));
+ to = memdup_user(from,size);
if (
- to==NULL
@@ -43,7 +45,9 @@ position p;
statement S1,S2;
@@
-* to = \(kmalloc@p\|kzalloc@p\)(size,GFP_KERNEL);
+* to = \(kmalloc@p\|kzalloc@p\)
+ (size,\(GFP_KERNEL\|GFP_USER\|
+ \(GFP_KERNEL\|GFP_USER\)|__GFP_NOWARN\));
if (to==NULL || ...) S1
if (copy_from_user(to, from, size) != 0)
S2
--
2.26.2
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH v3 3/3] coccinelle: api: filter out memdup_user definitions
Don't match memdup_user/vmemdup_user.
Signed-off-by: Denis Efremov
---
scripts/coccinelle/api/memdup_user.cocci | 17 +
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/scripts/coccinelle/api/memdup_user.cocci
b/scripts/coccinelle/api/memdup_user.cocci
index 60027e21c5e6..e01e95108405 100644
--- a/scripts/coccinelle/api/memdup_user.cocci
+++ b/scripts/coccinelle/api/memdup_user.cocci
@@ -15,12 +15,20 @@ virtual context
virtual org
virtual report
+@initialize:python@
+@@
+filter = frozenset(['memdup_user', 'vmemdup_user'])
+
+def relevant(p):
+return not (filter & {el.current_element for el in p})
+
@depends on patch@
expression from,to,size;
identifier l1,l2;
+position p : script:python() { relevant(p) };
@@
-- to = \(kmalloc\|kzalloc\)
+- to = \(kmalloc@p\|kzalloc@p\)
- (size,\(GFP_KERNEL\|GFP_USER\|
-\(GFP_KERNEL\|GFP_USER\)|__GFP_NOWARN\));
+ to = memdup_user(from,size);
@@ -42,9 +50,10 @@ identifier l1,l2;
@depends on patch@
expression from,to,size;
identifier l1,l2;
+position p : script:python() { relevant(p) };
@@
-- to = \(kvmalloc\|kvzalloc\)(size,\(GFP_KERNEL\|GFP_USER\));
+- to = \(kvmalloc@p\|kvzalloc@p\)(size,\(GFP_KERNEL\|GFP_USER\));
+ to = vmemdup_user(from,size);
if (
- to==NULL
@@ -63,7 +72,7 @@ identifier l1,l2;
@r depends on !patch@
expression from,to,size;
-position p;
+position p : script:python() { relevant(p) };
statement S1,S2;
@@
@@ -76,7 +85,7 @@ statement S1,S2;
@rv depends on !patch@
expression from,to,size;
-position p;
+position p : script:python() { relevant(p) };
statement S1,S2;
@@
--
2.26.2
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH v3 0/3] Update memdup_user.cocci
Add GFP_USER to the allocation flags and handle vmemdup_user(). Changes in v2: - memdup_user/vmemdup_user matching suppressed - PoC for selfcheck virtual rule Changes in v3: - add missing '-' for patch rule in kmalloc/kzalloc call args - selfcheck rule dropped from patchset Denis Efremov (3): coccinelle: api: extend memdup_user transformation with GFP_USER coccinelle: api: extend memdup_user rule with vmemdup_user() coccinelle: api: filter out memdup_user definitions scripts/coccinelle/api/memdup_user.cocci | 64 ++-- 1 file changed, 61 insertions(+), 3 deletions(-) -- 2.26.2 ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH v3 2/3] coccinelle: api: extend memdup_user rule with vmemdup_user()
Add vmemdup_user() transformations to the memdup_user.cocci rule.
Commit 50fd2f298bef ("new primitive: vmemdup_user()") introduced
vmemdup_user(). The function uses kvmalloc with GPF_USER flag.
Signed-off-by: Denis Efremov
---
scripts/coccinelle/api/memdup_user.cocci | 45
1 file changed, 45 insertions(+)
diff --git a/scripts/coccinelle/api/memdup_user.cocci
b/scripts/coccinelle/api/memdup_user.cocci
index 0e29d41ecab8..60027e21c5e6 100644
--- a/scripts/coccinelle/api/memdup_user.cocci
+++ b/scripts/coccinelle/api/memdup_user.cocci
@@ -39,6 +39,28 @@ identifier l1,l2;
-...+>
- }
+@depends on patch@
+expression from,to,size;
+identifier l1,l2;
+@@
+
+- to = \(kvmalloc\|kvzalloc\)(size,\(GFP_KERNEL\|GFP_USER\));
++ to = vmemdup_user(from,size);
+ if (
+- to==NULL
++ IS_ERR(to)
+ || ...) {
+ <+... when != goto l1;
+- -ENOMEM
++ PTR_ERR(to)
+ ...+>
+ }
+- if (copy_from_user(to, from, size) != 0) {
+-<+... when != goto l2;
+--EFAULT
+-...+>
+- }
+
@r depends on !patch@
expression from,to,size;
position p;
@@ -52,6 +74,17 @@ statement S1,S2;
if (copy_from_user(to, from, size) != 0)
S2
+@rv depends on !patch@
+expression from,to,size;
+position p;
+statement S1,S2;
+@@
+
+* to = \(kvmalloc@p\|kvzalloc@p\)(size,\(GFP_KERNEL\|GFP_USER\));
+ if (to==NULL || ...) S1
+ if (copy_from_user(to, from, size) != 0)
+ S2
+
@script:python depends on org@
p << r.p;
@@
@@ -63,3 +96,15 @@ p << r.p;
@@
coccilib.report.print_report(p[0], "WARNING opportunity for memdup_user")
+
+@script:python depends on org@
+p << rv.p;
+@@
+
+coccilib.org.print_todo(p[0], "WARNING opportunity for vmemdup_user")
+
+@script:python depends on report@
+p << rv.p;
+@@
+
+coccilib.report.print_report(p[0], "WARNING opportunity for vmemdup_user")
--
2.26.2
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [v2 1/4] coccinelle: api: extend memdup_user transformation with GFP_USER
Hi, On 7/18/20 9:45 AM, Julia Lawall wrote: > This on is indeed a problem. I think it was not detected in testing, > because in the current kernel the rule never applies. But Denis, in > > - to = \(kmalloc\|kzalloc\) > (size,\(GFP_KERNEL\|GFP_USER\| > \(GFP_KERNEL\|GFP_USER\)|__GFP_NOWARN\)); > > you do indeed need to put - in front of the second and third lines as > well. Thanks, Markus, Julia. I will send v3. Julia, is it ok with you, if I will drop the last patch with "selfcheck" this time? Regards, Denis ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH v2] coccinelle: api: add kvfree script
Ping? ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH v2 0/4] Update memdup_user.cocci
Ping? On 6/8/20 6:00 PM, Denis Efremov wrote: > Add GFP_USER to the allocation flags and handle vmemdup_user(). > The third patch supresses memdup_user(), vmemdup_user() functions > detection. Last patch is a proof of concept for the rule selfchecking. > Gives the ability to detect that an open-coded pattern in a function > definition that we search for in the kernel sources changed. > > Denis Efremov (4): > coccinelle: api: extend memdup_user transformation with GFP_USER > coccinelle: api: extend memdup_user rule with vmemdup_user() > coccinelle: api: filter out memdup_user definitions > coccinelle: api: add selfcheck for memdup_user rule > > scripts/coccinelle/api/memdup_user.cocci | 106 ++- > 1 file changed, 103 insertions(+), 3 deletions(-) > ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH v4] coccinelle: api: add kzfree script
Check for memset()/memzero_explicit() followed by kfree()/vfree()/kvfree().
Signed-off-by: Denis Efremov
---
Changes in v2:
- memset_explicit() added
- kvfree_sensitive() added
- forall added to r1
- ... between memset and kfree added
Changes in v3:
- Explicit filter for definitions instead of !(file in "...") conditions
- type T added to match casts
- memzero_explicit() patterns fixed
- additional rule "cond" added to filter false-positives
Changes in v4:
- memset call fixed in rp_memset
- @m added to rp_memset,rp_memzero rules
scripts/coccinelle/api/kzfree.cocci | 101
1 file changed, 101 insertions(+)
create mode 100644 scripts/coccinelle/api/kzfree.cocci
diff --git a/scripts/coccinelle/api/kzfree.cocci
b/scripts/coccinelle/api/kzfree.cocci
new file mode 100644
index ..33625bd7cec9
--- /dev/null
+++ b/scripts/coccinelle/api/kzfree.cocci
@@ -0,0 +1,101 @@
+// SPDX-License-Identifier: GPL-2.0-only
+///
+/// Use kzfree, kvfree_sensitive rather than memset or
+/// memzero_explicit followed by kfree
+///
+// Confidence: High
+// Copyright: (C) 2020 Denis Efremov ISPRAS
+// Options: --no-includes --include-headers
+//
+// Keywords: kzfree, kvfree_sensitive
+//
+
+virtual context
+virtual patch
+virtual org
+virtual report
+
+@initialize:python@
+@@
+# kmalloc_oob_in_memset uses memset to explicitly trigger out-of-bounds access
+filter = frozenset(['kmalloc_oob_in_memset', 'kzfree', 'kvfree_sensitive'])
+
+def relevant(p):
+return not (filter & {el.current_element for el in p})
+
+@cond@
+position ok;
+@@
+
+if (...)
+ \(memset@ok\|memzero_explicit@ok\)(...);
+
+@r depends on !patch forall@
+expression E;
+position p : script:python() { relevant(p) };
+position m != cond.ok;
+type T;
+@@
+
+(
+* memset@m((T)E, 0, ...);
+|
+* memzero_explicit@m((T)E, ...);
+)
+ ... when != E
+ when strict
+* \(kfree\|vfree\|kvfree\)(E)@p;
+
+@rp_memzero depends on patch@
+expression E, size;
+position p : script:python() { relevant(p) };
+position m != cond.ok;
+type T;
+@@
+
+- memzero_explicit@m((T)E, size);
+ ... when != E
+ when strict
+// TODO: uncomment when kfree_sensitive will be merged.
+// Only this case is commented out because developers
+// may not like patches like this since kzfree uses memset
+// internally (not memzero_explicit).
+//(
+//- kfree(E)@p;
+//+ kfree_sensitive(E);
+//|
+- \(vfree\|kvfree\)(E)@p;
++ kvfree_sensitive(E, size);
+//)
+
+@rp_memset depends on patch@
+expression E, size;
+position p : script:python() { relevant(p) };
+position m != cond.ok;
+type T;
+@@
+
+- memset@m((T)E, 0, size);
+ ... when != E
+ when strict
+(
+- kfree(E)@p;
++ kzfree(E);
+|
+- \(vfree\|kvfree\)(E)@p;
++ kvfree_sensitive(E, size);
+)
+
+@script:python depends on report@
+p << r.p;
+@@
+
+coccilib.report.print_report(p[0],
+ "WARNING: opportunity for kzfree/kvfree_sensitive")
+
+@script:python depends on org@
+p << r.p;
+@@
+
+coccilib.org.print_todo(p[0],
+ "WARNING: opportunity for kzfree/kvfree_sensitive")
--
2.26.2
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH v4] coccinelle: misc: add array_size_dup script to detect missed overflow checks
Detect an opencoded expression that is used before or after array_size()/array3_size()/struct_size() to compute the same size. Signed-off-by: Denis Efremov --- Changes in v2: - python rules moved next to SmPL patterns - assignment operator used - struct_size patterns fixed to check only E3, since E1, E2 are sizeofs of a structure and a member of a structure Changes in v3: - s/overlow/overflow/ typo fixed (thanks, Markus) - \(\|\) changed to &\(E1\|E2\) - print strings breaks removed Changes in v4: - duplicates warning removed - python2 compatability in report& modes added - s/down the code/later/ warning changed - \(E1\|E2\|subE1\|subE2\) patterns simplified to \(subE1\|subE2\) scripts/coccinelle/misc/array_size_dup.cocci | 209 +++ 1 file changed, 209 insertions(+) create mode 100644 scripts/coccinelle/misc/array_size_dup.cocci diff --git a/scripts/coccinelle/misc/array_size_dup.cocci b/scripts/coccinelle/misc/array_size_dup.cocci new file mode 100644 index ..d3d635b2d4fc --- /dev/null +++ b/scripts/coccinelle/misc/array_size_dup.cocci @@ -0,0 +1,209 @@ +// SPDX-License-Identifier: GPL-2.0-only +/// +/// Check for array_size(), array3_size(), struct_size() duplicates. +/// Three types of patterns for these functions: +/// 1. An opencoded expression is used before array_size() to compute the same size +/// 2. An opencoded expression is used after array_size() to compute the same size +/// From security point of view only first case is relevant. These functions +/// perform arithmetic overflow check. Thus, if we use an opencoded expression +/// before a call to the *_size() function we can miss an overflow. +/// +// Confidence: High +// Copyright: (C) 2020 Denis Efremov ISPRAS +// Options: --no-includes --include-headers --no-loops + +virtual context +virtual report +virtual org + +@as@ +expression E1, E2; +@@ + +array_size(E1, E2) + +@as_next@ +expression subE1 <= as.E1; +expression subE2 <= as.E2; +expression as.E1, as.E2, E3; +assignment operator aop; +position p1, p2; +@@ + +* E1 * E2@p1 + ... when != \(subE1\|subE2\) aop E3 + when != &\(subE1\|subE2\) +* array_size(E1, E2)@p2 + +@script:python depends on report@ +p1 << as_next.p1; +p2 << as_next.p2; +@@ + +msg = "WARNING: array_size is used later (line %s) to compute the same size" % (p2[0].line) +coccilib.report.print_report(p1[0], msg) + +@script:python depends on org@ +p1 << as_next.p1; +p2 << as_next.p2; +@@ + +msg = "WARNING: array_size is used later (line %s) to compute the same size" % (p2[0].line) +coccilib.org.print_todo(p1[0], msg) + +@as_prev@ +expression subE1 <= as.E1; +expression subE2 <= as.E2; +expression as.E1, as.E2, E3; +assignment operator aop; +position p1, p2; +@@ + +* array_size(E1, E2)@p1 + ... when != \(subE1\|subE2\) aop E3 + when != &\(subE1\|subE2\) +* E1 * E2@p2 + +@script:python depends on report@ +p1 << as_prev.p1; +p2 << as_prev.p2; +@@ + +msg = "WARNING: array_size is already used (line %s) to compute the same size" % (p1[0].line) +coccilib.report.print_report(p2[0], msg) + +@script:python depends on org@ +p1 << as_prev.p1; +p2 << as_prev.p2; +@@ + +msg = "WARNING: array_size is already used (line %s) to compute the same size" % (p1[0].line) +coccilib.org.print_todo(p2[0], msg) + +@as3@ +expression E1, E2, E3; +@@ + +array3_size(E1, E2, E3) + +@as3_next@ +expression subE1 <= as3.E1; +expression subE2 <= as3.E2; +expression subE3 <= as3.E3; +expression as3.E1, as3.E2, as3.E3, E4; +assignment operator aop; +position p1, p2; +@@ + +* E1 * E2 * E3@p1 + ... when != \(subE1\|subE2\|subE3\) aop E4 + when != &\(subE1\|subE2\|subE3\) +* array3_size(E1, E2, E3)@p2 + +@script:python depends on report@ +p1 << as3_next.p1; +p2 << as3_next.p2; +@@ + +msg = "WARNING: array3_size is used later (line %s) to compute the same size" % (p2[0].line) +coccilib.report.print_report(p1[0], msg) + +@script:python depends on org@ +p1 << as3_next.p1; +p2 << as3_next.p2; +@@ + +msg = "WARNING: array3_size is used later (line %s) to compute the same size" % (p2[0].line) +coccilib.org.print_todo(p1[0], msg) + +@as3_prev@ +expression subE1 <= as3.E1; +expression subE2 <= as3.E2; +expression subE3 <= as3.E3; +expression as3.E1, as3.E2, as3.E3, E4; +assignment operator aop; +position p1, p2; +@@ + +* array3_size(E1, E2, E3)@p1 + ... when != \(subE1\|subE2\|subE3\) aop E4 + when != &\(subE1\|subE2\|subE3\) +* E1 * E2 * E3@p2 + +@script:python depends on report@ +p1 << as3_prev.p1; +p2 << as3_prev.p2; +@@ + +msg = "WARNING: array3_size is already used (line %s) to compute the same size" % (p1[0].line) +coccilib.report.print_report(p2[0], msg) + +@script:python depends on org@ +p1 << as3_prev.p1; +p2 << as3_prev.p2; +@@ + +msg = "WARNING: array3_
Re: [Cocci] [PATCH v3] coccinelle: misc: add array_size_dup script to detect missed overflow checks
What do you think about removing duplicates warning from the rule?
I mean this kind of warnings: "WARNING: same array_size (line {p1[0].line})"
As for now, I think it's better to not disturb developers with this kind
of things.
Thanks,
Denis
>> +@as_dup@
>> +expression subE1 <= as.E1;
>> +expression subE2 <= as.E2;
>> +expression as.E1, as.E2, E3;
>> +assignment operator aop;
>> +position p1, p2;
>> +@@
>> +
>> +* array_size(E1, E2)@p1
>> + ... when != \(E1\|E2\|subE1\|subE2\) aop E3
>> + when != &\(E1\|E2\|subE1\|subE2\)
>> +* array_size(E1, E2)@p2
>> +
>> +@script:python depends on report@
>> +p1 << as_dup.p1;
>> +p2 << as_dup.p2;
>> +@@
>> +
>> +coccilib.report.print_report(p2[0],
>> +f"WARNING: same array_size (line {p1[0].line})")
>> +
>> +@script:python depends on org@
>> +p1 << as_dup.p1;
>> +p2 << as_dup.p2;
>> +@@
>> +
>> +coccilib.org.print_todo(p2[0],
>> +f"WARNING: same array_size (line {p1[0].line})")
>> +
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH v3] coccinelle: misc: add array_size_dup script to detect missed overflow checks
On 6/21/20 11:56 PM, Julia Lawall wrote: > Is it a python 2 vs python 3 thing? Yes, python2 is no longer supported and I thought it would be safe to use this syntax. Ok, I will make it portable in v4. Denis ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH] coccinelle: api/kstrdup: fix coccinelle position
There is a typo in rule r2. Position p1 should be attached to kzalloc()
call.
Fixes: 29a36d4dec6c ("scripts/coccinelle: improve the coverage of some semantic
patches")
Signed-off-by: Denis Efremov
---
scripts/coccinelle/api/kstrdup.cocci | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/coccinelle/api/kstrdup.cocci
b/scripts/coccinelle/api/kstrdup.cocci
index 19f2645e6076..3c6dc5469ee4 100644
--- a/scripts/coccinelle/api/kstrdup.cocci
+++ b/scripts/coccinelle/api/kstrdup.cocci
@@ -66,7 +66,7 @@ position p1,p2;
* x = strlen(from) + 1;
... when != \( x = E1 \| from = E1 \)
-* to = \(kmalloc@p1\|kzalloc@p2\)(x,flag);
+* to = \(kmalloc@p1\|kzalloc@p1\)(x,flag);
... when != \(x = E2 \| from = E2 \| to = E2 \)
if (to==NULL || ...) S
... when != \(x = E3 \| from = E3 \| to = E3 \)
--
2.26.2
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH v3] coccinelle: misc: add array_size_dup script to detect missed overflow checks
Detect an opencoded expression that is used before or after
array_size()/array3_size()/struct_size() to compute the same size.
Cc: Gustavo A. R. Silva
Cc: Kees Cook
Signed-off-by: Denis Efremov
---
Changes in v2:
- python rules moved next to SmPL patterns
- assignment operator used
- struct_size patterns fixed to check only E3, since
E1, E2 are sizeofs of a structure and a member
of a structure
Changes in v3:
- s/overlow/overflow/ typo fixed (thanks, Markus)
- \(\|\) changed to &\(E1\|E2\)
- print strings breaks removed
scripts/coccinelle/misc/array_size_dup.cocci | 297 +++
1 file changed, 297 insertions(+)
create mode 100644 scripts/coccinelle/misc/array_size_dup.cocci
diff --git a/scripts/coccinelle/misc/array_size_dup.cocci
b/scripts/coccinelle/misc/array_size_dup.cocci
new file mode 100644
index ..d03740257e97
--- /dev/null
+++ b/scripts/coccinelle/misc/array_size_dup.cocci
@@ -0,0 +1,297 @@
+// SPDX-License-Identifier: GPL-2.0-only
+///
+/// Check for array_size(), array3_size(), struct_size() duplicates.
+/// Three types of patterns for these functions:
+/// 1. An opencoded expression is used before array_size() to compute the
same size
+/// 2. An opencoded expression is used after array_size() to compute the same
size
+/// 3. Consecutive calls of array_size() with the same values
+/// From security point of view only first case is relevant. These functions
+/// perform arithmetic overflow check. Thus, if we use an opencoded expression
+/// before a call to the *_size() function we can miss an overflow.
+///
+// Confidence: High
+// Copyright: (C) 2020 Denis Efremov ISPRAS
+// Options: --no-includes --include-headers --no-loops
+
+virtual context
+virtual report
+virtual org
+
+@as@
+expression E1, E2;
+@@
+
+array_size(E1, E2)
+
+@as_next@
+expression subE1 <= as.E1;
+expression subE2 <= as.E2;
+expression as.E1, as.E2, E3;
+assignment operator aop;
+position p1, p2;
+@@
+
+* E1 * E2@p1
+ ... when != \(E1\|E2\|subE1\|subE2\) aop E3
+ when != &\(E1\|E2\|subE1\|subE2\)
+* array_size(E1, E2)@p2
+
+@script:python depends on report@
+p1 << as_next.p1;
+p2 << as_next.p2;
+@@
+
+coccilib.report.print_report(p1[0],
+f"WARNING: array_size is used down the code (line {p2[0].line}) to compute the
same size")
+
+@script:python depends on org@
+p1 << as_next.p1;
+p2 << as_next.p2;
+@@
+
+coccilib.org.print_todo(p1[0],
+f"WARNING: array_size is used down the code (line {p2[0].line}) to compute the
same size")
+
+@as_prev@
+expression subE1 <= as.E1;
+expression subE2 <= as.E2;
+expression as.E1, as.E2, E3;
+assignment operator aop;
+position p1, p2;
+@@
+
+* array_size(E1, E2)@p1
+ ... when != \(E1\|E2\|subE1\|subE2\) aop E3
+ when != &\(E1\|E2\|subE1\|subE2\)
+* E1 * E2@p2
+
+@script:python depends on report@
+p1 << as_prev.p1;
+p2 << as_prev.p2;
+@@
+
+coccilib.report.print_report(p2[0],
+f"WARNING: array_size is already used (line {p1[0].line}) to compute the same
size")
+
+@script:python depends on org@
+p1 << as_prev.p1;
+p2 << as_prev.p2;
+@@
+
+coccilib.org.print_todo(p2[0],
+f"WARNING: array_size is already used (line {p1[0].line}) to compute the same
size")
+
+@as_dup@
+expression subE1 <= as.E1;
+expression subE2 <= as.E2;
+expression as.E1, as.E2, E3;
+assignment operator aop;
+position p1, p2;
+@@
+
+* array_size(E1, E2)@p1
+ ... when != \(E1\|E2\|subE1\|subE2\) aop E3
+ when != &\(E1\|E2\|subE1\|subE2\)
+* array_size(E1, E2)@p2
+
+@script:python depends on report@
+p1 << as_dup.p1;
+p2 << as_dup.p2;
+@@
+
+coccilib.report.print_report(p2[0],
+f"WARNING: same array_size (line {p1[0].line})")
+
+@script:python depends on org@
+p1 << as_dup.p1;
+p2 << as_dup.p2;
+@@
+
+coccilib.org.print_todo(p2[0],
+f"WARNING: same array_size (line {p1[0].line})")
+
+@as3@
+expression E1, E2, E3;
+@@
+
+array3_size(E1, E2, E3)
+
+@as3_next@
+expression subE1 <= as3.E1;
+expression subE2 <= as3.E2;
+expression subE3 <= as3.E3;
+expression as3.E1, as3.E2, as3.E3, E4;
+assignment operator aop;
+position p1, p2;
+@@
+
+* E1 * E2 * E3@p1
+ ... when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\) aop E4
+ when != &\(E1\|E2\|E3\|subE1\|subE2\|subE3\)
+* array3_size(E1, E2, E3)@p2
+
+@script:python depends on report@
+p1 << as3_next.p1;
+p2 << as3_next.p2;
+@@
+
+coccilib.report.print_report(p1[0],
+f"WARNING: array3_size is used down the code (line {p2[0].line}) to compute
the same size")
+
+@script:python depends on org@
+p1 << as3_next.p1;
+p2 << as3_next.p2;
+@@
+
+coccilib.org.print_todo(p1[0],
+f"WARNING: array3_size is used down the code (line {p2[0].line}) to compute
the same size")
+
+@as3_prev@
+expression subE1 <= as3.E1;
+expression subE2 <= as3.E2;
+expression subE3 <= as3.E3;
+expression as3.E1, as3.E2, as3.E3
Re: [Cocci] [PATCH v2] coccinelle: misc: add array_size_dup script to detect missed overflow checks
Hi,
On 6/18/20 2:34 PM, Markus Elfring wrote:
> Why did you repeat a typo from the previous patch subject?
Where is the typo? I can't handle your suggestions because your mails constantly
break the threads. I just can't find them after due to missed/wrong In-Reply-To
headers. Again, this mail doesn't contain In-Reply-To header and highly likely I
will miss it when I will prepare next version of the patch.
>> +expression subE1 <= as.E1;
>> +expression subE2 <= as.E2;
>> +expression as.E1, as.E2, E3;
>
> How do you think about to use the following SmPL code variant?
>
> expression subE1 <= as.E1, subE2 <= as.E2, as.E1, as.E2, E3;
It's less readable and harder to review.
>
>> + when != \(\|\|\|\)
>
> I suggest to move the ampersand before the disjunction in such
> SmPL code exclusion specifications.
>
> + when != & \(E1 \| E2 \| subE1 \| subE2\)
Ok, I will fix this if there will be next version.
>
>
>> +coccilib.report.print_report(p2[0],
>> +f"WARNING: array_size is already used (line {p1[0].line}) to compute \
>> +the same size")
>
> I would prefer an other code formatting at such places.
>
> +coccilib.report.print_report(p2[0],
> + f"WARNING: array_size is already used (line
> {p1[0].line}) to compute the same size.")
>
No. It's pointless to break the line to save 5 chars this way.
I can use instead:
coccilib.report.print_report(p2[0], f"WARNING: array_size is already used (line
{p1[0].line}) to compute the same size")
or
msg = f"WARNING: array_size is already used (line {p1[0].line}) to compute the
same size"
coccilib.report.print_report(p2[0], msg)
or
coccilib.report.print_report(p2[0],
f"WARNING: array_size is already used (line {p1[0].line}) to compute the same
size")
And I prefer the last one if Julia will allow me to use more than 80 chars in
print string.
Thanks,
Denis
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH v2] coccinelle: misc: add array_size_dup script to detect missed overlow checks
Detect an opencoded expression that is used before or after
array_size()/array3_size()/struct_size() to compute the same size.
Cc: Gustavo A. R. Silva
Cc: Kees Cook
Signed-off-by: Denis Efremov
---
Changes in v2:
- python rules moved next to SmPL patterns
- assignment operator used
- struct_size patterns fixed to check only E3, since
E1, E2 are sizeofs of a structure and a member
of a structure
scripts/coccinelle/misc/array_size_dup.cocci | 309 +++
1 file changed, 309 insertions(+)
create mode 100644 scripts/coccinelle/misc/array_size_dup.cocci
diff --git a/scripts/coccinelle/misc/array_size_dup.cocci
b/scripts/coccinelle/misc/array_size_dup.cocci
new file mode 100644
index ..c5214310285c
--- /dev/null
+++ b/scripts/coccinelle/misc/array_size_dup.cocci
@@ -0,0 +1,309 @@
+// SPDX-License-Identifier: GPL-2.0-only
+///
+/// Check for array_size(), array3_size(), struct_size() duplicates.
+/// Three types of patterns for these functions:
+/// 1. An opencoded expression is used before array_size() to compute the
same size
+/// 2. An opencoded expression is used after array_size() to compute the same
size
+/// 3. Consecutive calls of array_size() with the same values
+/// From security point of view only first case is relevant. These functions
+/// perform arithmetic overflow check. Thus, if we use an opencoded expression
+/// before a call to the *_size() function we can miss an overflow.
+///
+// Confidence: High
+// Copyright: (C) 2020 Denis Efremov ISPRAS
+// Options: --no-includes --include-headers --no-loops
+
+virtual context
+virtual report
+virtual org
+
+@as@
+expression E1, E2;
+@@
+
+array_size(E1, E2)
+
+@as_next@
+expression subE1 <= as.E1;
+expression subE2 <= as.E2;
+expression as.E1, as.E2, E3;
+assignment operator aop;
+position p1, p2;
+@@
+
+* E1 * E2@p1
+ ... when != \(E1\|E2\|subE1\|subE2\) aop E3
+ when != \(\|\|\|\)
+* array_size(E1, E2)@p2
+
+@script:python depends on report@
+p1 << as_next.p1;
+p2 << as_next.p2;
+@@
+
+coccilib.report.print_report(p1[0],
+f"WARNING: array_size is used down the code (line {p2[0].line}) to compute \
+the same size")
+
+@script:python depends on org@
+p1 << as_next.p1;
+p2 << as_next.p2;
+@@
+
+coccilib.org.print_todo(p1[0],
+f"WARNING: array_size is used down the code (line {p2[0].line}) to compute \
+the same size")
+
+@as_prev@
+expression subE1 <= as.E1;
+expression subE2 <= as.E2;
+expression as.E1, as.E2, E3;
+assignment operator aop;
+position p1, p2;
+@@
+
+* array_size(E1, E2)@p1
+ ... when != \(E1\|E2\|subE1\|subE2\) aop E3
+ when != \(\|\|\|\)
+* E1 * E2@p2
+
+@script:python depends on report@
+p1 << as_prev.p1;
+p2 << as_prev.p2;
+@@
+
+coccilib.report.print_report(p2[0],
+f"WARNING: array_size is already used (line {p1[0].line}) to compute \
+the same size")
+
+@script:python depends on org@
+p1 << as_prev.p1;
+p2 << as_prev.p2;
+@@
+
+coccilib.org.print_todo(p2[0],
+f"WARNING: array_size is already used (line {p1[0].line}) to compute \
+the same size")
+
+@as_dup@
+expression subE1 <= as.E1;
+expression subE2 <= as.E2;
+expression as.E1, as.E2, E3;
+assignment operator aop;
+position p1, p2;
+@@
+
+* array_size(E1, E2)@p1
+ ... when != \(E1\|E2\|subE1\|subE2\) aop E3
+ when != \(\|\|\|\)
+* array_size(E1, E2)@p2
+
+@script:python depends on report@
+p1 << as_dup.p1;
+p2 << as_dup.p2;
+@@
+
+coccilib.report.print_report(p2[0],
+f"WARNING: same array_size (line {p1[0].line})")
+
+@script:python depends on org@
+p1 << as_dup.p1;
+p2 << as_dup.p2;
+@@
+
+coccilib.org.print_todo(p2[0],
+f"WARNING: same array_size (line {p1[0].line})")
+
+@as3@
+expression E1, E2, E3;
+@@
+
+array3_size(E1, E2, E3)
+
+@as3_next@
+expression subE1 <= as3.E1;
+expression subE2 <= as3.E2;
+expression subE3 <= as3.E3;
+expression as3.E1, as3.E2, as3.E3, E4;
+assignment operator aop;
+position p1, p2;
+@@
+
+* E1 * E2 * E3@p1
+ ... when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\) aop E4
+ when != \(\|\|\|\|\|\)
+* array3_size(E1, E2, E3)@p2
+
+@script:python depends on report@
+p1 << as3_next.p1;
+p2 << as3_next.p2;
+@@
+
+coccilib.report.print_report(p1[0],
+f"WARNING: array3_size is used down the code (line {p2[0].line}) to compute \
+the same size")
+
+@script:python depends on org@
+p1 << as3_next.p1;
+p2 << as3_next.p2;
+@@
+
+coccilib.org.print_todo(p1[0],
+f"WARNING: array3_size is used down the code (line {p2[0].line}) to compute \
+the same size")
+
+@as3_prev@
+expression subE1 <= as3.E1;
+expression subE2 <= as3.E2;
+expression subE3 <= as3.E3;
+expression as3.E1, as3.E2, as3.E3, E4;
+assignment operator aop;
+position p1, p2;
+@@
+
+* array3_size(E1, E2, E3)@p1
+ ... when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\) aop E4
+ when != \(\|\|\|\|\|\)
+* E1 * E2 * E
Re: [Cocci] [PATCH v3] coccinelle: api: add kzfree script
>> +@rp_memset depends on patch@
>> +expression E, size;
>> +position p : script:python() { relevant(p) };
>> +type T;
>> +@@
>> +
>> +- memset((T)E, size)@p;
>
> This is missing a 0 argument.
>
Thanks, I will send v4.
>
>
>> + ... when != E
>> + when strict
>> +(
>> +- kfree(E);
>> ++ kzfree(E);
>> +|
>> +- \(vfree\|kvfree\)(E);
>> ++ kvfree_sensitive(E, size);
>> +)
>
> I'm not sure why you want kzfree in the first case, but kvfree_sensitive
> in the second case.
>
As for now in kernel:
memset(E,0,...) && kfree(E) is kzfree()
There are no vzfree or kvzfree functions.
Thus, we use kvfree_sensitive().
Maybe it's worth to wait for this patchset:
https://lkml.org/lkml/2020/6/16/1163
With it the rule will use:
(
- kfree(E);
+ kfree_sensitive(E);
|
- \(vfree\|kvfree\)(E);
+ kvfree_sensitive(E, size);
)
Thanks,
Denis
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH] coccinelle: misc: add array_size_dup script to detect missed overlow checks
On 6/17/20 11:30 PM, Julia Lawall wrote: > > > On Mon, 15 Jun 2020, Denis Efremov wrote: > >> Detect an opencoded expression that is used before or after >> array_size()/array3_size()/struct_size() to compute the same size. > > This would benefit from the assignemnt operator metavariables as well. > > Also, it could be better to put the python rules up next the SmPL pattern > matching rules that they are associated with. > Thanks, I will send v2. Here is the KSPP ticket with patches https://github.com/KSPP/linux/issues/83 ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH] coccinelle: api: add device_attr_show script
On 6/17/20 11:27 PM, Julia Lawall wrote: > > > On Mon, 15 Jun 2020, Denis Efremov wrote: > >> According to the documentation[1] show() methods of device attributes >> should return the number of bytes printed into the buffer. This is >> the return value of scnprintf(). show() must not use snprintf() >> when formatting the value to be returned to user space. snprintf() >> returns the length the resulting string would be, assuming it all >> fit into the destination array[2]. scnprintf() return the length of >> the string actually created in buf. If one can guarantee that an >> overflow will never happen sprintf() can be used otherwise scnprintf(). > > The semantic patch looks fine. Do you have any accepted patches from > this? It's not my patches, but: 3f9f8daad342 cpuidle: sysfs: Fix the overlap for showing available governors 117e2cb3 sparc: use scnprintf() in show_pciobppath_attr() in vio.c 03a1b56f501e sparc: use scnprintf() in show_pciobppath_attr() in pci.c 3dee04262898 iio: tsl2772: Use scnprintf() for avoiding potential buffer overflow dbdd24eaac4e edd: Use scnprintf() for avoiding potential buffer overflow abdd9feb45ed btrfs: sysfs: Use scnprintf() instead of snprintf() f21431f2de33 thermal: int340x_thermal: Use scnprintf() for avoiding potential buffer overflow 40501c70e3f0 s390/zcrypt: replace snprintf/sprintf with scnprintf eb3e064b8dd1 s390/zcrypt: Use scnprintf() for avoiding potential buffer overflow 06b522d6de9d video: uvesafb: Use scnprintf() for avoiding potential buffer overflow bf1b615ad97e video: omapfb: Use scnprintf() for avoiding potential buffer overflow b40e288bfb53 platform/x86: sony-laptop: Use scnprintf() for avoiding potential buffer overflow ef21e1750158 ALSA: Use scnprintf() instead of snprintf() for show and many more Thanks, Denis ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH] coccinelle: misc: add array_size_dup script to detect missed overlow checks
> > Awesome! I'll take a look into this. :) > Here is another script for your #83 ticket. Currently, it issues 598 warnings. // SPDX-License-Identifier: GPL-2.0-only /// /// Check for missing overflow checks in allocation functions. /// Low confidence because it's pointless to check for overflow /// relatively small allocations. /// // Confidence: Low // Copyright: (C) 2020 Denis Efremov ISPRAS // Options: --no-includes --include-headers virtual patch virtual context virtual org virtual report @depends on patch@ expression E1, E2, E3, E4, size; @@ ( - size = E1 * E2; + size = array_size(E1, E2); | - size = E1 * E2 * E3; + size = array3_size(E1, E2, E3); | - size = E1 * E2 + E3; + size = struct_size(E1, E2, E3); ) ... when != size = E4 when != size += E4 when != size -= E4 when != size *= E4 when != \(kmalloc\|krealloc\|kzalloc\|kzalloc_node\| vmalloc\|vzalloc\|vzalloc_node\| kvmalloc\|kvzalloc\|kvzalloc_node\| sock_kmalloc\| f2fs_kmalloc\|f2fs_kzalloc\|f2fs_kvmalloc\|f2fs_kvzalloc\| devm_kmalloc\|devm_kzalloc\) (..., size, ...) @r depends on !patch@ expression E1, E2, E3, E4, size; position p; @@ ( * size = E1 * E2;@p | * size = E1 * E2 * E3;@p | * size = E1 * E2 + E3;@p ) ... when != size = E4 when != size += E4 when != size -= E4 when != size *= E4 when != * \(kmalloc\|krealloc\|kzalloc\|kzalloc_node\| vmalloc\|vzalloc\|vzalloc_node\| kvmalloc\|kvzalloc\|kvzalloc_node\| sock_kmalloc\| f2fs_kmalloc\|f2fs_kzalloc\|f2fs_kvmalloc\|f2fs_kvzalloc\| devm_kmalloc\|devm_kzalloc\) (..., size, ...) @script:python depends on report@ p << r.p; @@ coccilib.report.print_report(p[0], "WARNING: missing overflow check") @script:python depends on org@ p << r.p; @@ coccilib.org.print_todo(p[0], "WARNING: missing overflow check") ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH] coccinelle: misc: add array_size_dup script to detect missed overlow checks
> > > Awesome! I'll take a look into this. :) > It would be helpful to get a feedback from you after. What kind of warnings are helpful and what are not? "duplicate calls" and "opencoded expression after array_size()" look doubtful to me. I think that maintainers will not like these patches. Thanks, Denis ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH] coccinelle: misc: add array_size_dup script to detect missed overlow checks
On 6/15/20 9:23 PM, Kees Cook wrote: > On Mon, Jun 15, 2020 at 01:20:45PM +0300, Denis Efremov wrote: >> Detect an opencoded expression that is used before or after >> array_size()/array3_size()/struct_size() to compute the same size. >> >> Cc: Kees Cook >> Signed-off-by: Denis Efremov > > Oh, very cool! How much does this find currently? > opencoded expression before the function call: ./drivers/net/ethernet/cavium/liquidio/request_manager.c:98:34-59: WARNING: array_size is used down the code (line 103) to compute the same size ./drivers/media/test-drivers/vivid/vivid-core.c:1120:26-34: WARNING: array_size is used down the code (line 1122) to compute the same size ./drivers/scsi/megaraid/megaraid_sas_fusion.c:5184:11-31: WARNING: array_size is used down the code (line 5191) to compute the same size ./drivers/scsi/megaraid/megaraid_sas_fusion.c:5200:2-37: WARNING: array_size is used down the code (line 5207) to compute the same size ./fs/cifs/misc.c:853:17-39: WARNING: array_size is used down the code (line 858) to compute the same size ./fs/cifs/misc.c:863:17-38: WARNING: array_size is used down the code (line 868) to compute the same size ./drivers/scsi/fnic/fnic_trace.c:562:25-48: WARNING: array_size is used down the code (line 566) to compute the same size opencoded expression after the function call: ./net/ethtool/ioctl.c:1976:55-66: WARNING: array_size is already used (line 1957) to compute the same size ./net/ethtool/ioctl.c:1921:55-66: WARNING: array_size is already used (line 1909) to compute the same size ./drivers/net/ethernet/cavium/liquidio/request_manager.c:111:29-54: WARNING: array_size is already used (line 103) to compute the same size ./drivers/staging/rts5208/ms.c:2309:55-56: WARNING: array_size is already used (line 2305) to compute the same size ./drivers/video/fbdev/core/fbcon.c:642:52-53: WARNING: array3_size is already used (line 638) to compute the same size ./drivers/video/fbdev/core/fbcon.c:679:47-48: WARNING: array3_size is already used (line 638) to compute the same size ./drivers/usb/misc/sisusbvga/sisusb_con.c:1229:54-56: WARNING: array_size is already used (line 1226) to compute the same size ./fs/afs/cmservice.c:271:45-46: WARNING: array3_size is already used (line 267) to compute the same size ./drivers/mtd/ftl.c:270:49-65: WARNING: array_size is already used (line 266) to compute the same size ./drivers/scsi/qla2xxx/tcm_qla2xxx.c:1608:6-42: WARNING: array_size is already used (line 1605) to compute the same size ./drivers/scsi/qla2xxx/tcm_qla2xxx.c:1613:8-44: WARNING: array_size is already used (line 1605) to compute the same size ./drivers/net/ppp/bsd_comp.c:439:13-37: WARNING: array_size is already used (line 409) to compute the same size ./drivers/net/wireless/ath/ath5k/debug.c:957:20-21: WARNING: array_size is already used (line 934) to compute the same size ./drivers/scsi/fnic/fnic_trace.c:575:3-26: WARNING: array_size is already used (line 566) to compute the same size ./drivers/scsi/fnic/fnic_trace.c:592:32-53: WARNING: array_size is already used (line 580) to compute the same size ./drivers/scsi/fnic/fnic_trace.c:504:30-51: WARNING: array_size is already used (line 492) to compute the same size ./drivers/staging/rts5208/rtsx_chip.c:1475:17-18: WARNING: array_size is already used (line 1458) to compute the same size ./kernel/kexec_file.c:917:8-25: WARNING: array_size is already used (line 913) to compute the same size ./drivers/rapidio/devices/rio_mport_cdev.c:984:8-25: WARNING: array_size is already used (line 978) to compute the same size ./fs/reiserfs/bitmap.c:1463:22-37: WARNING: array_size is already used (line 1459) to compute the same size duplicate calls: ./drivers/media/test-drivers/vivid/vivid-core.c:1125:59-60: WARNING: same array_size (line 1122) ./drivers/media/common/v4l2-tpg/v4l2-tpg-core.c:142:36-37: WARNING: same array_size (line 138) ./drivers/media/common/v4l2-tpg/v4l2-tpg-core.c:129:41-42: WARNING: same array3_size (line 123) ./drivers/media/common/v4l2-tpg/v4l2-tpg-core.c:146:40-41: WARNING: same array3_size (line 123) ./drivers/media/common/v4l2-tpg/v4l2-tpg-core.c:146:40-41: WARNING: same array3_size (line 129) ./drivers/net/ethernet/cavium/liquidio/octeon_droq.c:289:27-28: WARNING: same array_size (line 284) ./drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c:857:59-60: WARNING: same struct_size (line 854) ./fs/f2fs/super.c:3478:34-35: WARNING: same array_size (line 3478) ./drivers/net/wireless/zydas/zd1211rw/zd_usb.c:1637:45-46: WARNING: same struct_size (line 1634) ./drivers/net/ethernet/netronome/nfp/flower/cmsg.c:221:49-50: WARNING: same struct_size (line 219) ./drivers/staging/rts5208/rtsx_chip.c:1458:36-37: WARNING: same array_size (line 1454) ./drivers/net/ethernet/neterion/vxge/vxge-config.c:2664:59-60: WARNING: same array_size (line 2654) ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH] coccinelle: api: add kzfree script
On 6/15/20 3:03 PM, Dan Carpenter wrote: > On Sun, Jun 14, 2020 at 10:42:54PM +0300, Denis Efremov wrote: >> On 6/4/20 7:27 PM, Joe Perches wrote: >>> On Thu, 2020-06-04 at 17:08 +0300, Denis Efremov wrote: >>>> Check for memset() with 0 followed by kfree(). >>> >>> Perhaps those uses should be memzero_explicit or kvfree_sensitive. >>> >> >> Is it safe to suggest to use kzfree instead of memzero_explicit && kfree? >> Or it would be better to use kvfree_sensitive in this case. >> >> kzfree uses memset(0) with no barrier_data. > > Yeah. That seems buggy. It should have a barrier. Also I thought I > saw somewhere that Linus doesn't like the name and so that's why we have > the _sensitive() name? > Oh, there are already patches for renaming kzfree to kfree_sensitive. https://lkml.org/lkml/2020/4/13/729 It seems they are not accepted despite multiple acks, through. Thanks, Denis ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH] coccinelle: api: add device_attr_show script
According to the documentation[1] show() methods of device attributes
should return the number of bytes printed into the buffer. This is
the return value of scnprintf(). show() must not use snprintf()
when formatting the value to be returned to user space. snprintf()
returns the length the resulting string would be, assuming it all
fit into the destination array[2]. scnprintf() return the length of
the string actually created in buf. If one can guarantee that an
overflow will never happen sprintf() can be used otherwise scnprintf().
[1] Documentation/filesystems/sysfs.txt
[2] "snprintf() confusion" https://lwn.net/Articles/69419/
Signed-off-by: Denis Efremov
---
scripts/coccinelle/api/device_attr_show.cocci | 55 +++
1 file changed, 55 insertions(+)
create mode 100644 scripts/coccinelle/api/device_attr_show.cocci
diff --git a/scripts/coccinelle/api/device_attr_show.cocci
b/scripts/coccinelle/api/device_attr_show.cocci
new file mode 100644
index ..d8ec4bb8ac41
--- /dev/null
+++ b/scripts/coccinelle/api/device_attr_show.cocci
@@ -0,0 +1,55 @@
+// SPDX-License-Identifier: GPL-2.0-only
+///
+/// From Documentation/filesystems/sysfs.txt:
+/// show() must not use snprintf() when formatting the value to be
+/// returned to user space. If you can guarantee that an overflow
+/// will never happen you can use sprintf() otherwise you must use
+/// scnprintf().
+///
+// Confidence: High
+// Copyright: (C) 2020 Denis Efremov ISPRAS
+// Options: --no-includes --include-headers
+//
+
+virtual report
+virtual org
+virtual context
+virtual patch
+
+@r depends on !patch@
+identifier show, dev, attr, buf;
+position p;
+@@
+
+ssize_t show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ <...
+* return snprintf@p(...);
+ ...>
+}
+
+@rp depends on patch@
+identifier show, dev, attr, buf;
+@@
+
+ssize_t show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ <...
+ return
+- snprintf
++ scnprintf
+ (...);
+ ...>
+}
+
+@script: python depends on report@
+p << r.p;
+@@
+
+coccilib.report.print_report(p[0], "WARNING: use scnprintf or sprintf")
+
+@script: python depends on org@
+p << r.p;
+@@
+
+coccilib.org.print_todo(p[0], "WARNING: use scnprintf or sprintf")
--
2.26.2
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH] coccinelle: misc: add array_size_dup script to detect missed overlow checks
Detect an opencoded expression that is used before or after array_size()/array3_size()/struct_size() to compute the same size. Cc: Kees Cook Signed-off-by: Denis Efremov --- scripts/coccinelle/misc/array_size_dup.cocci | 347 +++ 1 file changed, 347 insertions(+) create mode 100644 scripts/coccinelle/misc/array_size_dup.cocci diff --git a/scripts/coccinelle/misc/array_size_dup.cocci b/scripts/coccinelle/misc/array_size_dup.cocci new file mode 100644 index ..08919a938754 --- /dev/null +++ b/scripts/coccinelle/misc/array_size_dup.cocci @@ -0,0 +1,347 @@ +// SPDX-License-Identifier: GPL-2.0-only +/// +/// Check for array_size(), array3_size(), struct_size() duplicates. +/// Three types of patterns for these functions: +/// 1. An opencoded expression is used before array_size() to compute the same size +/// 2. An opencoded expression is used after array_size() to compute the same size +/// 3. Consecutive calls of array_size() with the same values +/// From security point of view only first case is relevant. These functions +/// perform arithmetic overflow check. Thus, if we use an opencoded expression +/// before a call to the *_size() function we can miss an overflow. +/// +// Confidence: High +// Copyright: (C) 2020 Denis Efremov ISPRAS +// Options: --no-includes --include-headers --no-loops + +virtual context +virtual report +virtual org + +@as@ +expression E1, E2; +@@ + +array_size(E1, E2) + +@as_next@ +expression subE1 <= as.E1; +expression as.E1; +expression subE2 <= as.E2; +expression as.E2; +expression E3; +position p1, p2; +@@ + +* E1 * E2@p1 + ... when != \(E1\|E2\|subE1\|subE2\)=E3 + when != \(E1\|E2\|subE1\|subE2\)+=E3 + when != \(E1\|E2\|subE1\|subE2\)-=E3 + when != \(E1\|E2\|subE1\|subE2\)*=E3 + when != \(\|\|\|\) +* array_size(E1, E2)@p2 + +@as_prev@ +expression subE1 <= as.E1; +expression as.E1; +expression subE2 <= as.E2; +expression as.E2; +expression E3; +position p1, p2; +@@ + +* array_size(E1, E2)@p1 + ... when != \(E1\|E2\|subE1\|subE2\)=E3 + when != \(E1\|E2\|subE1\|subE2\)+=E3 + when != \(E1\|E2\|subE1\|subE2\)-=E3 + when != \(E1\|E2\|subE1\|subE2\)*=E3 + when != \(\|\|\|\) +* E1 * E2@p2 + +@as_dup@ +expression subE1 <= as.E1; +expression as.E1; +expression subE2 <= as.E2; +expression as.E2; +expression E3; +position p1, p2; +@@ + +* array_size(E1, E2)@p1 + ... when != \(E1\|E2\|subE1\|subE2\)=E3 + when != \(E1\|E2\|subE1\|subE2\)+=E3 + when != \(E1\|E2\|subE1\|subE2\)-=E3 + when != \(E1\|E2\|subE1\|subE2\)*=E3 + when != \(\|\|\|\) +* array_size(E1, E2)@p2 + +@as3@ +expression E1, E2, E3; +@@ + +array3_size(E1, E2, E3) + +@as3_next@ +expression subE1 <= as3.E1; +expression as3.E1; +expression subE2 <= as3.E2; +expression as3.E2; +expression subE3 <= as3.E3; +expression as3.E3; +expression E4; +position p1, p2; +@@ + +* E1 * E2 * E3@p1 + ... when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)=E4 + when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)+=E4 + when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)-=E4 + when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)*=E4 + when != \(\|\|\|\|\|\) +* array3_size(E1, E2, E3)@p2 + +@as3_prev@ +expression subE1 <= as3.E1; +expression as3.E1; +expression subE2 <= as3.E2; +expression as3.E2; +expression subE3 <= as3.E3; +expression as3.E3; +expression E4; +position p1, p2; +@@ + +* array3_size(E1, E2, E3)@p1 + ... when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)=E4 + when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)+=E4 + when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)-=E4 + when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)*=E4 + when != \(\|\|\|\|\|\) +* E1 * E2 * E3@p2 + +@as3_dup@ +expression subE1 <= as3.E1; +expression as3.E1; +expression subE2 <= as3.E2; +expression as3.E2; +expression subE3 <= as3.E3; +expression as3.E3; +expression E4; +position p1, p2; +@@ + +* array3_size(E1, E2, E3)@p1 + ... when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)=E4 + when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)+=E4 + when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)-=E4 + when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)*=E4 + when != \(\|\|\|\|\|\) +* array3_size(E1, E2, E3)@p2 + +@ss@ +expression E1, E2, E3; +@@ + +struct_size(E1, E2, E3) + +@ss_next@ +expression subE1 <= ss.E1; +expression ss.E1; +expression subE2 <= ss.E2; +expression ss.E2; +expression subE3 <= ss.E3; +expression ss.E3; +expression E4; +position p1, p2; +@@ + +* E1 * E2 + E3@p1 + ... when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)=E4 + when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)+=E4 + when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)-=E4 + when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)*=E4 + when != \(\|\|\|\|\|\) +* struct_size(E1, E2, E3)@p2 + +@ss_prev@ +expression subE1 <= ss.E1; +expression ss.E1; +expression subE2 <= ss.E2; +expression ss.E2; +expression subE3 <= ss.E3; +expression ss.E3; +expression E4; +position p1,
[Cocci] [PATCH v3] coccinelle: api: add kzfree script
Check for memset()/memzero_explicit() followed by kfree()/vfree()/kvfree().
Signed-off-by: Denis Efremov
---
Changes in v2:
- memset_explicit() added
- kvfree_sensitive() added
- forall added to r1
- ... between memset and kfree added
Changes in v3:
- Explicit filter for definitions instead of !(file in "...") conditions
- type T added to match casts
- memzero_explicit() patterns fixed
- additional rule "cond" added to filter false-positives
scripts/coccinelle/api/kzfree.cocci | 90 +
1 file changed, 90 insertions(+)
create mode 100644 scripts/coccinelle/api/kzfree.cocci
diff --git a/scripts/coccinelle/api/kzfree.cocci
b/scripts/coccinelle/api/kzfree.cocci
new file mode 100644
index ..4758ca5a781e
--- /dev/null
+++ b/scripts/coccinelle/api/kzfree.cocci
@@ -0,0 +1,90 @@
+// SPDX-License-Identifier: GPL-2.0-only
+///
+/// Use kzfree, kvfree_sensitive rather than memset or
+/// memzero_explicit followed by kfree
+///
+// Confidence: High
+// Copyright: (C) 2020 Denis Efremov ISPRAS
+// Options: --no-includes --include-headers
+//
+// Keywords: kzfree, kvfree_sensitive
+//
+
+virtual context
+virtual patch
+virtual org
+virtual report
+
+@initialize:python@
+@@
+# kmalloc_oob_in_memset uses memset to explicitly trigger out-of-bounds access
+filter = frozenset(['kmalloc_oob_in_memset', 'kzfree', 'kvfree_sensitive'])
+
+def relevant(p):
+return not (filter & {el.current_element for el in p})
+
+@cond@
+position ok;
+@@
+
+if (...)
+ \(memset@ok\|memzero_explicit@ok\)(...);
+
+@r depends on !patch forall@
+expression E;
+position p : script:python() { relevant(p) };
+position m != cond.ok;
+type T;
+@@
+
+(
+* memset@m((T)E, 0, ...);
+|
+* memzero_explicit@m((T)E, ...);
+)
+ ... when != E
+ when strict
+* \(kfree\|vfree\|kvfree\)(E)@p;
+
+@rp_memzero depends on patch@
+expression E, size;
+position p : script:python() { relevant(p) };
+type T;
+@@
+
+- memzero_explicit((T)E, size)@p;
+ ... when != E
+ when strict
+- \(kfree\|vfree\|kvfree\)(E);
++ kvfree_sensitive(E, size);
+
+@rp_memset depends on patch@
+expression E, size;
+position p : script:python() { relevant(p) };
+type T;
+@@
+
+- memset((T)E, size)@p;
+ ... when != E
+ when strict
+(
+- kfree(E);
++ kzfree(E);
+|
+- \(vfree\|kvfree\)(E);
++ kvfree_sensitive(E, size);
+)
+
+@script:python depends on report@
+p << r.p;
+@@
+
+coccilib.report.print_report(p[0],
+ "WARNING: opportunity for kzfree/kvfree_sensitive")
+
+@script:python depends on org@
+p << r.p;
+@@
+
+coccilib.org.print_todo(p[0],
+ "WARNING: opportunity for kzfree/kvfree_sensitive")
--
2.26.2
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
