All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH] coccinelle: api: add kvfree script
  2020-06-06  7:30 ` Markus Elfring
  (?)
@ 2020-06-06 14:04 ` Markus Elfring
  -1 siblings, 0 replies; 34+ messages in thread
From: Markus Elfring @ 2020-06-06 14:04 UTC (permalink / raw)
  To: Denis Efremov, Coccinelle, Gilles Muller, Julia Lawall,
	Masahiro Yamada, Michal Marek, Nicolas Palix
  Cc: linux-kernel, kernel-janitors

> +@choice@
> +expression E, E1;
> +position kok, vok;
> +@@
> +
> +(
> +  if (...) {
> +    ...
> +    E = \(kmalloc@kok\|…\)(...)

Further implementation details from this SmPL script caught my software
development attention.

* Is there a need to add the specification “when any” to the SmPL ellipses
  before such assignment statements?

* A limited search approach was expressed. Will additional source code variations
  become relevant?
  + switch statement
  + if branches with single statements
  + conditional operator


> +@opportunity depends on !patch …@
> +  E = \(kmalloc\|…\)(..., size, ...)
> +  ... when != E = E1
> +      when != size = E1

I wonder that two assignments should be excluded here according to
the same expression metavariable.


+@pkfree depends on patch exists@
…
+- \(kfree\|kvfree\)(E)
++ vfree(E)

Would you like to use a SmPL code variant like the following
at any more places?
(Is it occasionally helpful to increase the change precision?)

+-\(kfree\|kvfree\)
++vfree
+      (E)


Regards,
Markus

^ permalink raw reply	[flat|nested] 34+ messages in thread
* Re: [PATCH] coccinelle: api: add kvfree script
@ 2020-06-06  7:30 ` Markus Elfring
  0 siblings, 0 replies; 34+ messages in thread
From: Markus Elfring @ 2020-06-06  7:30 UTC (permalink / raw)
  To: Denis Efremov, Coccinelle, Gilles Muller, Julia Lawall,
	Masahiro Yamada, Michal Marek, Nicolas Palix
  Cc: kernel-janitors, linux-kernel

> Check that alloc and free types of functions match each other.

Further software development challenges are interesting also for such an use case.


> +/// Check that kvmalloc'ed memory is freed by kfree functions,
> +/// vmalloc'ed by vfree functions and kvmalloc'ed by kvfree
> +/// functions.

* How do you think about to offer a wording suggestion for subjects of
  generated patches?

* Will the presented case distinction trigger further improvements for
  the desired matching?

* Would you like to generalise the safe handling of allocations
  and corresponding release of system resources?


> +// Confidence: High

I suggest to reconsider this information once more.


> +virtual patch
> +virtual report
> +virtual org
> +virtual context

+virtual patch, report, org, context

Is such a SmPL code variant more succinct?


> +@choice@

* Can it be that this SmPL rule is not relevant for all operation modes?

* Will additional dependencies matter?


> +    E = \(kmalloc@kok\|kzalloc@kok\|krealloc@kok\|kcalloc@kok\|kmalloc_node@kok\|kzalloc_node@kok\|kmalloc_array@kok\|kmalloc_array_node@kok\|kcalloc_node@kok\)(...)

I would prefer an other coding style here.

* Items for such SmPL disjunctions can be specified also on multiple lines.

* The semantic patch language supports further means to handle function name lists
  in more convenient ways.
  Would you like to work with customised constraints?


> +msg = "WARNING: kmalloc is used to allocate this memory at line %s" % (k[0].line)
> +coccilib.report.print_report(p[0], msg)

* I propose once more omit the extra variable “msg” at similar places.
  The desired message object can be directly passed as a function parameter.

* I find the diagnostic text insufficient.

* Can the corresponding function category be dynamically determined?


Are you looking for opportunities to avoid unwanted code duplication?

Regards,
Markus

^ permalink raw reply	[flat|nested] 34+ messages in thread
* [PATCH] coccinelle: api: add kvfree script
@ 2020-06-05 20:42 Denis Efremov
  0 siblings, 0 replies; 34+ messages in thread
From: Denis Efremov @ 2020-06-05 20:42 UTC (permalink / raw)
  To: Julia Lawall; +Cc: Denis Efremov, cocci, linux-kernel

Check that alloc and free types of functions match each other.

Signed-off-by: Denis Efremov <efremov@linux.com>
---
List of patches to stable:
- https://lkml.org/lkml/2020/6/1/713
- https://lkml.org/lkml/2020/6/5/200
- https://lkml.org/lkml/2020/6/5/838
- https://lkml.org/lkml/2020/6/5/887

Other patches:
- https://lkml.org/lkml/2020/6/1/701
- https://lkml.org/lkml/2020/6/5/839
- https://lkml.org/lkml/2020/6/5/864
- https://lkml.org/lkml/2020/6/5/865
- https://lkml.org/lkml/2020/6/5/895
- https://lkml.org/lkml/2020/6/5/901

There is a false positive that I can't beat:
fs/btrfs/send.c:1119:11-12: WARNING: kmalloc is used to allocate
this memory at line 1036

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

diff --git a/scripts/coccinelle/api/kvfree.cocci b/scripts/coccinelle/api/kvfree.cocci
new file mode 100644
index 000000000000..e3fa3d0fd2fd
--- /dev/null
+++ b/scripts/coccinelle/api/kvfree.cocci
@@ -0,0 +1,196 @@
+// SPDX-License-Identifier: GPL-2.0-only
+///
+/// Check that kvmalloc'ed memory is freed by kfree functions,
+/// vmalloc'ed by vfree functions and kvmalloc'ed by kvfree
+/// functions.
+///
+// Confidence: High
+// Copyright: (C) 2020 Denis Efremov ISPRAS
+// Options: --no-includes --include-headers
+//
+
+virtual patch
+virtual report
+virtual org
+virtual context
+
+
+@choice@
+expression E, E1;
+position kok, vok;
+@@
+
+(
+  if (...) {
+    ...
+    E = \(kmalloc@kok\|kzalloc@kok\|krealloc@kok\|kcalloc@kok\|kmalloc_node@kok\|kzalloc_node@kok\|kmalloc_array@kok\|kmalloc_array_node@kok\|kcalloc_node@kok\)(...)
+    ...
+  } else {
+    ...
+    E = \(vmalloc@vok\|vzalloc@vok\|vmalloc_user@vok\|vmalloc_node@vok\|vzalloc_node@vok\|vmalloc_exec@vok\|vmalloc_32@vok\|vmalloc_32_user@vok\|__vmalloc@vok\|__vmalloc_node_range@vok\|__vmalloc_node@vok\)(...)
+    ...
+  }
+|
+  E = \(kmalloc\|kzalloc\|krealloc\|kcalloc\|kmalloc_node\|kzalloc_node\|kmalloc_array\|kmalloc_array_node\|kcalloc_node\)(...)
+  ... when != E = E1
+      when any
+  if (\(!E\|E == NULL\)) {
+    ...
+    E = \(vmalloc@vok\|vzalloc@vok\|vmalloc_user@vok\|vmalloc_node@vok\|vzalloc_node@vok\|vmalloc_exec@vok\|vmalloc_32@vok\|vmalloc_32_user@vok\|__vmalloc@vok\|__vmalloc_node_range@vok\|__vmalloc_node@vok\)(...)
+    ...
+  }
+)
+
+// exclude mm/vmalloc.c because of kvmalloc* definitions
+@opportunity depends on !patch && !(file in "mm/vmalloc.c")@
+expression E, E1, size;
+position p;
+@@
+
+(
+* if (\(size <= E1\|size < E1\|size = E1\|size > E1\) || ...)@p {
+    ...
+    E = \(kmalloc\|kzalloc\|krealloc\|kcalloc\|kmalloc_node\|kzalloc_node\|kmalloc_array\|kmalloc_array_node\|kcalloc_node\)(..., size, ...)
+    ...
+  } else {
+    ...
+    E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\|vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\|__vmalloc_node_range\|__vmalloc_node\)(..., size, ...)
+    ...
+  }
+|
+  E = \(kmalloc\|kzalloc\|krealloc\|kcalloc\|kmalloc_node\|kzalloc_node\|kmalloc_array\|kmalloc_array_node\|kcalloc_node\)(..., size, ...)
+  ... when != E = E1
+      when != size = E1
+      when any
+* if (\(!E\|E == NULL\))@p {
+    ...
+    E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\|vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\|__vmalloc_node_range\|__vmalloc_node\)(..., size, ...)
+    ...
+  }
+)
+
+@vfree depends on !patch@
+expression E;
+position k != choice.kok;
+position p;
+@@
+
+* E = \(kmalloc@k\|kzalloc@k\|krealloc@k\|kcalloc@k\|kmalloc_node@k\|kzalloc_node@k\|kmalloc_array@k\|kmalloc_array_node@k\|kcalloc_node@k\)(...)
+  ... when != if (...) { ... E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\|vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\|__vmalloc_node_range\|__vmalloc_node\|kvmalloc\|kvzalloc\|kvcalloc\|kvzalloc_node\|kvmalloc_node\|kvmalloc_array\)(...); ... }
+      when != is_vmalloc_addr(E)
+      when any
+* \(vfree\|vfree_atomic\|kvfree\)(E)@p
+
+@pvfree depends on patch exists@
+expression E;
+position k != choice.kok;
+@@
+
+  E = \(kmalloc@k\|kzalloc@k\|krealloc@k\|kcalloc@k\|kmalloc_node@k\|kzalloc_node@k\|kmalloc_array@k\|kmalloc_array_node@k\|kcalloc_node@k\)(...)
+  ... when != if (...) { ... E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\|vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\|__vmalloc_node_range\|__vmalloc_node\|kvmalloc\|kvzalloc\|kvcalloc\|kvzalloc_node\|kvmalloc_node\|kvmalloc_array\)(...); ... }
+      when != is_vmalloc_addr(E)
+      when any
+- \(vfree\|vfree_atomic\|kvfree\)(E)
++ kfree(E)
+
+@kfree depends on !patch@
+expression E;
+position v != choice.vok;
+position p;
+@@
+
+* E = \(vmalloc@v\|vzalloc@v\|vmalloc_user@v\|vmalloc_node@v\|vzalloc_node@v\|vmalloc_exec@v\|vmalloc_32@v\|vmalloc_32_user@v\|__vmalloc@v\|__vmalloc_node_range@v\|__vmalloc_node@v\)(...)
+  ... when != !is_vmalloc_addr(E)
+      when any
+* \(kfree\|kzfree\|kvfree\)(E)
+
+@pkfree depends on patch exists@
+expression E;
+position v != choice.vok;
+@@
+
+  E = \(vmalloc@v\|vzalloc@v\|vmalloc_user@v\|vmalloc_node@v\|vzalloc_node@v\|vmalloc_exec@v\|vmalloc_32@v\|vmalloc_32_user@v\|__vmalloc@v\|__vmalloc_node_range@v\|__vmalloc_node@v\)(...)
+  ... when != !is_vmalloc_addr(E)
+      when any
+- \(kfree\|kvfree\)(E)
++ vfree(E)
+
+@kvfree depends on !patch@
+expression E;
+position p, k;
+@@
+
+* E = \(kvmalloc\|kvzalloc\|kvcalloc\|kvzalloc_node\|kvmalloc_node\|kvmalloc_array\)(...)@k
+  ... when != is_vmalloc_addr(E)
+      when any
+* \(kfree\|kzfree\|vfree\|vfree_atomic\)(E)@p
+
+@pkvfree depends on patch exists@
+expression E;
+@@
+
+  E = \(kvmalloc\|kvzalloc\|kvcalloc\|kvzalloc_node\|kvmalloc_node\|kvmalloc_array\)(...)
+  ... when != is_vmalloc_addr(E)
+      when any
+- \(kfree\|vfree\)(E)
++ kvfree(E)
+
+@script: python depends on report@
+k << vfree.k;
+p << vfree.p;
+@@
+
+msg = "WARNING: kmalloc is used to allocate this memory at line %s" % (k[0].line)
+coccilib.report.print_report(p[0], msg)
+
+@script: python depends on org@
+k << vfree.k;
+p << vfree.p;
+@@
+
+msg = "WARNING: kmalloc is used to allocate this memory at line %s" % (k[0].line)
+coccilib.org.print_todo(p[0], msg)
+
+@script: python depends on report@
+v << kfree.v;
+p << kfree.p;
+@@
+
+msg = "WARNING: vmalloc is used to allocate this memory at line %s" % (v[0].line)
+coccilib.report.print_report(p[0], msg)
+
+@script: python depends on org@
+v << kfree.v;
+p << kfree.p;
+@@
+
+msg = "WARNING: vmalloc is used to allocate this memory at line %s" % (v[0].line)
+coccilib.org.print_todo(p[0], msg)
+
+@script: python depends on report@
+k << kvfree.k;
+p << kvfree.p;
+@@
+
+msg = "WARNING: kvmalloc is used to allocate this memory at line %s" % (k[0].line)
+coccilib.report.print_report(p[0], msg)
+
+@script: python depends on org@
+k << kvfree.k;
+p << kvfree.p;
+@@
+
+msg = "WARNING: kvmalloc is used to allocate this memory at line %s" % (k[0].line)
+coccilib.org.print_todo(p[0], msg)
+
+@script: python depends on report@
+p << opportunity.p;
+@@
+
+coccilib.report.print_report(p[0], "WARNING: opportunity for kvmalloc")
+
+@script: python depends on org@
+p << opportunity.p;
+@@
+
+coccilib.org.print_todo(p[0], "WARNING: opportunity for kvmalloc")
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 34+ messages in thread

end of thread, other threads:[~2020-06-07 14:02 UTC | newest]

Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-06 14:04 [PATCH] coccinelle: api: add kvfree script Markus Elfring
2020-06-06 14:04 ` [Cocci] " Markus Elfring
2020-06-06 14:04 ` Markus Elfring
2020-06-06 14:39 ` Julia Lawall
2020-06-06 14:39   ` [Cocci] " Julia Lawall
2020-06-06 14:39   ` Julia Lawall
2020-06-06 15:10   ` Markus Elfring
2020-06-06 15:10     ` [Cocci] " Markus Elfring
2020-06-06 15:10     ` Markus Elfring
  -- strict thread matches above, loose matches on Subject: below --
2020-06-06  7:30 [PATCH] " Markus Elfring
2020-06-06  7:30 ` [Cocci] " Markus Elfring
2020-06-06  7:30 ` Markus Elfring
2020-06-06  7:46 ` Julia Lawall
2020-06-06  7:46   ` [Cocci] " Julia Lawall
2020-06-06  7:46   ` Julia Lawall
2020-06-06  9:50   ` Markus Elfring
2020-06-06  9:50     ` [Cocci] " Markus Elfring
2020-06-06  9:50     ` Markus Elfring
2020-06-07 14:02     ` Coccinelle: Extending capabilities for source file pre-selection Markus Elfring
2020-06-07 14:02       ` [Cocci] " Markus Elfring
2020-06-07 14:02       ` Markus Elfring
2020-06-06 11:06   ` coccinelle: api: add kvfree script Markus Elfring
2020-06-06 11:06     ` [Cocci] " Markus Elfring
2020-06-06 11:06     ` Markus Elfring
2020-06-06 11:11     ` Julia Lawall
2020-06-06 11:11       ` [Cocci] " Julia Lawall
2020-06-06 11:11       ` Julia Lawall
2020-06-06 11:21       ` Markus Elfring
2020-06-06 11:21         ` [Cocci] " Markus Elfring
2020-06-06 11:21         ` Markus Elfring
2020-06-07  6:42         ` Coccinelle: Improving software components around usage of SmPL disjunctions Markus Elfring
2020-06-07  6:42           ` [Cocci] " Markus Elfring
2020-06-07  6:42           ` Markus Elfring
2020-06-05 20:42 [PATCH] coccinelle: api: add kvfree script Denis Efremov

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.