linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/3] Update memdup_user.cocci
@ 2020-07-20 16:22 Denis Efremov
  2020-07-20 16:22 ` [PATCH v3 1/3] coccinelle: api: extend memdup_user transformation with GFP_USER Denis Efremov
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Denis Efremov @ 2020-07-20 16:22 UTC (permalink / raw)
  To: Julia Lawall; +Cc: Denis Efremov, cocci, linux-kernel

Add GFP_USER to the allocation flags and handle vmemdup_user().

Changes in v2:
 - memdup_user/vmemdup_user matching suppressed
 - PoC for selfcheck virtual rule
Changes in v3:
 - add missing '-' for patch rule in kmalloc/kzalloc call args
 - selfcheck rule dropped from patchset

Denis Efremov (3):
  coccinelle: api: extend memdup_user transformation with GFP_USER
  coccinelle: api: extend memdup_user rule with vmemdup_user()
  coccinelle: api: filter out memdup_user definitions

 scripts/coccinelle/api/memdup_user.cocci | 64 ++++++++++++++++++++++--
 1 file changed, 61 insertions(+), 3 deletions(-)

-- 
2.26.2


^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH v3 1/3] coccinelle: api: extend memdup_user transformation with GFP_USER
  2020-07-20 16:22 [PATCH v3 0/3] Update memdup_user.cocci Denis Efremov
@ 2020-07-20 16:22 ` Denis Efremov
  2020-07-20 16:22 ` [PATCH v3 2/3] coccinelle: api: extend memdup_user rule with vmemdup_user() Denis Efremov
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 6+ messages in thread
From: Denis Efremov @ 2020-07-20 16:22 UTC (permalink / raw)
  To: Julia Lawall; +Cc: Denis Efremov, cocci, linux-kernel

Match GFP_USER and optional __GFP_NOWARN allocations with
memdup_user.cocci rule.
Commit 6c2c97a24f09 ("memdup_user(): switch to GFP_USER") switched
memdup_user() from GFP_KERNEL to GFP_USER. In almost all cases it
is still a good idea to recommend memdup_user() for GFP_KERNEL
allocations. The motivation behind altering memdup_user() to GFP_USER:
https://lkml.org/lkml/2018/1/6/333

Signed-off-by: Denis Efremov <efremov@linux.com>
---
 scripts/coccinelle/api/memdup_user.cocci | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/scripts/coccinelle/api/memdup_user.cocci b/scripts/coccinelle/api/memdup_user.cocci
index c809ab10bbce..0e29d41ecab8 100644
--- a/scripts/coccinelle/api/memdup_user.cocci
+++ b/scripts/coccinelle/api/memdup_user.cocci
@@ -20,7 +20,9 @@ expression from,to,size;
 identifier l1,l2;
 @@
 
--  to = \(kmalloc\|kzalloc\)(size,GFP_KERNEL);
+-  to = \(kmalloc\|kzalloc\)
+-		(size,\(GFP_KERNEL\|GFP_USER\|
+-		      \(GFP_KERNEL\|GFP_USER\)|__GFP_NOWARN\));
 +  to = memdup_user(from,size);
    if (
 -      to==NULL
@@ -43,7 +45,9 @@ position p;
 statement S1,S2;
 @@
 
-*  to = \(kmalloc@p\|kzalloc@p\)(size,GFP_KERNEL);
+*  to = \(kmalloc@p\|kzalloc@p\)
+		(size,\(GFP_KERNEL\|GFP_USER\|
+		      \(GFP_KERNEL\|GFP_USER\)|__GFP_NOWARN\));
    if (to==NULL || ...) S1
    if (copy_from_user(to, from, size) != 0)
    S2
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH v3 2/3] coccinelle: api: extend memdup_user rule with vmemdup_user()
  2020-07-20 16:22 [PATCH v3 0/3] Update memdup_user.cocci Denis Efremov
  2020-07-20 16:22 ` [PATCH v3 1/3] coccinelle: api: extend memdup_user transformation with GFP_USER Denis Efremov
@ 2020-07-20 16:22 ` Denis Efremov
  2020-07-20 16:22 ` [PATCH v3 3/3] coccinelle: api: filter out memdup_user definitions Denis Efremov
  2020-07-24 20:02 ` [PATCH v3 0/3] Update memdup_user.cocci Julia Lawall
  3 siblings, 0 replies; 6+ messages in thread
From: Denis Efremov @ 2020-07-20 16:22 UTC (permalink / raw)
  To: Julia Lawall; +Cc: Denis Efremov, cocci, linux-kernel

Add vmemdup_user() transformations to the memdup_user.cocci rule.
Commit 50fd2f298bef ("new primitive: vmemdup_user()") introduced
vmemdup_user(). The function uses kvmalloc with GPF_USER flag.

Signed-off-by: Denis Efremov <efremov@linux.com>
---
 scripts/coccinelle/api/memdup_user.cocci | 45 ++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/scripts/coccinelle/api/memdup_user.cocci b/scripts/coccinelle/api/memdup_user.cocci
index 0e29d41ecab8..60027e21c5e6 100644
--- a/scripts/coccinelle/api/memdup_user.cocci
+++ b/scripts/coccinelle/api/memdup_user.cocci
@@ -39,6 +39,28 @@ identifier l1,l2;
 -    ...+>
 -  }
 
+@depends on patch@
+expression from,to,size;
+identifier l1,l2;
+@@
+
+-  to = \(kvmalloc\|kvzalloc\)(size,\(GFP_KERNEL\|GFP_USER\));
++  to = vmemdup_user(from,size);
+   if (
+-      to==NULL
++      IS_ERR(to)
+                 || ...) {
+   <+... when != goto l1;
+-  -ENOMEM
++  PTR_ERR(to)
+   ...+>
+   }
+-  if (copy_from_user(to, from, size) != 0) {
+-    <+... when != goto l2;
+-    -EFAULT
+-    ...+>
+-  }
+
 @r depends on !patch@
 expression from,to,size;
 position p;
@@ -52,6 +74,17 @@ statement S1,S2;
    if (copy_from_user(to, from, size) != 0)
    S2
 
+@rv depends on !patch@
+expression from,to,size;
+position p;
+statement S1,S2;
+@@
+
+*  to = \(kvmalloc@p\|kvzalloc@p\)(size,\(GFP_KERNEL\|GFP_USER\));
+   if (to==NULL || ...) S1
+   if (copy_from_user(to, from, size) != 0)
+   S2
+
 @script:python depends on org@
 p << r.p;
 @@
@@ -63,3 +96,15 @@ p << r.p;
 @@
 
 coccilib.report.print_report(p[0], "WARNING opportunity for memdup_user")
+
+@script:python depends on org@
+p << rv.p;
+@@
+
+coccilib.org.print_todo(p[0], "WARNING opportunity for vmemdup_user")
+
+@script:python depends on report@
+p << rv.p;
+@@
+
+coccilib.report.print_report(p[0], "WARNING opportunity for vmemdup_user")
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH v3 3/3] coccinelle: api: filter out memdup_user definitions
  2020-07-20 16:22 [PATCH v3 0/3] Update memdup_user.cocci Denis Efremov
  2020-07-20 16:22 ` [PATCH v3 1/3] coccinelle: api: extend memdup_user transformation with GFP_USER Denis Efremov
  2020-07-20 16:22 ` [PATCH v3 2/3] coccinelle: api: extend memdup_user rule with vmemdup_user() Denis Efremov
@ 2020-07-20 16:22 ` Denis Efremov
  2020-07-24 20:02 ` [PATCH v3 0/3] Update memdup_user.cocci Julia Lawall
  3 siblings, 0 replies; 6+ messages in thread
From: Denis Efremov @ 2020-07-20 16:22 UTC (permalink / raw)
  To: Julia Lawall; +Cc: Denis Efremov, cocci, linux-kernel

Don't match memdup_user/vmemdup_user.

Signed-off-by: Denis Efremov <efremov@linux.com>
---
 scripts/coccinelle/api/memdup_user.cocci | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/scripts/coccinelle/api/memdup_user.cocci b/scripts/coccinelle/api/memdup_user.cocci
index 60027e21c5e6..e01e95108405 100644
--- a/scripts/coccinelle/api/memdup_user.cocci
+++ b/scripts/coccinelle/api/memdup_user.cocci
@@ -15,12 +15,20 @@ virtual context
 virtual org
 virtual report
 
+@initialize:python@
+@@
+filter = frozenset(['memdup_user', 'vmemdup_user'])
+
+def relevant(p):
+    return not (filter & {el.current_element for el in p})
+
 @depends on patch@
 expression from,to,size;
 identifier l1,l2;
+position p : script:python() { relevant(p) };
 @@
 
--  to = \(kmalloc\|kzalloc\)
+-  to = \(kmalloc@p\|kzalloc@p\)
 -		(size,\(GFP_KERNEL\|GFP_USER\|
 -		      \(GFP_KERNEL\|GFP_USER\)|__GFP_NOWARN\));
 +  to = memdup_user(from,size);
@@ -42,9 +50,10 @@ identifier l1,l2;
 @depends on patch@
 expression from,to,size;
 identifier l1,l2;
+position p : script:python() { relevant(p) };
 @@
 
--  to = \(kvmalloc\|kvzalloc\)(size,\(GFP_KERNEL\|GFP_USER\));
+-  to = \(kvmalloc@p\|kvzalloc@p\)(size,\(GFP_KERNEL\|GFP_USER\));
 +  to = vmemdup_user(from,size);
    if (
 -      to==NULL
@@ -63,7 +72,7 @@ identifier l1,l2;
 
 @r depends on !patch@
 expression from,to,size;
-position p;
+position p : script:python() { relevant(p) };
 statement S1,S2;
 @@
 
@@ -76,7 +85,7 @@ statement S1,S2;
 
 @rv depends on !patch@
 expression from,to,size;
-position p;
+position p : script:python() { relevant(p) };
 statement S1,S2;
 @@
 
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/3] coccinelle: api: extend memdup_user transformation with GFP_USER
@ 2020-07-21  8:24 Markus Elfring
  0 siblings, 0 replies; 6+ messages in thread
From: Markus Elfring @ 2020-07-21  8:24 UTC (permalink / raw)
  To: Denis Efremov, Coccinelle, Gilles Muller, Julia Lawall,
	Masahiro Yamada, Michal Marek, Nicolas Palix
  Cc: linux-kernel, kernel-janitors

> Match GFP_USER and optional __GFP_NOWARN allocations with
> memdup_user.cocci rule.

I suggest to clarify software design consequences according to such information
a bit more.


I find it helpful if you would have included also my email address directly
in the message field “To” or “Cc”.
Are there further reasons to consider for the extension of the recipient lists?


> +-  to = \(kmalloc\|kzalloc\)
> +-		(size,\(GFP_KERNEL\|GFP_USER\|
> +-		      \(GFP_KERNEL\|GFP_USER\)|__GFP_NOWARN\));

* Would you ever dare to specify such a source code search pattern
  on a single line?

* I find the specification of SmPL disjunctions questionable
  for the determination of relevant flags.
  Could previous patch review trigger concerns and further considerations
  for the proper handling of optional source code parts?


> +*  to = \(kmalloc@p\|kzalloc@p\)
> +		(size,\(GFP_KERNEL\|GFP_USER\|
> +		      \(GFP_KERNEL\|GFP_USER\)|__GFP_NOWARN\));

Would you like to use the SmPL asterisk really only for a single line?


How will the chances evolve to continue the clarification also for
the open issue “Safer source code analysis by "memdup_user.cocci"”?
https://github.com/coccinelle/coccinelle/issues/78

Regards,
Markus

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH v3 0/3] Update memdup_user.cocci
  2020-07-20 16:22 [PATCH v3 0/3] Update memdup_user.cocci Denis Efremov
                   ` (2 preceding siblings ...)
  2020-07-20 16:22 ` [PATCH v3 3/3] coccinelle: api: filter out memdup_user definitions Denis Efremov
@ 2020-07-24 20:02 ` Julia Lawall
  3 siblings, 0 replies; 6+ messages in thread
From: Julia Lawall @ 2020-07-24 20:02 UTC (permalink / raw)
  To: Denis Efremov; +Cc: cocci, linux-kernel



On Mon, 20 Jul 2020, Denis Efremov wrote:

> Add GFP_USER to the allocation flags and handle vmemdup_user().
>
> Changes in v2:
>  - memdup_user/vmemdup_user matching suppressed
>  - PoC for selfcheck virtual rule
> Changes in v3:
>  - add missing '-' for patch rule in kmalloc/kzalloc call args
>  - selfcheck rule dropped from patchset
>
> Denis Efremov (3):
>   coccinelle: api: extend memdup_user transformation with GFP_USER
>   coccinelle: api: extend memdup_user rule with vmemdup_user()
>   coccinelle: api: filter out memdup_user definitions

All three applied.  Thanks.

julia

>
>  scripts/coccinelle/api/memdup_user.cocci | 64 ++++++++++++++++++++++--
>  1 file changed, 61 insertions(+), 3 deletions(-)
>
> --
> 2.26.2
>
>

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-07-24 20:02 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-07-20 16:22 [PATCH v3 0/3] Update memdup_user.cocci Denis Efremov
2020-07-20 16:22 ` [PATCH v3 1/3] coccinelle: api: extend memdup_user transformation with GFP_USER Denis Efremov
2020-07-20 16:22 ` [PATCH v3 2/3] coccinelle: api: extend memdup_user rule with vmemdup_user() Denis Efremov
2020-07-20 16:22 ` [PATCH v3 3/3] coccinelle: api: filter out memdup_user definitions Denis Efremov
2020-07-24 20:02 ` [PATCH v3 0/3] Update memdup_user.cocci Julia Lawall
  -- strict thread matches above, loose matches on Subject: below --
2020-07-21  8:24 [PATCH v3 1/3] coccinelle: api: extend memdup_user transformation with GFP_USER Markus Elfring

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).