[PATCH v2 0/3] coccinelle: catchup on memory allocation functions

[PATCH v2 0/3] coccinelle: catchup on memory allocation functions

[Yann Droneaud]

Hi,

Please find updated patches that remove coccicheck's
blindness regarding the following functions:
- kzfree()
- krealloc()
- __krealloc()
- devm_kmalloc()
- devm_kvasprintf()
- devm_kasprintf()
- devm_kmalloc_array()
- devm_kcalloc()
- devm_kstrdup()
- devm_kmemdup()
- devm_get_free_pages()
- free_pages()
- free_page()

Changes since v1 [1]:
- add kzfree() to ifnullfree.cocci
- added acked-by

 (note: v1 was a single patch update to v0)

Changes since v0 [2]:
- don't use regular expression in kfree.cocci

[1] http://lkml.kernel.org/r/1456150168-24028-1-git-send-email-ydroneaud@opteya.com
[2] http://lkml.kernel.org/r/cover.1455638829.git.ydroneaud@opteya.com

Regards.

Yann Droneaud (3):
  coccinelle: also catch kzfree() issues
  coccinelle: recognize more devm_* memory allocation functions
  coccinelle: catch krealloc() on devm_*() allocated memory

 scripts/coccinelle/free/devm_free.cocci  | 26 ++++++++++++++++++++++++++
 scripts/coccinelle/free/ifnullfree.cocci |  4 +++-
 scripts/coccinelle/free/kfree.cocci      | 18 +++++++++++++++---
 scripts/coccinelle/free/kfreeaddr.cocci  |  6 +++++-
 4 files changed, 49 insertions(+), 5 deletions(-)

-- 
2.7.4

[PATCH v2 1/3] coccinelle: also catch kzfree() issues

[Yann Droneaud]

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

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

Cc: Johannes Weiner <hannes@cmpxchg.org>
Signed-off-by: Yann Droneaud <ydroneaud@opteya.com>
---
 scripts/coccinelle/free/devm_free.cocci  |  2 ++
 scripts/coccinelle/free/ifnullfree.cocci |  4 +++-
 scripts/coccinelle/free/kfree.cocci      | 18 +++++++++++++++---
 scripts/coccinelle/free/kfreeaddr.cocci  |  6 +++++-
 4 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/scripts/coccinelle/free/devm_free.cocci b/scripts/coccinelle/free/devm_free.cocci
index 3d9349012bb3..83c03adec1c5 100644
--- a/scripts/coccinelle/free/devm_free.cocci
+++ b/scripts/coccinelle/free/devm_free.cocci
@@ -48,6 +48,8 @@ position p;
 (
 * kfree@p(x)
 |
+* kzfree@p(x)
+|
 * free_irq@p(x)
 |
 * iounmap@p(x)
diff --git a/scripts/coccinelle/free/ifnullfree.cocci b/scripts/coccinelle/free/ifnullfree.cocci
index 52bd235286fa..14a4cd98e83b 100644
--- a/scripts/coccinelle/free/ifnullfree.cocci
+++ b/scripts/coccinelle/free/ifnullfree.cocci
@@ -20,6 +20,8 @@ expression E;
 (
   kfree(E);
 |
+  kzfree(E);
+|
   debugfs_remove(E);
 |
   debugfs_remove_recursive(E);
@@ -39,7 +41,7 @@ position p;
 @@
 
 * if (E != NULL)
-*	\(kfree@p\|debugfs_remove@p\|debugfs_remove_recursive@p\|
+*	\(kfree@p\|kzfree@p\|debugfs_remove@p\|debugfs_remove_recursive@p\|
 *         usb_free_urb@p\|kmem_cache_destroy@p\|mempool_destroy@p\|
 *         dma_pool_destroy@p\)(E);
 
diff --git a/scripts/coccinelle/free/kfree.cocci b/scripts/coccinelle/free/kfree.cocci
index 577b78056990..ac438da4fd7b 100644
--- a/scripts/coccinelle/free/kfree.cocci
+++ b/scripts/coccinelle/free/kfree.cocci
@@ -20,7 +20,11 @@ expression E;
 position p1;
 @@
 
-kfree@p1(E)
+(
+* kfree@p1(E)
+|
+* kzfree@p1(E)
+)
 
 @print expression@
 constant char [] c;
@@ -60,7 +64,11 @@ position ok;
 @@
 
 while (1) { ...
-  kfree@ok(E)
+(
+* kfree@ok(E)
+|
+* kzfree@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@p1(E,...)
+(
+* kfree@p1(E,...)
+|
+* kzfree@p1(E,...)
+)
 ...
 (
  iter(...,subE,...) S // no use
diff --git a/scripts/coccinelle/free/kfreeaddr.cocci b/scripts/coccinelle/free/kfreeaddr.cocci
index ce8aacc314cb..d46063b1db8b 100644
--- a/scripts/coccinelle/free/kfreeaddr.cocci
+++ b/scripts/coccinelle/free/kfreeaddr.cocci
@@ -16,7 +16,11 @@ identifier f;
 position p;
 @@
 
+(
 * kfree@p(&e->f)
+|
+* kzfree@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.7.4

Re: [PATCH v2 1/3] coccinelle: also catch kzfree() issues

[Julia Lawall]

On Mon, 23 May 2016, Yann Droneaud wrote:

> Since commit 3ef0e5ba4673 ('slab: introduce kzfree()'),
> kfree() is no more the only function to be considered:
> kzfree() should be recognized too.
>
> In particular, kzfree() must not be called on memory
> allocated through devm_*() functions.
>
> Cc: Johannes Weiner <hannes@cmpxchg.org>
> Signed-off-by: Yann Droneaud <ydroneaud@opteya.com>

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

> ---
>  scripts/coccinelle/free/devm_free.cocci  |  2 ++
>  scripts/coccinelle/free/ifnullfree.cocci |  4 +++-
>  scripts/coccinelle/free/kfree.cocci      | 18 +++++++++++++++---
>  scripts/coccinelle/free/kfreeaddr.cocci  |  6 +++++-
>  4 files changed, 25 insertions(+), 5 deletions(-)
>
> diff --git a/scripts/coccinelle/free/devm_free.cocci b/scripts/coccinelle/free/devm_free.cocci
> index 3d9349012bb3..83c03adec1c5 100644
> --- a/scripts/coccinelle/free/devm_free.cocci
> +++ b/scripts/coccinelle/free/devm_free.cocci
> @@ -48,6 +48,8 @@ position p;
>  (
>  * kfree@p(x)
>  |
> +* kzfree@p(x)
> +|
>  * free_irq@p(x)
>  |
>  * iounmap@p(x)
> diff --git a/scripts/coccinelle/free/ifnullfree.cocci b/scripts/coccinelle/free/ifnullfree.cocci
> index 52bd235286fa..14a4cd98e83b 100644
> --- a/scripts/coccinelle/free/ifnullfree.cocci
> +++ b/scripts/coccinelle/free/ifnullfree.cocci
> @@ -20,6 +20,8 @@ expression E;
>  (
>    kfree(E);
>  |
> +  kzfree(E);
> +|
>    debugfs_remove(E);
>  |
>    debugfs_remove_recursive(E);
> @@ -39,7 +41,7 @@ position p;
>  @@
>
>  * if (E != NULL)
> -*	\(kfree@p\|debugfs_remove@p\|debugfs_remove_recursive@p\|
> +*	\(kfree@p\|kzfree@p\|debugfs_remove@p\|debugfs_remove_recursive@p\|
>  *         usb_free_urb@p\|kmem_cache_destroy@p\|mempool_destroy@p\|
>  *         dma_pool_destroy@p\)(E);
>
> diff --git a/scripts/coccinelle/free/kfree.cocci b/scripts/coccinelle/free/kfree.cocci
> index 577b78056990..ac438da4fd7b 100644
> --- a/scripts/coccinelle/free/kfree.cocci
> +++ b/scripts/coccinelle/free/kfree.cocci
> @@ -20,7 +20,11 @@ expression E;
>  position p1;
>  @@
>
> -kfree@p1(E)
> +(
> +* kfree@p1(E)
> +|
> +* kzfree@p1(E)
> +)
>
>  @print expression@
>  constant char [] c;
> @@ -60,7 +64,11 @@ position ok;
>  @@
>
>  while (1) { ...
> -  kfree@ok(E)
> +(
> +* kfree@ok(E)
> +|
> +* kzfree@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@p1(E,...)
> +(
> +* kfree@p1(E,...)
> +|
> +* kzfree@p1(E,...)
> +)
>  ...
>  (
>   iter(...,subE,...) S // no use
> diff --git a/scripts/coccinelle/free/kfreeaddr.cocci b/scripts/coccinelle/free/kfreeaddr.cocci
> index ce8aacc314cb..d46063b1db8b 100644
> --- a/scripts/coccinelle/free/kfreeaddr.cocci
> +++ b/scripts/coccinelle/free/kfreeaddr.cocci
> @@ -16,7 +16,11 @@ identifier f;
>  position p;
>  @@
>
> +(
>  * kfree@p(&e->f)
> +|
> +* kzfree@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.7.4
>
>

Re: [PATCH v2 1/3] coccinelle: also catch kzfree() issues

[Michal Marek]

On 2016-05-23 17:18, Julia Lawall wrote:
> 
> 
> On Mon, 23 May 2016, Yann Droneaud wrote:
> 
>> Since commit 3ef0e5ba4673 ('slab: introduce kzfree()'),
>> kfree() is no more the only function to be considered:
>> kzfree() should be recognized too.
>>
>> In particular, kzfree() must not be called on memory
>> allocated through devm_*() functions.
>>
>> Cc: Johannes Weiner <hannes@cmpxchg.org>
>> Signed-off-by: Yann Droneaud <ydroneaud@opteya.com>
> 
> Acked-by: Julia Lawall <julia.lawall@lip6.fr>

Hi Julia,

does your ACK apply to the other two patches as well?

Thanks,
Michal

Re: [PATCH v2 1/3] coccinelle: also catch kzfree() issues

[Julia Lawall]

On Mon, 20 Jun 2016, Michal Marek wrote:

> On 2016-05-23 17:18, Julia Lawall wrote:
> > 
> > 
> > On Mon, 23 May 2016, Yann Droneaud wrote:
> > 
> >> Since commit 3ef0e5ba4673 ('slab: introduce kzfree()'),
> >> kfree() is no more the only function to be considered:
> >> kzfree() should be recognized too.
> >>
> >> In particular, kzfree() must not be called on memory
> >> allocated through devm_*() functions.
> >>
> >> Cc: Johannes Weiner <hannes@cmpxchg.org>
> >> Signed-off-by: Yann Droneaud <ydroneaud@opteya.com>
> > 
> > Acked-by: Julia Lawall <julia.lawall@lip6.fr>
> 
> Hi Julia,
> 
> does your ACK apply to the other two patches as well?

Sorry, I seem to have missed the other two.  I have reviewed them now, and 
the ack applies to all three.  Thanks for checking on it.

julia

Re: [PATCH v2 1/3] coccinelle: also catch kzfree() issues

[Michal Marek]

Dne 20.6.2016 v 22:21 Julia Lawall napsal(a):
> 
> 
> On Mon, 20 Jun 2016, Michal Marek wrote:
> 
>> On 2016-05-23 17:18, Julia Lawall wrote:
>>>
>>>
>>> On Mon, 23 May 2016, Yann Droneaud wrote:
>>>
>>>> Since commit 3ef0e5ba4673 ('slab: introduce kzfree()'),
>>>> kfree() is no more the only function to be considered:
>>>> kzfree() should be recognized too.
>>>>
>>>> In particular, kzfree() must not be called on memory
>>>> allocated through devm_*() functions.
>>>>
>>>> Cc: Johannes Weiner <hannes@cmpxchg.org>
>>>> Signed-off-by: Yann Droneaud <ydroneaud@opteya.com>
>>>
>>> Acked-by: Julia Lawall <julia.lawall@lip6.fr>
>>
>> Hi Julia,
>>
>> does your ACK apply to the other two patches as well?
> 
> Sorry, I seem to have missed the other two.  I have reviewed them now, and 
> the ack applies to all three.  Thanks for checking on it.

Np. Applied to kbuild.git#misc now.

Michal

Re: [PATCH v2 1/3] coccinelle: also catch kzfree() issues

[Yann Droneaud]

Hi,

Le mardi 21 juin 2016 à 11:43 +0200, Michal Marek a écrit :
> Dne 20.6.2016 v 22:21 Julia Lawall napsal(a):
> > On Mon, 20 Jun 2016, Michal Marek wrote:
> > On 2016-05-23 17:18, Julia Lawall wrote:
> > > > On Mon, 23 May 2016, Yann Droneaud wrote:
> > > > 
> > > > > Since commit 3ef0e5ba4673 ('slab: introduce kzfree()'),
> > > > > kfree() is no more the only function to be considered:
> > > > > kzfree() should be recognized too.
> > > > > 
> > > > > In particular, kzfree() must not be called on memory
> > > > > allocated through devm_*() functions.
> > > > > 
> > > > > Cc: Johannes Weiner <hannes@cmpxchg.org>
> > > > > Signed-off-by: Yann Droneaud <ydroneaud@opteya.com>
> > > > 
> > > > Acked-by: Julia Lawall <julia.lawall@lip6.fr>
> > > 
> > > Hi Julia,
> > > 
> > > does your ACK apply to the other two patches as well?
> > 
> > Sorry, I seem to have missed the other two.  I have reviewed them
> > now, and the ack applies to all three.  Thanks for checking on it.
> 

Back in February, those two other patches were already Acked-by:

http://lkml.kernel.org/r/alpine.DEB.2.10.1602161818100.2685@hadrien
http://lkml.kernel.org/r/alpine.DEB.2.10.1602161819340.2685@hadrien

I've (re)sent them with added Acked-by:, and thought it would not
require further Acked-by:.

Anyway, this one was new and required proper review. Thanks a lot.

> Np. Applied to kbuild.git#misc now.
> 

Thanks a lot.

-- 
Yann Droneaud
OPTEYA

Re: [PATCH v2 1/3] coccinelle: also catch kzfree() issues

[Julia Lawall]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1567 bytes --]



On Tue, 21 Jun 2016, Yann Droneaud wrote:

> Hi,
>
> Le mardi 21 juin 2016 à 11:43 +0200, Michal Marek a écrit :
> > Dne 20.6.2016 v 22:21 Julia Lawall napsal(a):
> > > On Mon, 20 Jun 2016, Michal Marek wrote:
> > > On 2016-05-23 17:18, Julia Lawall wrote:
> > > > > On Mon, 23 May 2016, Yann Droneaud wrote:
> > > > >
> > > > > > Since commit 3ef0e5ba4673 ('slab: introduce kzfree()'),
> > > > > > kfree() is no more the only function to be considered:
> > > > > > kzfree() should be recognized too.
> > > > > >
> > > > > > In particular, kzfree() must not be called on memory
> > > > > > allocated through devm_*() functions.
> > > > > >
> > > > > > Cc: Johannes Weiner <hannes@cmpxchg.org>
> > > > > > Signed-off-by: Yann Droneaud <ydroneaud@opteya.com>
> > > > >
> > > > > Acked-by: Julia Lawall <julia.lawall@lip6.fr>
> > > >
> > > > Hi Julia,
> > > >
> > > > does your ACK apply to the other two patches as well?
> > >
> > > Sorry, I seem to have missed the other two.  I have reviewed them
> > > now, and the ack applies to all three.  Thanks for checking on it.
> >
>
> Back in February, those two other patches were already Acked-by:
>
> http://lkml.kernel.org/r/alpine.DEB.2.10.1602161818100.2685@hadrien
> http://lkml.kernel.org/r/alpine.DEB.2.10.1602161819340.2685@hadrien
>
> I've (re)sent them with added Acked-by:, and thought it would not
> require further Acked-by:.
>
> Anyway, this one was new and required proper review. Thanks a lot.

Thanks for the reminder.  Since they are comitted now, things are going in
the right direction.

julia

[PATCH v2 2/3] coccinelle: recognize more devm_* memory allocation functions

[Yann Droneaud]

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 <joe@perches.com>
Cc: Manish Badarkhe <badarkhe.manish@gmail.com>
Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Cc: Eli Billauer <eli.billauer@gmail.com>
Cc: Himangi Saraogi <himangi774@gmail.com>
Cc: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: Wolfram Sang <w.sang@pengutronix.de>
Cc: Daniel Thompson <daniel.thompson@linaro.org>
Acked-by: Julia Lawall <julia.lawall@lip6.fr>
Signed-off-by: Yann Droneaud <ydroneaud@opteya.com>
---
 scripts/coccinelle/free/devm_free.cocci | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/scripts/coccinelle/free/devm_free.cocci b/scripts/coccinelle/free/devm_free.cocci
index 83c03adec1c5..3794cd97494b 100644
--- a/scripts/coccinelle/free/devm_free.cocci
+++ b/scripts/coccinelle/free/devm_free.cocci
@@ -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@p(x)
 |
+* free_pages@p(x, ...)
+|
+* free_page@p(x)
+|
 * free_irq@p(x)
 |
 * iounmap@p(x)
-- 
2.7.4

[PATCH v2 3/3] coccinelle: catch krealloc() on devm_*() allocated memory

[Yann Droneaud]

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 <tj@kernel.org>
Cc: Pekka Enberg <penberg@cs.helsinki.fi>
Acked-by: Julia Lawall <julia.lawall@lip6.fr>
Signed-off-by: Yann Droneaud <ydroneaud@opteya.com>
---
 scripts/coccinelle/free/devm_free.cocci | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/scripts/coccinelle/free/devm_free.cocci b/scripts/coccinelle/free/devm_free.cocci
index 3794cd97494b..c990d2c7ee16 100644
--- a/scripts/coccinelle/free/devm_free.cocci
+++ b/scripts/coccinelle/free/devm_free.cocci
@@ -66,6 +66,10 @@ position p;
 |
 * kzfree@p(x)
 |
+* __krealloc@p(x, ...)
+|
+* krealloc@p(x, ...)
+|
 * free_pages@p(x, ...)
 |
 * free_page@p(x)
-- 
2.7.4