coccinelle: catchup on memory allocation functions

Since commit 3ef0e5ba4673 ('slab: introduce kzfree()'), kfree() is no more the only function to be considered.

Cc Johannes Weiner <[email protected]>

Signed-off-by Yann Droneaud <[email protected]>

---
 scripts/coccinelle/free/devm_free.cocci | 2 ++
 scripts/coccinelle/free/kfree.cocci     | 3 +++
 scripts/coccinelle/free/kfreeaddr.cocci | 6 +++++-
 3 files changed, 10 insertions(+), 1 deletion(-)

scripts/coccinelle/free/devm_free.cocci +2 -0

index 3d9349012bb3..83c03adec1c5 100644
@@ -48,6 +48,8 @@ position p;
 (
 * kfree at p(x)
 |
+* kzfree at p(x)
+|
 * free_irq at p(x)
 |
 * iounmap at p(x)

scripts/coccinelle/free/kfree.cocci +0 -0

index 577b78056990..c3246b47f51f 100644

scripts/coccinelle/free/kfree.cocci +2 -0

 expression E;
 position p1;
 @@

@@ -54,6 +55,7 @@ position p;
  sizeof(<+...E@p...+>)
 
 @loop exists@
+identifier kfree =~ "kz?free";
 expression E;
 identifier l;
 position ok;

@@ -67,6 +69,7 @@ while (1) { ...
 }
 
 @r exists@
+identifier kfree =~ "kz?free";
 expression free.E, subE<=free.E, E2;
 expression E1;
 iterator iter;

scripts/coccinelle/free/kfreeaddr.cocci +5 -2

index ce8aacc314cb..d46063b1db8b 100644
@@ -16,7 +16,11 @@ identifier f;
 position p;
 @@
 
+(
 * kfree at p(&e->f)
+|
+* kzfree at p(&e->f)
+)
 
 @script:python depends on org@
 p << r.p;

@@ -28,5 +32,5 @@ cocci.print_main("kfree",p)
 p << r.p;
 @@
 
-msg = "ERROR: kfree of structure field"
+msg = "ERROR: invalid free of structure field"
 coccilib.report.print_report(p[0],msg)
--

2.5.0

Quoted 2 lines

@free@
+identifier kfree =~ "kz?free";

Thanks for the suggestions. However, the regular expression is not such a good idea.

Coccinelle doesn't make any optimizations based on regulat expressions.

It would be better to put a disjunction with kfree and kzfree explicitly, as in the other cases.

Quoted 5 lines

Quoted 2 lines

@free@
+identifier kfree =~ "kz?free";

Thanks for the suggestions. However, the regular expression is not such a good idea.

How much is such a SmPL constraint still usable then?

Quoted 1 line

Coccinelle doesn't make any optimizations based on regulat expressions.

Where can your software optimise the source code search?

Quoted 4 lines

It would be better to put a disjunction with kfree and kzfree explicitly, as in the other cases.

What are the circumstances for corresponding benefits?

Regards,
Markus

Quoted 3 lines

Quoted 1 line

Coccinelle doesn't make any optimizations based on regulat expressions.

Where can your software optimise the source code search?

When the name appears explicitly in the matching code, Coccinelle will parse and process only files that contain that name.

Since commit 3ef0e5ba4673 ('slab: introduce kzfree()'), kfree() is no more the only function to be considered.

In particular, kzfree() must not be called on memory allocated through devm_*() functions.

Cc Johannes Weiner <[email protected]>

Signed-off-by Yann Droneaud <[email protected]>

---
Hi Julia,

As you suggested, I've use disjunctions instead of regular expressions (which I managed to use incorrectly: eg. without ^...$ they catch other functions than kfree(), such as kfree_skb()).

I've think we should also catch krealloc(, size), where size is 0, but it's beyond my understanding of coccinelle if size is not a plain 0 constant.

Perhaps you could help me for this one.

Regards.

 scripts/coccinelle/free/devm_free.cocci |  2 ++
 scripts/coccinelle/free/kfree.cocci     | 18 +++++++++++++++---
 scripts/coccinelle/free/kfreeaddr.cocci |  6 +++++-
 3 files changed, 22 insertions(+), 4 deletions(-)

scripts/coccinelle/free/devm_free.cocci +2 -0

index 3d9349012bb3..83c03adec1c5 100644
@@ -48,6 +48,8 @@ position p;
 (
 * kfree at p(x)
 |
+* kzfree at p(x)
+|
 * free_irq at p(x)
 |
 * iounmap at p(x)

scripts/coccinelle/free/kfree.cocci +15 -3

index 577b78056990..ac438da4fd7b 100644
@@ -20,7 +20,11 @@ expression E;
 position p1;
 @@
 
-kfree at p1(E)
+(
+* kfree at p1(E)
+|
+* kzfree at p1(E)
+)
 
 @print expression@
 constant char [] c;

@@ -60,7 +64,11 @@ position ok;
 @@
 
 while (1) { ...
-  kfree at ok(E)
+(
+* kfree at ok(E)
+|
+* kzfree at ok(E)
+)
   ... when != break;
       when != goto l;
       when forall

@@ -74,7 +82,11 @@ statement S;
 position free.p1!=loop.ok,p2!={print.p,sz.p};
 @@
 
-kfree at p1(E,...)
+(
+* kfree at p1(E,...)
+|
+* kzfree at p1(E,...)
+)
...
 (
  iter(...,subE,...) S // no use

scripts/coccinelle/free/kfreeaddr.cocci +5 -2

index ce8aacc314cb..d46063b1db8b 100644
@@ -16,7 +16,11 @@ identifier f;
 position p;
 @@
 
+(
 * kfree at p(&e->f)
+|
+* kzfree at p(&e->f)
+)
 
 @script:python depends on org@
 p << r.p;

@@ -28,5 +32,5 @@ cocci.print_main("kfree",p)
 p << r.p;
 @@
 
-msg = "ERROR: kfree of structure field"
+msg = "ERROR: invalid free of structure field"
 coccilib.report.print_report(p[0],msg)
--

2.5.0

Quoted 17 lines

Since commit 3ef0e5ba4673 ('slab: introduce kzfree()'), kfree() is no more the only function to be considered.

In particular, kzfree() must not be called on memory allocated through devm_*() functions.

Cc Johannes Weiner <[email protected]>

Signed-off-by Yann Droneaud <[email protected]>

---
Hi Julia,

As you suggested, I've use disjunctions instead of regular expressions (which I managed to use incorrectly: eg. without ^...$ they catch other functions than kfree(), such as kfree_skb()).

Quoted 5 lines

I've think we should also catch krealloc(, size), where size is 0, but it's beyond my understanding of coccinelle if size is not a plain 0 constant.

Perhaps you could help me for this one.

Do you have some examples?

? Coccinelle is not very good at tracking
values.? You can say something like:

size = 0
... when != size = e
krealloc(...,size)

I don't know if that would be useful in practice though.

Review

Quoted 8 lines

Regards.

? scripts/coccinelle/free/devm_free.cocci |? 2 ++
? scripts/coccinelle/free/kfree.cocci???? | 18 +++++++++++++++---
? scripts/coccinelle/free/kfreeaddr.cocci |? 6 +++++-

? 3 files changed, 22 insertions(+), 4 deletions(-)

(quoted diff) +0 -0

b/scripts/coccinelle/free/devm_free.cocci

Quoted 12 lines

index 3d9349012bb3..83c03adec1c5 100644

scripts/coccinelle/free/devm_free.cocci +0 -0

@@ -48,6 +48,8 @@ position p;

? (
? * kfree at p(x)
? |
+* kzfree at p(x)
+|
? * free_irq at p(x)
? |
? * iounmap at p(x)

Review

Quoted 1 line

(quoted diff) +0 -0

b/scripts/coccinelle/free/kfree.cocci

Quoted 42 lines

index 577b78056990..ac438da4fd7b 100644

scripts/coccinelle/free/kfree.cocci +0 -0

@@ -20,7 +20,11 @@ expression E;
 ? position p1;

? @@

scripts/coccinelle/free/kfree.cocci +5 -1

-kfree at p1(E)
+(
+* kfree at p1(E)
+|
+* kzfree at p1(E)
+)

? @print expression@
? constant char [] c;

scripts/coccinelle/free/kfree.cocci +0 -0

@@ -60,7 +64,11 @@ position ok;

? @@

? while (1) { ...
-? kfree at ok(E)

scripts/coccinelle/free/kfree.cocci +5 -0

+(
+* kfree at ok(E)
+|
+* kzfree at ok(E)
+)

??? ... when != break;
??????? when != goto l;
??????? when forall

scripts/coccinelle/free/kfree.cocci +0 -0

@@ -74,7 +82,11 @@ statement S;
 ? position free.p1!=loop.ok,p2!={print.p,sz.p};

? @@

scripts/coccinelle/free/kfree.cocci +5 -1

-kfree at p1(E,...)
+(
+* kfree at p1(E,...)
+|
+* kzfree at p1(E,...)
+)

? ...
? (
?? iter(...,subE,...) S // no use

Review

Quoted 1 line

(quoted diff) +0 -0

b/scripts/coccinelle/free/kfreeaddr.cocci

Quoted 19 lines

index ce8aacc314cb..d46063b1db8b 100644

scripts/coccinelle/free/kfreeaddr.cocci +0 -0

@@ -16,7 +16,11 @@ identifier f;
 ? position p;

? @@

+(
? * kfree at p(&e->f)
+|

scripts/coccinelle/free/kfreeaddr.cocci +2 -0

+* kzfree at p(&e->f)
+)

? @script:python depends on org@
? p << r.p;

scripts/coccinelle/free/kfreeaddr.cocci +0 -0

@@ -28,5 +32,5 @@ cocci.print_main("kfree",p)
 ? p << r.p;

? @@

Quoted 7 lines

scripts/coccinelle/free/kfreeaddr.cocci +1 -1

-msg = "ERROR: kfree of structure field"
+msg = "ERROR: invalid free of structure field"

? coccilib.report.print_report(p[0],msg)
--
2.5.0

Updates free/devm_free.cocci to recognize functions added by:

- commit 64c862a839a8 ('devres: add kernel standard devm_k.alloc functions')
- commit e31108cad3de ('devres: introduce API "devm_kstrdup"')
- commit 3046365bb470 ('devres: introduce API "devm_kmemdup')
- commit 43339bed7010 ('devres: Add devm_get_free_pages API')
- commit 75f2a4ead5d5 ('devres: Add devm_kasprintf and devm_kvasprintf API')

See also Documentation/driver-model/devres.txt

Cc Joe Perches <[email protected]>

Cc Manish Badarkhe <[email protected]>

Cc Srinivas Pandruvada <[email protected]>

Cc Eli Billauer <[email protected]>

Cc Himangi Saraogi <[email protected]>

Cc Geert Uytterhoeven <[email protected]>

Cc Wolfram Sang <[email protected]>

Cc Daniel Thompson <[email protected]>

Signed-off-by Yann Droneaud <[email protected]>

---
 scripts/coccinelle/free/devm_free.cocci | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

scripts/coccinelle/free/devm_free.cocci +20 -1

index 83c03adec1c5..3794cd97494b 100644
@@ -29,8 +29,24 @@ expression x;
 @@
 
 (
+ x = devm_kmalloc(...)
+|
+ x = devm_kvasprintf(...)
+|
+ x = devm_kasprintf(...)
+|
  x = devm_kzalloc(...)
 |
+ x = devm_kmalloc_array(...)
+|
+ x = devm_kcalloc(...)
+|
+ x = devm_kstrdup(...)
+|
+ x = devm_kmemdup(...)
+|
+ x = devm_get_free_pages(...)
+|
  x = devm_request_irq(...)
 |
  x = devm_ioremap(...)

@@ -50,6 +66,10 @@ position p;
 |
 * kzfree at p(x)
 |
+* free_pages at p(x, ...)
+|
+* free_page at p(x)
+|
 * free_irq at p(x)
 |
 * iounmap at p(x)
--

2.5.0

krealloc() must not be used against devm_*() allocated memory regions:

- if a bigger memory is to be allocated, krealloc() and __krealloc() could return a different pointer than the one given to them, creating a memory region which is not managed, thus it will not be automatically released on device removal.

- if a bigger memory is to be allocated, krealloc() could kfree() the managed memory region which is passed to it. The old pointer is left registered as a resource for the device. On device removal, this dangling pointer will be used and an unrelated memory region could be released.

- if the requested size is equal to 0, krealloc() can also just behave like kfree(). Here too, the old pointer is kept associated with the device. On device removal, this invalid pointer will be used and an unrelated memory region could be released.

For all these reasons, krealloc() must not be used on a pointer returned by devm_*() functions.

Cc Tejun Heo <[email protected]>

Cc Pekka Enberg <[email protected]>

Signed-off-by Yann Droneaud <[email protected]>

---
 scripts/coccinelle/free/devm_free.cocci | 4 ++++
 1 file changed, 4 insertions(+)

scripts/coccinelle/free/devm_free.cocci +4 -1

index 3794cd97494b..c990d2c7ee16 100644
@@ -66,6 +66,10 @@ position p;
 |
 * kzfree at p(x)
 |
+* __krealloc at p(x, ...)
+|
+* krealloc at p(x, ...)
+|
 * free_pages at p(x, ...)
 |
 * free_page at p(x)
--

2.5.0