[PATCH] Coccinelle report script for refcounters

[PATCH] Coccinelle report script for refcounters

[Elena Reshetova]

The below script can be used to detect potential misusage
of atomic_t type and API for reference counting purposes.
Now when we have a dedicated refcount_t type and API with
security protection implemented, people should be using it
instead.

Currently it still reports many occurences since we are
nowhere near the end of our kernel-wide conversion execrise,
but hopefully after couple of cycles more, the amount of
output would be much more limited.

Each script result must be analysed manually before any
conversion, since refcount_t might not suit for certain
purposes (for example if an object is not always destroyed
upon refcounter reaching zero, if increments from zero are
allowed in the code etc.)

As we go further and get less results in output, we will
improve the pattern to detect conversion cases more precisely.

Elena Reshetova (1):
  Coccinelle: add atomic_as_refcounter script

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

-- 
2.7.4

[Cocci] [PATCH] Coccinelle report script for refcounters

[Elena Reshetova]

The below script can be used to detect potential misusage
of atomic_t type and API for reference counting purposes.
Now when we have a dedicated refcount_t type and API with
security protection implemented, people should be using it
instead.

Currently it still reports many occurences since we are
nowhere near the end of our kernel-wide conversion execrise,
but hopefully after couple of cycles more, the amount of
output would be much more limited.

Each script result must be analysed manually before any
conversion, since refcount_t might not suit for certain
purposes (for example if an object is not always destroyed
upon refcounter reaching zero, if increments from zero are
allowed in the code etc.)

As we go further and get less results in output, we will
improve the pattern to detect conversion cases more precisely.

Elena Reshetova (1):
  Coccinelle: add atomic_as_refcounter script

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

-- 
2.7.4

[PATCH] Coccinelle: add atomic_as_refcounter script

[Elena Reshetova]

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 <elena.reshetova@intel.com>
---
 scripts/coccinelle/api/atomic_as_refcounter.cocci | 102 ++++++++++++++++++++++
 1 file changed, 102 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..a16d395
--- /dev/null
+++ b/scripts/coccinelle/api/atomic_as_refcounter.cocci
@@ -0,0 +1,102 @@
+// 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.*";
+
+@@
+
+(
+ atomic_dec_and_test@p1(&(a)->x)
+|
+ atomic_dec_and_lock@p1(&(a)->x, ...)
+|
+ atomic_long_dec_and_lock@p1(&(a)->x, ...)
+|
+ atomic_long_dec_and_test@p1(&(a)->x)
+|
+ atomic64_dec_and_test@p1(&(a)->x)
+|
+ local_dec_and_test@p1(&(a)->x)
+)
+...
+?y=a
+...
+(
+ fname@p2(a, ...);
+|
+ fname@p2(y, ...);
+|
+ fname2@p2(...);
+|
+ fname3@p2(...);
+|
+ fname4@p2(...);
+|
+ fname5@p2(...);
+|
+ fname6@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))
+
+@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 = atomic_add_return@p1(-1, ...);
+|
+x = atomic_long_add_return@p1(-1, ...);
+|
+x = atomic64_add_return@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

[Cocci] [PATCH] Coccinelle: add atomic_as_refcounter script

[Elena Reshetova]

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 <elena.reshetova@intel.com>
---
 scripts/coccinelle/api/atomic_as_refcounter.cocci | 102 ++++++++++++++++++++++
 1 file changed, 102 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..a16d395
--- /dev/null
+++ b/scripts/coccinelle/api/atomic_as_refcounter.cocci
@@ -0,0 +1,102 @@
+// 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
+
+ at 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.*";
+
+@@
+
+(
+ atomic_dec_and_test at p1(&(a)->x)
+|
+ atomic_dec_and_lock at p1(&(a)->x, ...)
+|
+ atomic_long_dec_and_lock at p1(&(a)->x, ...)
+|
+ atomic_long_dec_and_test at p1(&(a)->x)
+|
+ atomic64_dec_and_test at p1(&(a)->x)
+|
+ local_dec_and_test at p1(&(a)->x)
+)
+...
+?y=a
+...
+(
+ fname at p2(a, ...);
+|
+ fname at p2(y, ...);
+|
+ fname2 at p2(...);
+|
+ fname3 at p2(...);
+|
+ fname4 at p2(...);
+|
+ fname5 at p2(...);
+|
+ fname6 at p2(...);
+)
+
+
+ at 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))
+
+@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
+)
+
+ at script:python depends on report@
+p1 << r2.p1;
+@@
+msg = "atomic_add_unless"
+coccilib.report.print_report(p1[0], msg)
+
+ at r3 exists@
+identifier x;
+position p1;
+@@
+
+(
+x = atomic_add_return at p1(-1, ...);
+|
+x = atomic_long_add_return at p1(-1, ...);
+|
+x = atomic64_add_return at 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

Re: [PATCH] Coccinelle report script for refcounters

[Julia Lawall]

On Tue, 18 Jul 2017, Elena Reshetova wrote:

> The below script can be used to detect potential misusage
> of atomic_t type and API for reference counting purposes.
> Now when we have a dedicated refcount_t type and API with
> security protection implemented, people should be using it
> instead.
>
> Currently it still reports many occurences since we are
> nowhere near the end of our kernel-wide conversion execrise,
> but hopefully after couple of cycles more, the amount of
> output would be much more limited.
>
> Each script result must be analysed manually before any
> conversion, since refcount_t might not suit for certain
> purposes (for example if an object is not always destroyed
> upon refcounter reaching zero, if increments from zero are
> allowed in the code etc.)
>
> As we go further and get less results in output, we will
> improve the pattern to detect conversion cases more precisely.

The regexps are the best you can do?

julia

>
> Elena Reshetova (1):
>   Coccinelle: add atomic_as_refcounter script
>
>  scripts/coccinelle/api/atomic_as_refcounter.cocci | 102 ++++++++++++++++++++++
>  1 file changed, 102 insertions(+)
>  create mode 100644 scripts/coccinelle/api/atomic_as_refcounter.cocci
>
> --
> 2.7.4
>
>

[Cocci] [PATCH] Coccinelle report script for refcounters

[Julia Lawall]

On Tue, 18 Jul 2017, Elena Reshetova wrote:

> The below script can be used to detect potential misusage
> of atomic_t type and API for reference counting purposes.
> Now when we have a dedicated refcount_t type and API with
> security protection implemented, people should be using it
> instead.
>
> Currently it still reports many occurences since we are
> nowhere near the end of our kernel-wide conversion execrise,
> but hopefully after couple of cycles more, the amount of
> output would be much more limited.
>
> Each script result must be analysed manually before any
> conversion, since refcount_t might not suit for certain
> purposes (for example if an object is not always destroyed
> upon refcounter reaching zero, if increments from zero are
> allowed in the code etc.)
>
> As we go further and get less results in output, we will
> improve the pattern to detect conversion cases more precisely.

The regexps are the best you can do?

julia

>
> Elena Reshetova (1):
>   Coccinelle: add atomic_as_refcounter script
>
>  scripts/coccinelle/api/atomic_as_refcounter.cocci | 102 ++++++++++++++++++++++
>  1 file changed, 102 insertions(+)
>  create mode 100644 scripts/coccinelle/api/atomic_as_refcounter.cocci
>
> --
> 2.7.4
>
>

RE: [PATCH] Coccinelle report script for refcounters

[Reshetova, Elena]

> On Tue, 18 Jul 2017, Elena Reshetova wrote:
> 
> > The below script can be used to detect potential misusage
> > of atomic_t type and API for reference counting purposes.
> > Now when we have a dedicated refcount_t type and API with
> > security protection implemented, people should be using it
> > instead.
> >
> > Currently it still reports many occurences since we are
> > nowhere near the end of our kernel-wide conversion execrise,
> > but hopefully after couple of cycles more, the amount of
> > output would be much more limited.
> >
> > Each script result must be analysed manually before any
> > conversion, since refcount_t might not suit for certain
> > purposes (for example if an object is not always destroyed
> > upon refcounter reaching zero, if increments from zero are
> > allowed in the code etc.)
> >
> > As we go further and get less results in output, we will
> > improve the pattern to detect conversion cases more precisely.
> 
> The regexps are the best you can do?

They are simple and so far they were sufficient for the purpose since
they found pretty much all the cases we are aware about. I was thinking
on working to improve the pattern later on after we merge the bulk of
conversions and I have some cycles free on that front.

What would you suggest to do instead of regexps?

Best Regards,
Elena.

> 
> julia
> 
> >
> > Elena Reshetova (1):
> >   Coccinelle: add atomic_as_refcounter script
> >
> >  scripts/coccinelle/api/atomic_as_refcounter.cocci | 102
> ++++++++++++++++++++++
> >  1 file changed, 102 insertions(+)
> >  create mode 100644 scripts/coccinelle/api/atomic_as_refcounter.cocci
> >
> > --
> > 2.7.4
> >
> >

[Cocci] [PATCH] Coccinelle report script for refcounters

[Reshetova, Elena]

> On Tue, 18 Jul 2017, Elena Reshetova wrote:
> 
> > The below script can be used to detect potential misusage
> > of atomic_t type and API for reference counting purposes.
> > Now when we have a dedicated refcount_t type and API with
> > security protection implemented, people should be using it
> > instead.
> >
> > Currently it still reports many occurences since we are
> > nowhere near the end of our kernel-wide conversion execrise,
> > but hopefully after couple of cycles more, the amount of
> > output would be much more limited.
> >
> > Each script result must be analysed manually before any
> > conversion, since refcount_t might not suit for certain
> > purposes (for example if an object is not always destroyed
> > upon refcounter reaching zero, if increments from zero are
> > allowed in the code etc.)
> >
> > As we go further and get less results in output, we will
> > improve the pattern to detect conversion cases more precisely.
> 
> The regexps are the best you can do?

They are simple and so far they were sufficient for the purpose since
they found pretty much all the cases we are aware about. I was thinking
on working to improve the pattern later on after we merge the bulk of
conversions and I have some cycles free on that front.

What would you suggest to do instead of regexps?

Best Regards,
Elena.

> 
> julia
> 
> >
> > Elena Reshetova (1):
> >   Coccinelle: add atomic_as_refcounter script
> >
> >  scripts/coccinelle/api/atomic_as_refcounter.cocci | 102
> ++++++++++++++++++++++
> >  1 file changed, 102 insertions(+)
> >  create mode 100644 scripts/coccinelle/api/atomic_as_refcounter.cocci
> >
> > --
> > 2.7.4
> >
> >

RE: [PATCH] Coccinelle report script for refcounters

[Julia Lawall]

On Tue, 18 Jul 2017, Reshetova, Elena wrote:

> > On Tue, 18 Jul 2017, Elena Reshetova wrote:
> >
> > > The below script can be used to detect potential misusage
> > > of atomic_t type and API for reference counting purposes.
> > > Now when we have a dedicated refcount_t type and API with
> > > security protection implemented, people should be using it
> > > instead.
> > >
> > > Currently it still reports many occurences since we are
> > > nowhere near the end of our kernel-wide conversion execrise,
> > > but hopefully after couple of cycles more, the amount of
> > > output would be much more limited.
> > >
> > > Each script result must be analysed manually before any
> > > conversion, since refcount_t might not suit for certain
> > > purposes (for example if an object is not always destroyed
> > > upon refcounter reaching zero, if increments from zero are
> > > allowed in the code etc.)
> > >
> > > As we go further and get less results in output, we will
> > > improve the pattern to detect conversion cases more precisely.
> >
> > The regexps are the best you can do?
>
> They are simple and so far they were sufficient for the purpose since
> they found pretty much all the cases we are aware about. I was thinking
> on working to improve the pattern later on after we merge the bulk of
> conversions and I have some cycles free on that front.
>
> What would you suggest to do instead of regexps?

Is there anything about the definitions of these functions that indicates
why they are important?

julia

>
> Best Regards,
> Elena.
>
> >
> > julia
> >
> > >
> > > Elena Reshetova (1):
> > >   Coccinelle: add atomic_as_refcounter script
> > >
> > >  scripts/coccinelle/api/atomic_as_refcounter.cocci | 102
> > ++++++++++++++++++++++
> > >  1 file changed, 102 insertions(+)
> > >  create mode 100644 scripts/coccinelle/api/atomic_as_refcounter.cocci
> > >
> > > --
> > > 2.7.4
> > >
> > >
>

[Cocci] [PATCH] Coccinelle report script for refcounters

[Julia Lawall]

On Tue, 18 Jul 2017, Reshetova, Elena wrote:

> > On Tue, 18 Jul 2017, Elena Reshetova wrote:
> >
> > > The below script can be used to detect potential misusage
> > > of atomic_t type and API for reference counting purposes.
> > > Now when we have a dedicated refcount_t type and API with
> > > security protection implemented, people should be using it
> > > instead.
> > >
> > > Currently it still reports many occurences since we are
> > > nowhere near the end of our kernel-wide conversion execrise,
> > > but hopefully after couple of cycles more, the amount of
> > > output would be much more limited.
> > >
> > > Each script result must be analysed manually before any
> > > conversion, since refcount_t might not suit for certain
> > > purposes (for example if an object is not always destroyed
> > > upon refcounter reaching zero, if increments from zero are
> > > allowed in the code etc.)
> > >
> > > As we go further and get less results in output, we will
> > > improve the pattern to detect conversion cases more precisely.
> >
> > The regexps are the best you can do?
>
> They are simple and so far they were sufficient for the purpose since
> they found pretty much all the cases we are aware about. I was thinking
> on working to improve the pattern later on after we merge the bulk of
> conversions and I have some cycles free on that front.
>
> What would you suggest to do instead of regexps?

Is there anything about the definitions of these functions that indicates
why they are important?

julia

>
> Best Regards,
> Elena.
>
> >
> > julia
> >
> > >
> > > Elena Reshetova (1):
> > >   Coccinelle: add atomic_as_refcounter script
> > >
> > >  scripts/coccinelle/api/atomic_as_refcounter.cocci | 102
> > ++++++++++++++++++++++
> > >  1 file changed, 102 insertions(+)
> > >  create mode 100644 scripts/coccinelle/api/atomic_as_refcounter.cocci
> > >
> > > --
> > > 2.7.4
> > >
> > >
>

RE: [PATCH] Coccinelle report script for refcounters

[Reshetova, Elena]

> On Tue, 18 Jul 2017, Reshetova, Elena wrote:
> 
> > > On Tue, 18 Jul 2017, Elena Reshetova wrote:
> > >
> > > > The below script can be used to detect potential misusage
> > > > of atomic_t type and API for reference counting purposes.
> > > > Now when we have a dedicated refcount_t type and API with
> > > > security protection implemented, people should be using it
> > > > instead.
> > > >
> > > > Currently it still reports many occurences since we are
> > > > nowhere near the end of our kernel-wide conversion execrise,
> > > > but hopefully after couple of cycles more, the amount of
> > > > output would be much more limited.
> > > >
> > > > Each script result must be analysed manually before any
> > > > conversion, since refcount_t might not suit for certain
> > > > purposes (for example if an object is not always destroyed
> > > > upon refcounter reaching zero, if increments from zero are
> > > > allowed in the code etc.)
> > > >
> > > > As we go further and get less results in output, we will
> > > > improve the pattern to detect conversion cases more precisely.
> > >
> > > The regexps are the best you can do?
> >
> > They are simple and so far they were sufficient for the purpose since
> > they found pretty much all the cases we are aware about. I was thinking
> > on working to improve the pattern later on after we merge the bulk of
> > conversions and I have some cycles free on that front.
> >
> > What would you suggest to do instead of regexps?
> 
> Is there anything about the definitions of these functions that indicates
> why they are important?

I am not sure I understand the question fully. Do you mean the functions
used in the rules, such as atomic_dec_and_test() etc.?
If yes, then for example the combination of atomic_dec_and_test(&(a)->x)
on a pointer, then followed later by some kind of *free*(a) function (kfree, kmem_cache_free() etc.)
on that pointer is a quite common indicator that we are dealing with a reference counter since
they would normally free resources when counter reaches zero. 
Again, it is not a 100% indicator since I have seen weird schemes that for example
free a resource upon reaching -1, or free it in one case and don't free on another,
but such cases are more rare. 

Does this answer your questions?

Best Regards,
Elena.

> 
> julia
> 
> >
> > Best Regards,
> > Elena.
> >
> > >
> > > julia
> > >
> > > >
> > > > Elena Reshetova (1):
> > > >   Coccinelle: add atomic_as_refcounter script
> > > >
> > > >  scripts/coccinelle/api/atomic_as_refcounter.cocci | 102
> > > ++++++++++++++++++++++
> > > >  1 file changed, 102 insertions(+)
> > > >  create mode 100644 scripts/coccinelle/api/atomic_as_refcounter.cocci
> > > >
> > > > --
> > > > 2.7.4
> > > >
> > > >
> >

[Cocci] [PATCH] Coccinelle report script for refcounters

[Reshetova, Elena]

> On Tue, 18 Jul 2017, Reshetova, Elena wrote:
> 
> > > On Tue, 18 Jul 2017, Elena Reshetova wrote:
> > >
> > > > The below script can be used to detect potential misusage
> > > > of atomic_t type and API for reference counting purposes.
> > > > Now when we have a dedicated refcount_t type and API with
> > > > security protection implemented, people should be using it
> > > > instead.
> > > >
> > > > Currently it still reports many occurences since we are
> > > > nowhere near the end of our kernel-wide conversion execrise,
> > > > but hopefully after couple of cycles more, the amount of
> > > > output would be much more limited.
> > > >
> > > > Each script result must be analysed manually before any
> > > > conversion, since refcount_t might not suit for certain
> > > > purposes (for example if an object is not always destroyed
> > > > upon refcounter reaching zero, if increments from zero are
> > > > allowed in the code etc.)
> > > >
> > > > As we go further and get less results in output, we will
> > > > improve the pattern to detect conversion cases more precisely.
> > >
> > > The regexps are the best you can do?
> >
> > They are simple and so far they were sufficient for the purpose since
> > they found pretty much all the cases we are aware about. I was thinking
> > on working to improve the pattern later on after we merge the bulk of
> > conversions and I have some cycles free on that front.
> >
> > What would you suggest to do instead of regexps?
> 
> Is there anything about the definitions of these functions that indicates
> why they are important?

I am not sure I understand the question fully. Do you mean the functions
used in the rules, such as atomic_dec_and_test() etc.?
If yes, then for example the combination of atomic_dec_and_test(&(a)->x)
on a pointer, then followed later by some kind of *free*(a) function (kfree, kmem_cache_free() etc.)
on that pointer is a quite common indicator that we are dealing with a reference counter since
they would normally free resources when counter reaches zero. 
Again, it is not a 100% indicator since I have seen weird schemes that for example
free a resource upon reaching -1, or free it in one case and don't free on another,
but such cases are more rare. 

Does this answer your questions?

Best Regards,
Elena.

> 
> julia
> 
> >
> > Best Regards,
> > Elena.
> >
> > >
> > > julia
> > >
> > > >
> > > > Elena Reshetova (1):
> > > >   Coccinelle: add atomic_as_refcounter script
> > > >
> > > >  scripts/coccinelle/api/atomic_as_refcounter.cocci | 102
> > > ++++++++++++++++++++++
> > > >  1 file changed, 102 insertions(+)
> > > >  create mode 100644 scripts/coccinelle/api/atomic_as_refcounter.cocci
> > > >
> > > > --
> > > > 2.7.4
> > > >
> > > >
> >

RE: [PATCH] Coccinelle report script for refcounters

[Julia Lawall]

On Tue, 18 Jul 2017, Reshetova, Elena wrote:

> > On Tue, 18 Jul 2017, Reshetova, Elena wrote:
> >
> > > > On Tue, 18 Jul 2017, Elena Reshetova wrote:
> > > >
> > > > > The below script can be used to detect potential misusage
> > > > > of atomic_t type and API for reference counting purposes.
> > > > > Now when we have a dedicated refcount_t type and API with
> > > > > security protection implemented, people should be using it
> > > > > instead.
> > > > >
> > > > > Currently it still reports many occurences since we are
> > > > > nowhere near the end of our kernel-wide conversion execrise,
> > > > > but hopefully after couple of cycles more, the amount of
> > > > > output would be much more limited.
> > > > >
> > > > > Each script result must be analysed manually before any
> > > > > conversion, since refcount_t might not suit for certain
> > > > > purposes (for example if an object is not always destroyed
> > > > > upon refcounter reaching zero, if increments from zero are
> > > > > allowed in the code etc.)
> > > > >
> > > > > As we go further and get less results in output, we will
> > > > > improve the pattern to detect conversion cases more precisely.
> > > >
> > > > The regexps are the best you can do?
> > >
> > > They are simple and so far they were sufficient for the purpose since
> > > they found pretty much all the cases we are aware about. I was thinking
> > > on working to improve the pattern later on after we merge the bulk of
> > > conversions and I have some cycles free on that front.
> > >
> > > What would you suggest to do instead of regexps?
> >
> > Is there anything about the definitions of these functions that indicates
> > why they are important?
>
> I am not sure I understand the question fully. Do you mean the functions
> used in the rules, such as atomic_dec_and_test() etc.?
> If yes, then for example the combination of atomic_dec_and_test(&(a)->x)
> on a pointer, then followed later by some kind of *free*(a) function (kfree, kmem_cache_free() etc.)
> on that pointer is a quite common indicator that we are dealing with a reference counter since
> they would normally free resources when counter reaches zero.
> Again, it is not a 100% indicator since I have seen weird schemes that for example
> free a resource upon reaching -1, or free it in one case and don't free on another,
> but such cases are more rare.

I just meant that the name of the function is not always reliable.  Maybe
there is some other property of the definition of the function, like being
a wrapper for kfree, that would be more reliable.  But in this case it is
quite possible that there is not.  We can try with the rule as is and see
what happens.

julia


>
> Does this answer your questions?
>
> Best Regards,
> Elena.
>
> >
> > julia
> >
> > >
> > > Best Regards,
> > > Elena.
> > >
> > > >
> > > > julia
> > > >
> > > > >
> > > > > Elena Reshetova (1):
> > > > >   Coccinelle: add atomic_as_refcounter script
> > > > >
> > > > >  scripts/coccinelle/api/atomic_as_refcounter.cocci | 102
> > > > ++++++++++++++++++++++
> > > > >  1 file changed, 102 insertions(+)
> > > > >  create mode 100644 scripts/coccinelle/api/atomic_as_refcounter.cocci
> > > > >
> > > > > --
> > > > > 2.7.4
> > > > >
> > > > >
> > >
>

[Cocci] [PATCH] Coccinelle report script for refcounters

[Julia Lawall]

On Tue, 18 Jul 2017, Reshetova, Elena wrote:

> > On Tue, 18 Jul 2017, Reshetova, Elena wrote:
> >
> > > > On Tue, 18 Jul 2017, Elena Reshetova wrote:
> > > >
> > > > > The below script can be used to detect potential misusage
> > > > > of atomic_t type and API for reference counting purposes.
> > > > > Now when we have a dedicated refcount_t type and API with
> > > > > security protection implemented, people should be using it
> > > > > instead.
> > > > >
> > > > > Currently it still reports many occurences since we are
> > > > > nowhere near the end of our kernel-wide conversion execrise,
> > > > > but hopefully after couple of cycles more, the amount of
> > > > > output would be much more limited.
> > > > >
> > > > > Each script result must be analysed manually before any
> > > > > conversion, since refcount_t might not suit for certain
> > > > > purposes (for example if an object is not always destroyed
> > > > > upon refcounter reaching zero, if increments from zero are
> > > > > allowed in the code etc.)
> > > > >
> > > > > As we go further and get less results in output, we will
> > > > > improve the pattern to detect conversion cases more precisely.
> > > >
> > > > The regexps are the best you can do?
> > >
> > > They are simple and so far they were sufficient for the purpose since
> > > they found pretty much all the cases we are aware about. I was thinking
> > > on working to improve the pattern later on after we merge the bulk of
> > > conversions and I have some cycles free on that front.
> > >
> > > What would you suggest to do instead of regexps?
> >
> > Is there anything about the definitions of these functions that indicates
> > why they are important?
>
> I am not sure I understand the question fully. Do you mean the functions
> used in the rules, such as atomic_dec_and_test() etc.?
> If yes, then for example the combination of atomic_dec_and_test(&(a)->x)
> on a pointer, then followed later by some kind of *free*(a) function (kfree, kmem_cache_free() etc.)
> on that pointer is a quite common indicator that we are dealing with a reference counter since
> they would normally free resources when counter reaches zero.
> Again, it is not a 100% indicator since I have seen weird schemes that for example
> free a resource upon reaching -1, or free it in one case and don't free on another,
> but such cases are more rare.

I just meant that the name of the function is not always reliable.  Maybe
there is some other property of the definition of the function, like being
a wrapper for kfree, that would be more reliable.  But in this case it is
quite possible that there is not.  We can try with the rule as is and see
what happens.

julia


>
> Does this answer your questions?
>
> Best Regards,
> Elena.
>
> >
> > julia
> >
> > >
> > > Best Regards,
> > > Elena.
> > >
> > > >
> > > > julia
> > > >
> > > > >
> > > > > Elena Reshetova (1):
> > > > >   Coccinelle: add atomic_as_refcounter script
> > > > >
> > > > >  scripts/coccinelle/api/atomic_as_refcounter.cocci | 102
> > > > ++++++++++++++++++++++
> > > > >  1 file changed, 102 insertions(+)
> > > > >  create mode 100644 scripts/coccinelle/api/atomic_as_refcounter.cocci
> > > > >
> > > > > --
> > > > > 2.7.4
> > > > >
> > > > >
> > >
>

Re: [PATCH] Coccinelle: add atomic_as_refcounter script

[Kees Cook]

On Tue, Jul 18, 2017 at 12:48 AM, Elena Reshetova
<elena.reshetova@intel.com> wrote:
> 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 <elena.reshetova@intel.com>
> ---
>  scripts/coccinelle/api/atomic_as_refcounter.cocci | 102 ++++++++++++++++++++++
>  1 file changed, 102 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..a16d395
> --- /dev/null
> +++ b/scripts/coccinelle/api/atomic_as_refcounter.cocci
> @@ -0,0 +1,102 @@
> +// 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.*";
> +
> +@@
> +
> +(
> + atomic_dec_and_test@p1(&(a)->x)
> [...]
> +)
> +...
> +?y=a
> +...
> +(
> + fname@p2(a, ...);
> +|
> + fname@p2(y, ...);
> +|
> [...]

Just to double check, this "?y=a" catches the seccomp case I pointed out?

        while (orig && atomic_dec_and_test(&orig->usage)) {
                struct seccomp_filter *freeme = orig;
                orig = orig->prev;
                seccomp_filter_free(freeme);
        }

Seems like it should match. Did this find anything else besides seccomp?

-Kees

-- 
Kees Cook
Pixel Security

[Cocci] [PATCH] Coccinelle: add atomic_as_refcounter script

[Kees Cook]

On Tue, Jul 18, 2017 at 12:48 AM, Elena Reshetova
<elena.reshetova@intel.com> wrote:
> 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 <elena.reshetova@intel.com>
> ---
>  scripts/coccinelle/api/atomic_as_refcounter.cocci | 102 ++++++++++++++++++++++
>  1 file changed, 102 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..a16d395
> --- /dev/null
> +++ b/scripts/coccinelle/api/atomic_as_refcounter.cocci
> @@ -0,0 +1,102 @@
> +// 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
> +
> + at 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.*";
> +
> +@@
> +
> +(
> + atomic_dec_and_test at p1(&(a)->x)
> [...]
> +)
> +...
> +?y=a
> +...
> +(
> + fname at p2(a, ...);
> +|
> + fname at p2(y, ...);
> +|
> [...]

Just to double check, this "?y=a" catches the seccomp case I pointed out?

        while (orig && atomic_dec_and_test(&orig->usage)) {
                struct seccomp_filter *freeme = orig;
                orig = orig->prev;
                seccomp_filter_free(freeme);
        }

Seems like it should match. Did this find anything else besides seccomp?

-Kees

-- 
Kees Cook
Pixel Security

RE: [PATCH] Coccinelle: add atomic_as_refcounter script

[Reshetova, Elena]

 On Tue, Jul 18, 2017 at 12:48 AM, Elena Reshetova
> <elena.reshetova@intel.com> wrote:
> > 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 <elena.reshetova@intel.com>
> > ---
> >  scripts/coccinelle/api/atomic_as_refcounter.cocci | 102
> ++++++++++++++++++++++
> >  1 file changed, 102 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..a16d395
> > --- /dev/null
> > +++ b/scripts/coccinelle/api/atomic_as_refcounter.cocci
> > @@ -0,0 +1,102 @@
> > +// 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.*";
> > +
> > +@@
> > +
> > +(
> > + atomic_dec_and_test@p1(&(a)->x)
> > [...]
> > +)
> > +...
> > +?y=a
> > +...
> > +(
> > + fname@p2(a, ...);
> > +|
> > + fname@p2(y, ...);
> > +|
> > [...]
> 
> Just to double check, this "?y=a" catches the seccomp case I pointed out?
> 
>         while (orig && atomic_dec_and_test(&orig->usage)) {
>                 struct seccomp_filter *freeme = orig;
>                 orig = orig->prev;
>                 seccomp_filter_free(freeme);
>         }
> 

Yes, it does find the seccomp case, I was specifically testing this new addition on it. 


> Seems like it should match. Did this find anything else besides seccomp?

Yes, it found about 20 new things, but I haven't had a chance to look at them all yet.
In any case, I would really love to merge the existing conversions first (we still have about 80 patches left)
and only after add more of them. I looked at some new found cases and for example this was one:

./crypto/cryptd.c:474:38-57: atomic_dec_and_test variation before object free at line 475.

static void cryptd_skcipher_complete(struct skcipher_request *req, int err)
{
    struct crypto_skcipher *tfm = crypto_skcipher_reqtfm(req);
    struct cryptd_skcipher_ctx *ctx = crypto_skcipher_ctx(tfm);
    struct cryptd_skcipher_request_ctx *rctx = skcipher_request_ctx(req);
    int refcnt = atomic_read(&ctx->refcnt);

    local_bh_disable();
    rctx->complete(&req->base, err);
    local_bh_enable();

    if (err != -EINPROGRESS && refcnt && atomic_dec_and_test(&ctx->refcnt))
        crypto_free_skcipher(tfm);
}

While it isn't exactly the case I had in mind when trying to modify the pattern to work
for seccomp case, it came as a nice bonus IMO since we do want to catch these cases as well.
Overall it seems that pointers/structures can be so nicely wrapped around in some cases,
that keeping the pattern as generic as possible is a good way to go. Otherwise we might
start losing cases ( I would prefer a bit more false positives in this case instead as soon as
they are fine to manage). 

Best Regards,
Elena.

> 
> -Kees
> 
> --
> Kees Cook
> Pixel Security

[Cocci] [PATCH] Coccinelle: add atomic_as_refcounter script

[Reshetova, Elena]

 On Tue, Jul 18, 2017 at 12:48 AM, Elena Reshetova
> <elena.reshetova@intel.com> wrote:
> > 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 <elena.reshetova@intel.com>
> > ---
> >  scripts/coccinelle/api/atomic_as_refcounter.cocci | 102
> ++++++++++++++++++++++
> >  1 file changed, 102 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..a16d395
> > --- /dev/null
> > +++ b/scripts/coccinelle/api/atomic_as_refcounter.cocci
> > @@ -0,0 +1,102 @@
> > +// 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
> > +
> > + at 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.*";
> > +
> > +@@
> > +
> > +(
> > + atomic_dec_and_test at p1(&(a)->x)
> > [...]
> > +)
> > +...
> > +?y=a
> > +...
> > +(
> > + fname at p2(a, ...);
> > +|
> > + fname at p2(y, ...);
> > +|
> > [...]
> 
> Just to double check, this "?y=a" catches the seccomp case I pointed out?
> 
>         while (orig && atomic_dec_and_test(&orig->usage)) {
>                 struct seccomp_filter *freeme = orig;
>                 orig = orig->prev;
>                 seccomp_filter_free(freeme);
>         }
> 

Yes, it does find the seccomp case, I was specifically testing this new addition on it. 


> Seems like it should match. Did this find anything else besides seccomp?

Yes, it found about 20 new things, but I haven't had a chance to look at them all yet.
In any case, I would really love to merge the existing conversions first (we still have about 80 patches left)
and only after add more of them. I looked at some new found cases and for example this was one:

./crypto/cryptd.c:474:38-57: atomic_dec_and_test variation before object free at line 475.

static void cryptd_skcipher_complete(struct skcipher_request *req, int err)
{
    struct crypto_skcipher *tfm = crypto_skcipher_reqtfm(req);
    struct cryptd_skcipher_ctx *ctx = crypto_skcipher_ctx(tfm);
    struct cryptd_skcipher_request_ctx *rctx = skcipher_request_ctx(req);
    int refcnt = atomic_read(&ctx->refcnt);

    local_bh_disable();
    rctx->complete(&req->base, err);
    local_bh_enable();

    if (err != -EINPROGRESS && refcnt && atomic_dec_and_test(&ctx->refcnt))
        crypto_free_skcipher(tfm);
}

While it isn't exactly the case I had in mind when trying to modify the pattern to work
for seccomp case, it came as a nice bonus IMO since we do want to catch these cases as well.
Overall it seems that pointers/structures can be so nicely wrapped around in some cases,
that keeping the pattern as generic as possible is a good way to go. Otherwise we might
start losing cases ( I would prefer a bit more false positives in this case instead as soon as
they are fine to manage). 

Best Regards,
Elena.

> 
> -Kees
> 
> --
> Kees Cook
> Pixel Security

Re: [PATCH] Coccinelle: add atomic_as_refcounter script

[Julia Lawall]

On Tue, 18 Jul 2017, Elena Reshetova wrote:

> 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 <elena.reshetova@intel.com>
> ---
>  scripts/coccinelle/api/atomic_as_refcounter.cocci | 102 ++++++++++++++++++++++
>  1 file changed, 102 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..a16d395
> --- /dev/null
> +++ b/scripts/coccinelle/api/atomic_as_refcounter.cocci
> @@ -0,0 +1,102 @@
> +// 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.*";
> +
> +@@
> +
> +(
> + atomic_dec_and_test@p1(&(a)->x)
> +|
> + atomic_dec_and_lock@p1(&(a)->x, ...)
> +|
> + atomic_long_dec_and_lock@p1(&(a)->x, ...)
> +|
> + atomic_long_dec_and_test@p1(&(a)->x)
> +|
> + atomic64_dec_and_test@p1(&(a)->x)
> +|
> + local_dec_and_test@p1(&(a)->x)
> +)
> +...
> +?y=a

This makes the line optional. And if it dosn't appear, there is no
constraint on y.  So the rule matches:

int main() {
  atomic64_dec_and_test(&(a)->x);
  free(b);
}

I would suggest to just make two rules, one with y=a and one without.

julia

> +...
> +(
> + fname@p2(a, ...);
> +|
> + fname@p2(y, ...);
> +|
> + fname2@p2(...);
> +|
> + fname3@p2(...);
> +|
> + fname4@p2(...);
> +|
> + fname5@p2(...);
> +|
> + fname6@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))
> +
> +@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 = atomic_add_return@p1(-1, ...);
> +|
> +x = atomic_long_add_return@p1(-1, ...);
> +|
> +x = atomic64_add_return@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
>
>

[Cocci] [PATCH] Coccinelle: add atomic_as_refcounter script

[Julia Lawall]

On Tue, 18 Jul 2017, Elena Reshetova wrote:

> 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 <elena.reshetova@intel.com>
> ---
>  scripts/coccinelle/api/atomic_as_refcounter.cocci | 102 ++++++++++++++++++++++
>  1 file changed, 102 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..a16d395
> --- /dev/null
> +++ b/scripts/coccinelle/api/atomic_as_refcounter.cocci
> @@ -0,0 +1,102 @@
> +// 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
> +
> + at 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.*";
> +
> +@@
> +
> +(
> + atomic_dec_and_test at p1(&(a)->x)
> +|
> + atomic_dec_and_lock at p1(&(a)->x, ...)
> +|
> + atomic_long_dec_and_lock at p1(&(a)->x, ...)
> +|
> + atomic_long_dec_and_test at p1(&(a)->x)
> +|
> + atomic64_dec_and_test at p1(&(a)->x)
> +|
> + local_dec_and_test at p1(&(a)->x)
> +)
> +...
> +?y=a

This makes the line optional. And if it dosn't appear, there is no
constraint on y.  So the rule matches:

int main() {
  atomic64_dec_and_test(&(a)->x);
  free(b);
}

I would suggest to just make two rules, one with y=a and one without.

julia

> +...
> +(
> + fname at p2(a, ...);
> +|
> + fname at p2(y, ...);
> +|
> + fname2 at p2(...);
> +|
> + fname3 at p2(...);
> +|
> + fname4 at p2(...);
> +|
> + fname5 at p2(...);
> +|
> + fname6 at p2(...);
> +)
> +
> +
> + at 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))
> +
> + at 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
> +)
> +
> + at script:python depends on report@
> +p1 << r2.p1;
> +@@
> +msg = "atomic_add_unless"
> +coccilib.report.print_report(p1[0], msg)
> +
> + at r3 exists@
> +identifier x;
> +position p1;
> +@@
> +
> +(
> +x = atomic_add_return at p1(-1, ...);
> +|
> +x = atomic_long_add_return at p1(-1, ...);
> +|
> +x = atomic64_add_return at p1(-1, ...);
> +)
> +
> + at script:python depends on report@
> +p1 << r3.p1;
> +@@
> +msg = "x = atomic_add_return(-1, ...)"
> +coccilib.report.print_report(p1[0], msg)
> +
> +
> --
> 2.7.4
>
>

RE: [PATCH] Coccinelle: add atomic_as_refcounter script

[Reshetova, Elena]

> On Tue, 18 Jul 2017, Elena Reshetova wrote:
> 
> > 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 <elena.reshetova@intel.com>
> > ---
> >  scripts/coccinelle/api/atomic_as_refcounter.cocci | 102
> ++++++++++++++++++++++
> >  1 file changed, 102 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..a16d395
> > --- /dev/null
> > +++ b/scripts/coccinelle/api/atomic_as_refcounter.cocci
> > @@ -0,0 +1,102 @@
> > +// 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.*";
> > +
> > +@@
> > +
> > +(
> > + atomic_dec_and_test@p1(&(a)->x)
> > +|
> > + atomic_dec_and_lock@p1(&(a)->x, ...)
> > +|
> > + atomic_long_dec_and_lock@p1(&(a)->x, ...)
> > +|
> > + atomic_long_dec_and_test@p1(&(a)->x)
> > +|
> > + atomic64_dec_and_test@p1(&(a)->x)
> > +|
> > + local_dec_and_test@p1(&(a)->x)
> > +)
> > +...
> > +?y=a
> 
> This makes the line optional. And if it dosn't appear, there is no
> constraint on y.  So the rule matches:
> 
> int main() {
>   atomic64_dec_and_test(&(a)->x);
>   free(b);
> }
> 
> I would suggest to just make two rules, one with y=a and one without.

Oh, thank you for the catch! I was a bit afraid it might be the case, but when
I tried it in practice it didn't show up that many additional cases compare to
not having ?y=a at all (only 20 or so), and when I checked some, they looked
worth a manual check anyway and interesting. 

But I will fix the rule and send a new version!

Best Regards,
Elena.

> 
> julia
> 
> > +...
> > +(
> > + fname@p2(a, ...);
> > +|
> > + fname@p2(y, ...);
> > +|
> > + fname2@p2(...);
> > +|
> > + fname3@p2(...);
> > +|
> > + fname4@p2(...);
> > +|
> > + fname5@p2(...);
> > +|
> > + fname6@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))
> > +
> > +@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 = atomic_add_return@p1(-1, ...);
> > +|
> > +x = atomic_long_add_return@p1(-1, ...);
> > +|
> > +x = atomic64_add_return@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
> >
> >

[Cocci] [PATCH] Coccinelle: add atomic_as_refcounter script

[Reshetova, Elena]

> On Tue, 18 Jul 2017, Elena Reshetova wrote:
> 
> > 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 <elena.reshetova@intel.com>
> > ---
> >  scripts/coccinelle/api/atomic_as_refcounter.cocci | 102
> ++++++++++++++++++++++
> >  1 file changed, 102 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..a16d395
> > --- /dev/null
> > +++ b/scripts/coccinelle/api/atomic_as_refcounter.cocci
> > @@ -0,0 +1,102 @@
> > +// 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
> > +
> > + at 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.*";
> > +
> > +@@
> > +
> > +(
> > + atomic_dec_and_test at p1(&(a)->x)
> > +|
> > + atomic_dec_and_lock at p1(&(a)->x, ...)
> > +|
> > + atomic_long_dec_and_lock at p1(&(a)->x, ...)
> > +|
> > + atomic_long_dec_and_test at p1(&(a)->x)
> > +|
> > + atomic64_dec_and_test at p1(&(a)->x)
> > +|
> > + local_dec_and_test at p1(&(a)->x)
> > +)
> > +...
> > +?y=a
> 
> This makes the line optional. And if it dosn't appear, there is no
> constraint on y.  So the rule matches:
> 
> int main() {
>   atomic64_dec_and_test(&(a)->x);
>   free(b);
> }
> 
> I would suggest to just make two rules, one with y=a and one without.

Oh, thank you for the catch! I was a bit afraid it might be the case, but when
I tried it in practice it didn't show up that many additional cases compare to
not having ?y=a at all (only 20 or so), and when I checked some, they looked
worth a manual check anyway and interesting. 

But I will fix the rule and send a new version!

Best Regards,
Elena.

> 
> julia
> 
> > +...
> > +(
> > + fname at p2(a, ...);
> > +|
> > + fname at p2(y, ...);
> > +|
> > + fname2 at p2(...);
> > +|
> > + fname3 at p2(...);
> > +|
> > + fname4 at p2(...);
> > +|
> > + fname5 at p2(...);
> > +|
> > + fname6 at p2(...);
> > +)
> > +
> > +
> > + at 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))
> > +
> > + at 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
> > +)
> > +
> > + at script:python depends on report@
> > +p1 << r2.p1;
> > +@@
> > +msg = "atomic_add_unless"
> > +coccilib.report.print_report(p1[0], msg)
> > +
> > + at r3 exists@
> > +identifier x;
> > +position p1;
> > +@@
> > +
> > +(
> > +x = atomic_add_return at p1(-1, ...);
> > +|
> > +x = atomic_long_add_return at p1(-1, ...);
> > +|
> > +x = atomic64_add_return at p1(-1, ...);
> > +)
> > +
> > + at script:python depends on report@
> > +p1 << r3.p1;
> > +@@
> > +msg = "x = atomic_add_return(-1, ...)"
> > +coccilib.report.print_report(p1[0], msg)
> > +
> > +
> > --
> > 2.7.4
> >
> >

[Cocci] add atomic_as_refcounter script

[SF Markus Elfring]

Dear Elena,

I have also taken another look at your script.

The SmPL rule ?r1? specifies six metavariables for identifiers. It seems that
the names ?fname3? till ?fname6? do not care for different function parameters
in the SmPL disjunction at the end.
How do you think about to reduce this disjunction by using a regular expression
with an alternation like ?.*(?:call_rcu|del|queue_work|schedule_work).*?
for a constraint?

Will it help to refactor another rule a bit?

@r3 exists@
identifier x;
position p1;
@@
 x =
(
 atomic_add_return at p1(-1, ...);
|
 atomic_long_add_return at p1(-1, ...);
|
 atomic64_add_return at p1(-1, ...);
)


Regards,
Markus