* [PATCH] Coccinelle report script for refcounters @ 2017-07-18 7:48 ` Elena Reshetova 0 siblings, 0 replies; 23+ messages in thread From: Elena Reshetova @ 2017-07-18 7:48 UTC (permalink / raw) To: julia.lawall Cc: linux-kernel, cocci, Gilles.Muller, nicolas.palix, mmarek, keescook, ishkamiel, 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 ^ permalink raw reply [flat|nested] 23+ messages in thread
* [Cocci] [PATCH] Coccinelle report script for refcounters @ 2017-07-18 7:48 ` Elena Reshetova 0 siblings, 0 replies; 23+ messages in thread From: Elena Reshetova @ 2017-07-18 7:48 UTC (permalink / raw) To: cocci 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 ^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH] Coccinelle: add atomic_as_refcounter script 2017-07-18 7:48 ` [Cocci] " Elena Reshetova @ 2017-07-18 7:48 ` Elena Reshetova -1 siblings, 0 replies; 23+ messages in thread From: Elena Reshetova @ 2017-07-18 7:48 UTC (permalink / raw) To: julia.lawall Cc: linux-kernel, cocci, Gilles.Muller, nicolas.palix, mmarek, keescook, ishkamiel, 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 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* [Cocci] [PATCH] Coccinelle: add atomic_as_refcounter script @ 2017-07-18 7:48 ` Elena Reshetova 0 siblings, 0 replies; 23+ messages in thread From: Elena Reshetova @ 2017-07-18 7:48 UTC (permalink / raw) To: cocci 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 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH] Coccinelle: add atomic_as_refcounter script 2017-07-18 7:48 ` [Cocci] " Elena Reshetova @ 2017-07-18 16:21 ` Kees Cook -1 siblings, 0 replies; 23+ messages in thread From: Kees Cook @ 2017-07-18 16:21 UTC (permalink / raw) To: Elena Reshetova Cc: Julia Lawall, LKML, cocci, Gilles Muller, Nicolas Palix, Michal Marek, Hans Liljestrand 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 ^ permalink raw reply [flat|nested] 23+ messages in thread
* [Cocci] [PATCH] Coccinelle: add atomic_as_refcounter script @ 2017-07-18 16:21 ` Kees Cook 0 siblings, 0 replies; 23+ messages in thread From: Kees Cook @ 2017-07-18 16:21 UTC (permalink / raw) To: cocci 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 ^ permalink raw reply [flat|nested] 23+ messages in thread
* RE: [PATCH] Coccinelle: add atomic_as_refcounter script 2017-07-18 16:21 ` [Cocci] " Kees Cook @ 2017-07-19 10:54 ` Reshetova, Elena -1 siblings, 0 replies; 23+ messages in thread From: Reshetova, Elena @ 2017-07-19 10:54 UTC (permalink / raw) To: Kees Cook Cc: Julia Lawall, LKML, cocci, Gilles Muller, Nicolas Palix, Michal Marek, Hans Liljestrand 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 ^ permalink raw reply [flat|nested] 23+ messages in thread
* [Cocci] [PATCH] Coccinelle: add atomic_as_refcounter script @ 2017-07-19 10:54 ` Reshetova, Elena 0 siblings, 0 replies; 23+ messages in thread From: Reshetova, Elena @ 2017-07-19 10:54 UTC (permalink / raw) To: cocci 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 ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] Coccinelle: add atomic_as_refcounter script 2017-07-18 7:48 ` [Cocci] " Elena Reshetova @ 2017-08-04 15:23 ` Julia Lawall -1 siblings, 0 replies; 23+ messages in thread From: Julia Lawall @ 2017-08-04 15:23 UTC (permalink / raw) To: Elena Reshetova Cc: linux-kernel, cocci, Gilles Muller, nicolas.palix, mmarek, keescook, ishkamiel 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 > > ^ permalink raw reply [flat|nested] 23+ messages in thread
* [Cocci] [PATCH] Coccinelle: add atomic_as_refcounter script @ 2017-08-04 15:23 ` Julia Lawall 0 siblings, 0 replies; 23+ messages in thread From: Julia Lawall @ 2017-08-04 15:23 UTC (permalink / raw) To: cocci 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 > > ^ permalink raw reply [flat|nested] 23+ messages in thread
* RE: [PATCH] Coccinelle: add atomic_as_refcounter script 2017-08-04 15:23 ` [Cocci] " Julia Lawall @ 2017-08-07 11:06 ` Reshetova, Elena -1 siblings, 0 replies; 23+ messages in thread From: Reshetova, Elena @ 2017-08-07 11:06 UTC (permalink / raw) To: Julia Lawall Cc: linux-kernel, cocci, Gilles Muller, nicolas.palix, mmarek, keescook, ishkamiel > 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 > > > > ^ permalink raw reply [flat|nested] 23+ messages in thread
* [Cocci] [PATCH] Coccinelle: add atomic_as_refcounter script @ 2017-08-07 11:06 ` Reshetova, Elena 0 siblings, 0 replies; 23+ messages in thread From: Reshetova, Elena @ 2017-08-07 11:06 UTC (permalink / raw) To: cocci > 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 > > > > ^ permalink raw reply [flat|nested] 23+ messages in thread
* [Cocci] add atomic_as_refcounter script 2017-08-07 11:06 ` [Cocci] " Reshetova, Elena (?) @ 2017-08-08 18:34 ` SF Markus Elfring -1 siblings, 0 replies; 23+ messages in thread From: SF Markus Elfring @ 2017-08-08 18:34 UTC (permalink / raw) To: cocci 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 ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] Coccinelle report script for refcounters 2017-07-18 7:48 ` [Cocci] " Elena Reshetova @ 2017-07-18 8:47 ` Julia Lawall -1 siblings, 0 replies; 23+ messages in thread From: Julia Lawall @ 2017-07-18 8:47 UTC (permalink / raw) To: Elena Reshetova Cc: linux-kernel, cocci, Gilles Muller, nicolas.palix, mmarek, keescook, ishkamiel 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 > > ^ permalink raw reply [flat|nested] 23+ messages in thread
* [Cocci] [PATCH] Coccinelle report script for refcounters @ 2017-07-18 8:47 ` Julia Lawall 0 siblings, 0 replies; 23+ messages in thread From: Julia Lawall @ 2017-07-18 8:47 UTC (permalink / raw) To: cocci 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 > > ^ permalink raw reply [flat|nested] 23+ messages in thread
* RE: [PATCH] Coccinelle report script for refcounters 2017-07-18 8:47 ` [Cocci] " Julia Lawall @ 2017-07-18 9:30 ` Reshetova, Elena -1 siblings, 0 replies; 23+ messages in thread From: Reshetova, Elena @ 2017-07-18 9:30 UTC (permalink / raw) To: Julia Lawall Cc: linux-kernel, cocci, Gilles Muller, nicolas.palix, mmarek, keescook, ishkamiel > 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 > > > > ^ permalink raw reply [flat|nested] 23+ messages in thread
* [Cocci] [PATCH] Coccinelle report script for refcounters @ 2017-07-18 9:30 ` Reshetova, Elena 0 siblings, 0 replies; 23+ messages in thread From: Reshetova, Elena @ 2017-07-18 9:30 UTC (permalink / raw) To: cocci > 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 > > > > ^ permalink raw reply [flat|nested] 23+ messages in thread
* RE: [PATCH] Coccinelle report script for refcounters 2017-07-18 9:30 ` [Cocci] " Reshetova, Elena @ 2017-07-18 11:53 ` Julia Lawall -1 siblings, 0 replies; 23+ messages in thread From: Julia Lawall @ 2017-07-18 11:53 UTC (permalink / raw) To: Reshetova, Elena Cc: linux-kernel, cocci, Gilles Muller, nicolas.palix, mmarek, keescook, ishkamiel 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 > > > > > > > ^ permalink raw reply [flat|nested] 23+ messages in thread
* [Cocci] [PATCH] Coccinelle report script for refcounters @ 2017-07-18 11:53 ` Julia Lawall 0 siblings, 0 replies; 23+ messages in thread From: Julia Lawall @ 2017-07-18 11:53 UTC (permalink / raw) To: cocci 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 > > > > > > > ^ permalink raw reply [flat|nested] 23+ messages in thread
* RE: [PATCH] Coccinelle report script for refcounters 2017-07-18 11:53 ` [Cocci] " Julia Lawall @ 2017-07-18 12:27 ` Reshetova, Elena -1 siblings, 0 replies; 23+ messages in thread From: Reshetova, Elena @ 2017-07-18 12:27 UTC (permalink / raw) To: Julia Lawall Cc: linux-kernel, cocci, Gilles Muller, nicolas.palix, mmarek, keescook, ishkamiel > 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 > > > > > > > > > > ^ permalink raw reply [flat|nested] 23+ messages in thread
* [Cocci] [PATCH] Coccinelle report script for refcounters @ 2017-07-18 12:27 ` Reshetova, Elena 0 siblings, 0 replies; 23+ messages in thread From: Reshetova, Elena @ 2017-07-18 12:27 UTC (permalink / raw) To: cocci > 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 > > > > > > > > > > ^ permalink raw reply [flat|nested] 23+ messages in thread
* RE: [PATCH] Coccinelle report script for refcounters 2017-07-18 12:27 ` [Cocci] " Reshetova, Elena @ 2017-07-18 15:10 ` Julia Lawall -1 siblings, 0 replies; 23+ messages in thread From: Julia Lawall @ 2017-07-18 15:10 UTC (permalink / raw) To: Reshetova, Elena Cc: linux-kernel, cocci, Gilles Muller, nicolas.palix, mmarek, keescook, ishkamiel 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 > > > > > > > > > > > > > > ^ permalink raw reply [flat|nested] 23+ messages in thread
* [Cocci] [PATCH] Coccinelle report script for refcounters @ 2017-07-18 15:10 ` Julia Lawall 0 siblings, 0 replies; 23+ messages in thread From: Julia Lawall @ 2017-07-18 15:10 UTC (permalink / raw) To: cocci 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 > > > > > > > > > > > > > > ^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2017-08-08 18:34 UTC | newest] Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-07-18 7:48 [PATCH] Coccinelle report script for refcounters Elena Reshetova 2017-07-18 7:48 ` [Cocci] " Elena Reshetova 2017-07-18 7:48 ` [PATCH] Coccinelle: add atomic_as_refcounter script Elena Reshetova 2017-07-18 7:48 ` [Cocci] " Elena Reshetova 2017-07-18 16:21 ` Kees Cook 2017-07-18 16:21 ` [Cocci] " Kees Cook 2017-07-19 10:54 ` Reshetova, Elena 2017-07-19 10:54 ` [Cocci] " Reshetova, Elena 2017-08-04 15:23 ` Julia Lawall 2017-08-04 15:23 ` [Cocci] " Julia Lawall 2017-08-07 11:06 ` Reshetova, Elena 2017-08-07 11:06 ` [Cocci] " Reshetova, Elena 2017-08-08 18:34 ` [Cocci] " SF Markus Elfring 2017-07-18 8:47 ` [PATCH] Coccinelle report script for refcounters Julia Lawall 2017-07-18 8:47 ` [Cocci] " Julia Lawall 2017-07-18 9:30 ` Reshetova, Elena 2017-07-18 9:30 ` [Cocci] " Reshetova, Elena 2017-07-18 11:53 ` Julia Lawall 2017-07-18 11:53 ` [Cocci] " Julia Lawall 2017-07-18 12:27 ` Reshetova, Elena 2017-07-18 12:27 ` [Cocci] " Reshetova, Elena 2017-07-18 15:10 ` Julia Lawall 2017-07-18 15:10 ` [Cocci] " Julia Lawall
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.