Re: [PATCH v2 1/4] coccinelle: api: extend memdup_user transformation with GFP_USER

Re: [PATCH v2 1/4] coccinelle: api: extend memdup_user transformation with GFP_USER

[Markus Elfring]

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

You suggest another interesting software extension.


…
> +++ 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);

But I find the proposed change for such SmPL code inappropriate.

It was specified by the means of the semantic patch language
to replace an assignment statement for which a function call provides a value.
I would interpret your adjustment in the way that an expression list
should be preserved.
Were two minus characters forgotten for the first SmPL rule?

Further concerns should be taken into account for the SmPL rule “r”.

I would appreciate if the clarification will be continued also for
the topic “Safer source code analysis by "memdup_user.cocci"”?
https://github.com/coccinelle/coccinelle/issues/78

Regards,
Markus

Re: [PATCH v2 1/4] coccinelle: api: extend memdup_user transformation with GFP_USER

[Markus Elfring]

…
> +++ b/scripts/coccinelle/api/memdup_user.cocci
> @@ -20,7 +20,9 @@ expression from,to,size;
…
> +-  to = \(kmalloc\|kzalloc\)
> +		(size,\(GFP_KERNEL\|GFP_USER\|
> +		      \(GFP_KERNEL\|GFP_USER\)|__GFP_NOWARN\));

I got the impression that this SmPL code needs another correction also
according to the proposed SmPL disjunction.

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


Would you like to express by any other approach that a specific flag
is an optional source code transformation parameter?

Regards,
Markus

[PATCH v2 1/4] coccinelle: api: extend memdup_user transformation with GFP_USER

[Denis Efremov]

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..cadcc2e87881 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