[Cocci] [PATCH v4] provide rule for finding refcounters

Discussion:

Elena Reshetova

2017-08-30 06:15:57 UTC

changes in v4:
Removed unnessesary "()" around expression in rule 4.

changes in v3:
Removed unnessesary rule 4 conditions pointed by Julia.

changes in v2:
Following the suggestion from Julia the first rule is split into
2. The output does not differ that much between these two versions,
but rule became more precise.

Elena Reshetova (1):
Coccinelle: add atomic_as_refcounter script

scripts/coccinelle/api/atomic_as_refcounter.cocci | 131 ++++++++++++++++++++++
1 file changed, 131 insertions(+)
create mode 100644 scripts/coccinelle/api/atomic_as_refcounter.cocci

--
2.7.4

Elena Reshetova

2017-08-30 06:15:58 UTC

Permalink

atomic_as_refcounter.cocci script allows detecting
cases when refcount_t type and API should be used
instead of atomic_t.

Signed-off-by: Elena Reshetova <***@intel.com>
---
scripts/coccinelle/api/atomic_as_refcounter.cocci | 131 ++++++++++++++++++++++
1 file changed, 131 insertions(+)
create mode 100644 scripts/coccinelle/api/atomic_as_refcounter.cocci

diff --git a/scripts/coccinelle/api/atomic_as_refcounter.cocci b/scripts/coccinelle/api/atomic_as_refcounter.cocci
new file mode 100644
index 0000000..f83771b
--- /dev/null
+++ b/scripts/coccinelle/api/atomic_as_refcounter.cocci
@@ -0,0 +1,131 @@
+// Check if refcount_t type and API should be used
+// instead of atomic_t type when dealing with refcounters
+//
+// Copyright (c) 2016-2017, Elena Reshetova, Intel Corporation
+//
+// Confidence: Moderate
+// URL: http://coccinelle.lip6.fr/
+// Options: --include-headers --very-quiet
+
+virtual report
+
+@r1 exists@
+identifier a, x, y;
+position p1, p2;
+identifier fname =~ ".*free.*";
+identifier fname2 =~ ".*destroy.*";
+identifier fname3 =~ ".*del.*";
+identifier fname4 =~ ".*queue_work.*";
+identifier fname5 =~ ".*schedule_work.*";
+identifier fname6 =~ ".*call_rcu.*";
+
+@@
+
+(
+ ***@p1(&(a)->x)
+|
+ ***@p1(&(a)->x, ...)
+|
+ ***@p1(&(a)->x, ...)
+|
+ ***@p1(&(a)->x)
+|
+ ***@p1(&(a)->x)
+|
+ ***@p1(&(a)->x)
+)
+...
+(
+ ***@p2(a, ...);
+|
+ ***@p2(...);
+|
+ ***@p2(...);
+|
+ ***@p2(...);
+|
+ ***@p2(...);
+|
+ ***@p2(...);
+)
+
+
+@script:python depends on report@
+p1 << r1.p1;
+p2 << r1.p2;
+@@
+msg = "atomic_dec_and_test variation before object free at line %s."
+coccilib.report.print_report(p1[0], msg % (p2[0].line))
+
+@r4 exists@
+identifier a, x, y;
+position p1, p2;
+identifier fname =~ ".*free.*";
+
+@@
+
+(
+ ***@p1(&(a)->x)
+|
+ ***@p1(&(a)->x, ...)
+|
+ ***@p1(&(a)->x, ...)
+|
+ ***@p1(&(a)->x)
+|
+ ***@p1(&(a)->x)
+|
+ ***@p1(&(a)->x)
+)
+...
+y=a
+...
+***@p2(y, ...);
+
+
+@script:python depends on report@
+p1 << r4.p1;
+p2 << r4.p2;
+@@
+msg = "atomic_dec_and_test variation before object free at line %s."
+coccilib.report.print_report(p1[0], msg % (p2[0].line))
+
+@r2 exists@
+identifier a, x;
+position p1;
+@@
+
+(
+atomic_add_unless(&(a)->x,-1,1)@p1
+|
+atomic_long_add_unless(&(a)->x,-1,1)@p1
+|
+atomic64_add_unless(&(a)->x,-1,1)@p1
+)
+
+@script:python depends on report@
+p1 << r2.p1;
+@@
+msg = "atomic_add_unless"
+coccilib.report.print_report(p1[0], msg)
+
+@r3 exists@
+identifier x;
+position p1;
+@@
+
+(
+x = ***@p1(-1, ...);
+|
+x = ***@p1(-1, ...);
+|
+x = ***@p1(-1, ...);
+)
+
+@script:python depends on report@
+p1 << r3.p1;
+@@
+msg = "x = atomic_add_return(-1, ...)"
+coccilib.report.print_report(p1[0], msg)
+
+

--
2.7.4

SF Markus Elfring

2017-08-30 10:36:48 UTC

Permalink

Dear Elena,

I hoped on a more constructive feedback for remaining implementation details
which should be reconsidered once more.
https://systeme.lip6.fr/pipermail/cocci/2017-August/004341.html

* Will any more revisions be needed for your evolving script until special
development concerns will be taken into account appropriately?

* How do you prefer to clarify unsafe search patterns finally?

Regards,
Markus

Julia Lawall

2017-08-30 12:26:26 UTC

Permalink

Post by Elena Reshetova
atomic_as_refcounter.cocci script allows detecting
cases when refcount_t type and API should be used
instead of atomic_t.
---
scripts/coccinelle/api/atomic_as_refcounter.cocci | 131 ++++++++++++++++++++++
1 file changed, 131 insertions(+)
create mode 100644 scripts/coccinelle/api/atomic_as_refcounter.cocci
diff --git a/scripts/coccinelle/api/atomic_as_refcounter.cocci b/scripts/coccinelle/api/atomic_as_refcounter.cocci
new file mode 100644
index 0000000..f83771b
--- /dev/null
+++ b/scripts/coccinelle/api/atomic_as_refcounter.cocci
@@ -0,0 +1,131 @@
+// Check if refcount_t type and API should be used
+// instead of atomic_t type when dealing with refcounters
+//
+// Copyright (c) 2016-2017, Elena Reshetova, Intel Corporation
+//
+// Confidence: Moderate
+// URL: http://coccinelle.lip6.fr/
+// Options: --include-headers --very-quiet
+
+virtual report
+
+identifier a, x, y;
+position p1, p2;
+identifier fname =~ ".*free.*";
+identifier fname2 =~ ".*destroy.*";
+identifier fname3 =~ ".*del.*";
+identifier fname4 =~ ".*queue_work.*";
+identifier fname5 =~ ".*schedule_work.*";
+identifier fname6 =~ ".*call_rcu.*";
+
+
+(
+|
+|
+|
+|
+|
+)
+...
+(
+|
+|
+|
+|
+|
+)
+
+
+p1 << r1.p1;
+p2 << r1.p2;
+msg = "atomic_dec_and_test variation before object free at line %s."
+coccilib.report.print_report(p1[0], msg % (p2[0].line))
+
+identifier a, x, y;
+position p1, p2;
+identifier fname =~ ".*free.*";
+
+
+(
+|
+|
+|
+|
+|
+)
+...
+y=a
+...
+
+
+p1 << r4.p1;
+p2 << r4.p2;
+msg = "atomic_dec_and_test variation before object free at line %s."
+coccilib.report.print_report(p1[0], msg % (p2[0].line))
+
+identifier a, x;
+position p1;
+
+(
+|
+|
+)
+
+p1 << r2.p1;
+msg = "atomic_add_unless"
+coccilib.report.print_report(p1[0], msg)
+
+identifier x;
+position p1;
+
+(
+|
+|
+)
+
+p1 << r3.p1;
+msg = "x = atomic_add_return(-1, ...)"
+coccilib.report.print_report(p1[0], msg)
+
+
--
2.7.4

Reshetova, Elena

2017-08-30 12:44:59 UTC

Permalink

Post by Elena Reshetova

Post by Elena Reshetova
atomic_as_refcounter.cocci script allows detecting
cases when refcount_t type and API should be used
instead of atomic_t.

Thank you very much Julia! What is the correct path to merge this?
I will resend with your acked-by, but what is the tree that should merge it?

Best Regards,
Elena.

Post by Elena Reshetova

Post by Elena Reshetova
---
scripts/coccinelle/api/atomic_as_refcounter.cocci | 131

++++++++++++++++++++++

Post by Elena Reshetova
1 file changed, 131 insertions(+)
create mode 100644 scripts/coccinelle/api/atomic_as_refcounter.cocci
diff --git a/scripts/coccinelle/api/atomic_as_refcounter.cocci

b/scripts/coccinelle/api/atomic_as_refcounter.cocci

Post by Elena Reshetova
new file mode 100644
index 0000000..f83771b
--- /dev/null
+++ b/scripts/coccinelle/api/atomic_as_refcounter.cocci
@@ -0,0 +1,131 @@
+// Check if refcount_t type and API should be used
+// instead of atomic_t type when dealing with refcounters
+//
+// Copyright (c) 2016-2017, Elena Reshetova, Intel Corporation
+//
+// Confidence: Moderate
+// URL: http://coccinelle.lip6.fr/
+// Options: --include-headers --very-quiet
+
+virtual report
+
+identifier a, x, y;
+position p1, p2;
+identifier fname =~ ".*free.*";
+identifier fname2 =~ ".*destroy.*";
+identifier fname3 =~ ".*del.*";
+identifier fname4 =~ ".*queue_work.*";
+identifier fname5 =~ ".*schedule_work.*";
+identifier fname6 =~ ".*call_rcu.*";
+
+
+(
+|
+|
+|
+|
+|
+)
+...
+(
+|
+|
+|
+|
+|
+)
+
+
+p1 << r1.p1;
+p2 << r1.p2;
+msg = "atomic_dec_and_test variation before object free at line %s."
+coccilib.report.print_report(p1[0], msg % (p2[0].line))
+
+identifier a, x, y;
+position p1, p2;
+identifier fname =~ ".*free.*";
+
+
+(
+|
+|
+|
+|
+|
+)
+...
+y=a
+...
+
+
+p1 << r4.p1;
+p2 << r4.p2;
+msg = "atomic_dec_and_test variation before object free at line %s."
+coccilib.report.print_report(p1[0], msg % (p2[0].line))
+
+identifier a, x;
+position p1;
+
+(
+|
+|
+)
+
+p1 << r2.p1;
+msg = "atomic_add_unless"
+coccilib.report.print_report(p1[0], msg)
+
+identifier x;
+position p1;
+
+(
+|
+|
+)
+
+p1 << r3.p1;
+msg = "x = atomic_add_return(-1, ...)"
+coccilib.report.print_report(p1[0], msg)
+
+
--
2.7.4

Julia Lawall

2017-08-30 13:06:22 UTC

Permalink

Post by Reshetova, Elena

Post by Elena Reshetova

Post by Elena Reshetova
atomic_as_refcounter.cocci script allows detecting
cases when refcount_t type and API should be used
instead of atomic_t.

Thank you very much Julia! What is the correct path to merge this?
I will resend with your acked-by, but what is the tree that should merge it?

Marek will take it. If you want to resend, please get rid of the
metavariable y in the rule r1. It causes an unused variable warning.

julia

Post by Reshetova, Elena
Best Regards,
Elena.

Post by Elena Reshetova

Post by Elena Reshetova
---
scripts/coccinelle/api/atomic_as_refcounter.cocci | 131

++++++++++++++++++++++

Post by Elena Reshetova
1 file changed, 131 insertions(+)
create mode 100644 scripts/coccinelle/api/atomic_as_refcounter.cocci
diff --git a/scripts/coccinelle/api/atomic_as_refcounter.cocci

b/scripts/coccinelle/api/atomic_as_refcounter.cocci

Reshetova, Elena

2017-08-31 09:46:32 UTC

Permalink

Post by Julia Lawall

Post by Reshetova, Elena

Post by Elena Reshetova

Post by Elena Reshetova
atomic_as_refcounter.cocci script allows detecting
cases when refcount_t type and API should be used
instead of atomic_t.

Thank you very much Julia! What is the correct path to merge this?
I will resend with your acked-by, but what is the tree that should merge it?

Marek will take it. If you want to resend, please get rid of the
metavariable y in the rule r1. It causes an unused variable warning.

Sure, will do. Thank you!

Best Regards,
Elena.

Post by Julia Lawall
julia

Post by Reshetova, Elena
Best Regards,
Elena.

Post by Elena Reshetova

Post by Elena Reshetova
---
scripts/coccinelle/api/atomic_as_refcounter.cocci | 131

++++++++++++++++++++++

Post by Elena Reshetova
1 file changed, 131 insertions(+)
create mode 100644 scripts/coccinelle/api/atomic_as_refcounter.cocci
diff --git a/scripts/coccinelle/api/atomic_as_refcounter.cocci

b/scripts/coccinelle/api/atomic_as_refcounter.cocci

SF Markus Elfring

2017-08-30 13:43:23 UTC

Permalink

Post by Reshetova, Elena

Acked-by: Julia Lawall <julia.lawall at lip6.fr>

Thank you very much Julia!

I find that this acknowledgement could be presented a bit too early.

Post by Reshetova, Elena
What is the correct path to merge this?

It seems that you prefer to ignore some of my technical arguments (or concerns?).

I hope that a safer source code search approach can still be achieved here.

Regards,
Markus

SF Markus Elfring

2017-08-30 10:21:15 UTC

Permalink

…

Do you find this cover letter still relevant for the clarification
of remaining open issues in a single SmPL script?

Regards,
Markus

Elena Reshetova

2017-09-01 09:40:20 UTC

Permalink

atomic_as_refcounter.cocci script allows detecting
cases when refcount_t type and API should be used
instead of atomic_t.

Signed-off-by: Elena Reshetova <***@intel.com>
Acked-by: Julia Lawall <***@lip6.fr>
---
scripts/coccinelle/api/atomic_as_refcounter.cocci | 131 ++++++++++++++++++++++
1 file changed, 131 insertions(+)
create mode 100644 scripts/coccinelle/api/atomic_as_refcounter.cocci

diff --git a/scripts/coccinelle/api/atomic_as_refcounter.cocci b/scripts/coccinelle/api/atomic_as_refcounter.cocci
new file mode 100644
index 0000000..bfa880d
--- /dev/null
+++ b/scripts/coccinelle/api/atomic_as_refcounter.cocci
@@ -0,0 +1,131 @@
+// Check if refcount_t type and API should be used
+// instead of atomic_t type when dealing with refcounters
+//
+// Copyright (c) 2016-2017, Elena Reshetova, Intel Corporation
+//
+// Confidence: Moderate
+// URL: http://coccinelle.lip6.fr/
+// Options: --include-headers --very-quiet
+
+virtual report
+
+@r1 exists@
+identifier a, x;
+position p1, p2;
+identifier fname =~ ".*free.*";
+identifier fname2 =~ ".*destroy.*";
+identifier fname3 =~ ".*del.*";
+identifier fname4 =~ ".*queue_work.*";
+identifier fname5 =~ ".*schedule_work.*";
+identifier fname6 =~ ".*call_rcu.*";
+
+@@
+
+(
+ ***@p1(&(a)->x)
+|
+ ***@p1(&(a)->x, ...)
+|
+ ***@p1(&(a)->x, ...)
+|
+ ***@p1(&(a)->x)
+|
+ ***@p1(&(a)->x)
+|
+ ***@p1(&(a)->x)
+)
+...
+(
+ ***@p2(a, ...);
+|
+ ***@p2(...);
+|
+ ***@p2(...);
+|
+ ***@p2(...);
+|
+ ***@p2(...);
+|
+ ***@p2(...);
+)
+
+
+@script:python depends on report@
+p1 << r1.p1;
+p2 << r1.p2;
+@@
+msg = "atomic_dec_and_test variation before object free at line %s."
+coccilib.report.print_report(p1[0], msg % (p2[0].line))
+
+@r4 exists@
+identifier a, x, y;
+position p1, p2;
+identifier fname =~ ".*free.*";
+
+@@
+
+(
+ ***@p1(&(a)->x)
+|
+ ***@p1(&(a)->x, ...)
+|
+ ***@p1(&(a)->x, ...)
+|
+ ***@p1(&(a)->x)
+|
+ ***@p1(&(a)->x)
+|
+ ***@p1(&(a)->x)
+)
+...
+y=a
+...
+***@p2(y, ...);
+
+
+@script:python depends on report@
+p1 << r4.p1;
+p2 << r4.p2;
+@@
+msg = "atomic_dec_and_test variation before object free at line %s."
+coccilib.report.print_report(p1[0], msg % (p2[0].line))
+
+@r2 exists@
+identifier a, x;
+position p1;
+@@
+
+(
+atomic_add_unless(&(a)->x,-1,1)@p1
+|
+atomic_long_add_unless(&(a)->x,-1,1)@p1
+|
+atomic64_add_unless(&(a)->x,-1,1)@p1
+)
+
+@script:python depends on report@
+p1 << r2.p1;
+@@
+msg = "atomic_add_unless"
+coccilib.report.print_report(p1[0], msg)
+
+@r3 exists@
+identifier x;
+position p1;
+@@
+
+(
+x = ***@p1(-1, ...);
+|
+x = ***@p1(-1, ...);
+|
+x = ***@p1(-1, ...);
+)
+
+@script:python depends on report@
+p1 << r3.p1;
+@@
+msg = "x = atomic_add_return(-1, ...)"
+coccilib.report.print_report(p1[0], msg)
+
+

--
2.7.4

Kees Cook

2018-06-14 23:58:07 UTC

Permalink

On Fri, Sep 1, 2017 at 2:40 AM, Elena Reshetova

Post by Elena Reshetova
atomic_as_refcounter.cocci script allows detecting
cases when refcount_t type and API should be used
instead of atomic_t.

Reviewed-by: Kees Cook <***@chromium.org>

Oops, I think this got lost. Who can take this patch? I thought Julia
ran the scripts/coccinelle/ tree, but looking at git log, it looks
more like it's Masahiro? Either way, let's get this in the tree. Who
can take it?

Thanks!

-Kees

Post by Elena Reshetova
---
scripts/coccinelle/api/atomic_as_refcounter.cocci | 131 ++++++++++++++++++++++
1 file changed, 131 insertions(+)
create mode 100644 scripts/coccinelle/api/atomic_as_refcounter.cocci
diff --git a/scripts/coccinelle/api/atomic_as_refcounter.cocci b/scripts/coccinelle/api/atomic_as_refcounter.cocci
new file mode 100644
index 0000000..bfa880d
--- /dev/null
+++ b/scripts/coccinelle/api/atomic_as_refcounter.cocci
@@ -0,0 +1,131 @@
+// Check if refcount_t type and API should be used
+// instead of atomic_t type when dealing with refcounters
+//
+// Copyright (c) 2016-2017, Elena Reshetova, Intel Corporation
+//
+// Confidence: Moderate
+// URL: http://coccinelle.lip6.fr/
+// Options: --include-headers --very-quiet
+
+virtual report
+
+identifier a, x;
+position p1, p2;
+identifier fname =~ ".*free.*";
+identifier fname2 =~ ".*destroy.*";
+identifier fname3 =~ ".*del.*";
+identifier fname4 =~ ".*queue_work.*";
+identifier fname5 =~ ".*schedule_work.*";
+identifier fname6 =~ ".*call_rcu.*";
+
+
+(
+|
+|
+|
+|
+|
+)
+...
+(
+|
+|
+|
+|
+|
+)
+
+
+p1 << r1.p1;
+p2 << r1.p2;
+msg = "atomic_dec_and_test variation before object free at line %s."
+coccilib.report.print_report(p1[0], msg % (p2[0].line))
+
+identifier a, x, y;
+position p1, p2;
+identifier fname =~ ".*free.*";
+
+
+(
+|
+|
+|
+|
+|
+)
+...
+y=a
+...
+
+
+p1 << r4.p1;
+p2 << r4.p2;
+msg = "atomic_dec_and_test variation before object free at line %s."
+coccilib.report.print_report(p1[0], msg % (p2[0].line))
+
+identifier a, x;
+position p1;
+
+(
+|
+|
+)
+
+p1 << r2.p1;
+msg = "atomic_add_unless"
+coccilib.report.print_report(p1[0], msg)
+
+identifier x;
+position p1;
+
+(
+|
+|
+)
+
+p1 << r3.p1;
+msg = "x = atomic_add_return(-1, ...)"
+coccilib.report.print_report(p1[0], msg)
+
+
--
2.7.4

--
Kees Cook
Pixel Security

Julia Lawall

2018-06-15 05:06:20 UTC

Permalink

Post by Kees Cook
On Fri, Sep 1, 2017 at 2:40 AM, Elena Reshetova

Post by Elena Reshetova
atomic_as_refcounter.cocci script allows detecting
cases when refcount_t type and API should be used
instead of atomic_t.

Oops, I think this got lost. Who can take this patch? I thought Julia
ran the scripts/coccinelle/ tree, but looking at git log, it looks
more like it's Masahiro? Either way, let's get this in the tree. Who
can take it?

Masahiro takes patches.

julia

Post by Kees Cook
Thanks!
-Kees

--
Kees Cook
Pixel Security

Masahiro Yamada

2018-06-18 13:47:49 UTC

Permalink

Post by Julia Lawall

Post by Kees Cook
On Fri, Sep 1, 2017 at 2:40 AM, Elena Reshetova

Post by Elena Reshetova
atomic_as_refcounter.cocci script allows detecting
cases when refcount_t type and API should be used
instead of atomic_t.

Masahiro takes patches.
julia

Somehow I missed this one.
Now, applied to linux-kbuild. Thanks for the reminder.

(If Julia hosted a git repository, that would be better, though.)

--
Best Regards
Masahiro Yamada