From 43d96390d57aa0e61d15ce2bcb887df8516d58f5 Mon Sep 17 00:00:00 2001 From: Julia Lawall Date: Wed, 21 Sep 2016 17:48:39 +0200 Subject: [PATCH 1/4] Coccinelle: limit memdup_user transformation to GFP_KERNEL case Memdup_user encapsulates a memory allocation with the flag GFP_KERNEL, so only allow this flag in the original code. Signed-off-by: Julia Lawall Signed-off-by: Michal Marek --- scripts/coccinelle/api/memdup_user.cocci | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/scripts/coccinelle/api/memdup_user.cocci b/scripts/coccinelle/api/memdup_user.cocci index c606231b0e46..2a5aea8e8487 100644 --- a/scripts/coccinelle/api/memdup_user.cocci +++ b/scripts/coccinelle/api/memdup_user.cocci @@ -15,11 +15,11 @@ virtual org virtual report @depends on patch@ -expression from,to,size,flag; +expression from,to,size; identifier l1,l2; @@ -- to = \(kmalloc\|kzalloc\)(size,flag); +- to = \(kmalloc\|kzalloc\)(size,GFP_KERNEL); + to = memdup_user(from,size); if ( - to==NULL @@ -37,12 +37,12 @@ identifier l1,l2; - } @r depends on !patch@ -expression from,to,size,flag; +expression from,to,size; position p; statement S1,S2; @@ -* to = \(kmalloc@p\|kzalloc@p\)(size,flag); +* to = \(kmalloc@p\|kzalloc@p\)(size,GFP_KERNEL); if (to==NULL || ...) S1 if (copy_from_user(to, from, size) != 0) S2 From d97629f1686574a800a76eb0d2ce65e3f3d3ef92 Mon Sep 17 00:00:00 2001 From: Julia Lawall Date: Thu, 22 Sep 2016 10:28:55 +0200 Subject: [PATCH 2/4] Coccinelle: pm_runtime: ensure relevance of pm_runtime reports pm_runtime.cocci starts with one rule that searches for a variety of functions calls, followed by various rules that report errors. Previously, the only connection between the first rule and the rest was to check that the first rule had matched somewhere. Change the rules to propagate a position from the first rule to the others, to make sure that the sites reported on are the same as the sites that were identified as having the relevant functions. Signed-off-by: Julia Lawall Signed-off-by: Michal Marek --- scripts/coccinelle/api/pm_runtime.cocci | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/scripts/coccinelle/api/pm_runtime.cocci b/scripts/coccinelle/api/pm_runtime.cocci index 89b98a2f7a6f..d67ccf5f8227 100644 --- a/scripts/coccinelle/api/pm_runtime.cocci +++ b/scripts/coccinelle/api/pm_runtime.cocci @@ -17,9 +17,10 @@ virtual report @runtime_bad_err_handle exists@ expression ret; +position p; @@ ( -ret = \(pm_runtime_idle\| +ret@p = \(pm_runtime_idle\| pm_runtime_suspend\| pm_runtime_autosuspend\| pm_runtime_resume\| @@ -47,12 +48,13 @@ IS_ERR_VALUE(ret) // For context mode //---------------------------------------------------------- -@depends on runtime_bad_err_handle && context@ +@depends on context@ identifier pm_runtime_api; expression ret; +position runtime_bad_err_handle.p; @@ ( -ret = pm_runtime_api(...); +ret@p = pm_runtime_api(...); ... * IS_ERR_VALUE(ret) ... @@ -62,12 +64,13 @@ ret = pm_runtime_api(...); // For patch mode //---------------------------------------------------------- -@depends on runtime_bad_err_handle && patch@ +@depends on patch@ identifier pm_runtime_api; expression ret; +position runtime_bad_err_handle.p; @@ ( -ret = pm_runtime_api(...); +ret@p = pm_runtime_api(...); ... - IS_ERR_VALUE(ret) + ret < 0 @@ -78,13 +81,14 @@ ret = pm_runtime_api(...); // For org and report mode //---------------------------------------------------------- -@r depends on runtime_bad_err_handle && (org || report) exists@ +@r depends on (org || report) exists@ position p1, p2; identifier pm_runtime_api; expression ret; +position runtime_bad_err_handle.p; @@ ( -ret = pm_runtime_api@p1(...); +ret@p = pm_runtime_api@p1(...); ... IS_ERR_VALUE@p2(ret) ... From 1e01892e7ad521a96760da0d791b42badf755d3f Mon Sep 17 00:00:00 2001 From: Markus Elfring Date: Fri, 7 Oct 2016 16:06:15 +0200 Subject: [PATCH 3/4] scripts/coccicheck: Update reference for the corresponding documentation Use the current name (in a comment at the beginning of this script) for the file which was converted to the documentation format "reStructuredText" in August 2016. Fixes: 4b9033a33494 ("docs: sphinxify coccinelle.txt and add it to dev-tools") Signed-off-by: Markus Elfring Acked-by: Julia Lawall Signed-off-by: Michal Marek --- scripts/coccicheck | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/coccicheck b/scripts/coccicheck index c92c1528a54d..ec487b8e7051 100755 --- a/scripts/coccicheck +++ b/scripts/coccicheck @@ -1,7 +1,7 @@ #!/bin/bash # Linux kernel coccicheck # -# Read Documentation/coccinelle.txt +# Read Documentation/dev-tools/coccinelle.rst # # This script requires at least spatch # version 1.0.0-rc11. From c8990359d4b12f14656386526ddf904635076902 Mon Sep 17 00:00:00 2001 From: Nicholas Mc Guire Date: Sat, 8 Oct 2016 17:51:45 +0200 Subject: [PATCH 4/4] Coccinelle: flag conditions with no effect Report code constructs where the if and else branch are functionally identical. In cases where this is intended it really should be documented - most reported cases probably are bugs. Signed-off-by: Nicholas Mc Guire Signed-off-by: Michal Marek --- scripts/coccinelle/misc/cond_no_effect.cocci | 64 ++++++++++++++++++++ 1 file changed, 64 insertions(+) create mode 100644 scripts/coccinelle/misc/cond_no_effect.cocci diff --git a/scripts/coccinelle/misc/cond_no_effect.cocci b/scripts/coccinelle/misc/cond_no_effect.cocci new file mode 100644 index 000000000000..8467dbd1c465 --- /dev/null +++ b/scripts/coccinelle/misc/cond_no_effect.cocci @@ -0,0 +1,64 @@ +///Find conditions where if and else branch are functionally +// identical. +// +// There can be false positives in cases where the positional +// information is used (as with lockdep) or where the identity +// is a placeholder for not yet handled cases. +// Unfortunately there also seems to be a tendency to use +// the last if else/else as a "default behavior" - which some +// might consider a legitimate coding pattern. From discussion +// on kernelnewbies though it seems that this is not really an +// accepted pattern and if at all it would need to be commented +// +// In the Linux kernel it does not seem to actually report +// false positives except for those that were documented as +// being intentional. +// the two known cases are: +// arch/sh/kernel/traps_64.c:read_opcode() +// } else if ((pc & 1) == 0) { +// /* SHcompact */ +// /* TODO : provide handling for this. We don't really support +// user-mode SHcompact yet, and for a kernel fault, this would +// have to come from a module built for SHcompact. */ +// return -EFAULT; +// } else { +// /* misaligned */ +// return -EFAULT; +// } +// fs/kernfs/file.c:kernfs_fop_open() +// * Both paths of the branch look the same. They're supposed to +// * look that way and give @of->mutex different static lockdep keys. +// */ +// if (has_mmap) +// mutex_init(&of->mutex); +// else +// mutex_init(&of->mutex); +// +// All other cases look like bugs or at least lack of documentation +// +// Confidence: Moderate +// Copyright: (C) 2016 Nicholas Mc Guire, OSADL. GPLv2. +// Comments: +// Options: --no-includes --include-headers + +virtual org +virtual report + +@cond@ +statement S1; +position p; +@@ + +* if@p (...) S1 else S1 + +@script:python depends on org@ +p << cond.p; +@@ + +cocci.print_main("WARNING: possible condition with no effect (if == else)",p) + +@script:python depends on report@ +p << cond.p; +@@ + +coccilib.report.print_report(p[0],"WARNING: possible condition with no effect (if == else)")