* [PATCH 0/2] Update memdup_user.cocci @ 2020-05-30 20:53 Denis Efremov 2020-05-30 20:53 ` [PATCH 1/2] Coccinelle: extend memdup_user transformation with GFP_USER Denis Efremov 2020-05-30 20:53 ` [PATCH 2/2] Coccinelle: extend memdup_user rule with vmemdup_user() Denis Efremov 0 siblings, 2 replies; 10+ messages in thread From: Denis Efremov @ 2020-05-30 20:53 UTC (permalink / raw) To: Julia Lawall, Joe Perches; +Cc: Denis Efremov, cocci, linux-kernel Add GFP_USER to the allocation flags and handle vmemdup_user(). Denis Efremov (2): Coccinelle: extend memdup_user transformation with GFP_USER Coccinelle: extend memdup_user rule with vmemdup_user() scripts/coccinelle/api/memdup_user.cocci | 53 ++++++++++++++++++++++-- 1 file changed, 49 insertions(+), 4 deletions(-) -- 2.26.2 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/2] Coccinelle: extend memdup_user transformation with GFP_USER 2020-05-30 20:53 [PATCH 0/2] Update memdup_user.cocci Denis Efremov @ 2020-05-30 20:53 ` Denis Efremov 2020-06-02 13:24 ` [Cocci] " Julia Lawall 2020-06-06 8:24 ` Julia Lawall 2020-05-30 20:53 ` [PATCH 2/2] Coccinelle: extend memdup_user rule with vmemdup_user() Denis Efremov 1 sibling, 2 replies; 10+ messages in thread From: Denis Efremov @ 2020-05-30 20:53 UTC (permalink / raw) To: Julia Lawall, Joe Perches; +Cc: Denis Efremov, cocci, linux-kernel Match GFP_USER allocations with memdup_user.cocci rule. Commit 6c2c97a24f09 ("memdup_user(): switch to GFP_USER") switched memdup_user() from GFP_KERNEL to GFP_USER. In most cases it is still a good idea to use memdup_user() for GFP_KERNEL allocations. The motivation behind altering memdup_user() to GFP_USER is here: https://lkml.org/lkml/2018/1/6/333 Signed-off-by: Denis Efremov <efremov@linux.com> --- scripts/coccinelle/api/memdup_user.cocci | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/coccinelle/api/memdup_user.cocci b/scripts/coccinelle/api/memdup_user.cocci index c809ab10bbce..49f487e6a5c8 100644 --- a/scripts/coccinelle/api/memdup_user.cocci +++ b/scripts/coccinelle/api/memdup_user.cocci @@ -20,7 +20,7 @@ expression from,to,size; identifier l1,l2; @@ -- to = \(kmalloc\|kzalloc\)(size,GFP_KERNEL); +- to = \(kmalloc\|kzalloc\)(size,\(GFP_KERNEL\|GFP_USER\)); + to = memdup_user(from,size); if ( - to==NULL @@ -43,7 +43,7 @@ position p; statement S1,S2; @@ -* to = \(kmalloc@p\|kzalloc@p\)(size,GFP_KERNEL); +* to = \(kmalloc@p\|kzalloc@p\)(size,\(GFP_KERNEL\|GFP_USER\)); if (to==NULL || ...) S1 if (copy_from_user(to, from, size) != 0) S2 -- 2.26.2 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Cocci] [PATCH 1/2] Coccinelle: extend memdup_user transformation with GFP_USER 2020-05-30 20:53 ` [PATCH 1/2] Coccinelle: extend memdup_user transformation with GFP_USER Denis Efremov @ 2020-06-02 13:24 ` Julia Lawall 2020-06-06 8:24 ` Julia Lawall 1 sibling, 0 replies; 10+ messages in thread From: Julia Lawall @ 2020-06-02 13:24 UTC (permalink / raw) To: Denis Efremov; +Cc: Joe Perches, linux-kernel On Sat, 30 May 2020, Denis Efremov wrote: > Match GFP_USER allocations with memdup_user.cocci rule. > Commit 6c2c97a24f09 ("memdup_user(): switch to GFP_USER") switched > memdup_user() from GFP_KERNEL to GFP_USER. In most cases it is still > a good idea to use memdup_user() for GFP_KERNEL allocations. The > motivation behind altering memdup_user() to GFP_USER is here: > https://lkml.org/lkml/2018/1/6/333 Thanks for the patch series. I will test them and try to push them to Linus shortly. julia > > Signed-off-by: Denis Efremov <efremov@linux.com> > --- > scripts/coccinelle/api/memdup_user.cocci | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/scripts/coccinelle/api/memdup_user.cocci b/scripts/coccinelle/api/memdup_user.cocci > index c809ab10bbce..49f487e6a5c8 100644 > --- a/scripts/coccinelle/api/memdup_user.cocci > +++ b/scripts/coccinelle/api/memdup_user.cocci > @@ -20,7 +20,7 @@ expression from,to,size; > identifier l1,l2; > @@ > > -- to = \(kmalloc\|kzalloc\)(size,GFP_KERNEL); > +- to = \(kmalloc\|kzalloc\)(size,\(GFP_KERNEL\|GFP_USER\)); > + to = memdup_user(from,size); > if ( > - to==NULL > @@ -43,7 +43,7 @@ position p; > statement S1,S2; > @@ > > -* to = \(kmalloc@p\|kzalloc@p\)(size,GFP_KERNEL); > +* to = \(kmalloc@p\|kzalloc@p\)(size,\(GFP_KERNEL\|GFP_USER\)); > if (to==NULL || ...) S1 > if (copy_from_user(to, from, size) != 0) > S2 > -- > 2.26.2 > > _______________________________________________ > Cocci mailing list > Cocci@systeme.lip6.fr > https://systeme.lip6.fr/mailman/listinfo/cocci > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Cocci] [PATCH 1/2] Coccinelle: extend memdup_user transformation with GFP_USER 2020-05-30 20:53 ` [PATCH 1/2] Coccinelle: extend memdup_user transformation with GFP_USER Denis Efremov 2020-06-02 13:24 ` [Cocci] " Julia Lawall @ 2020-06-06 8:24 ` Julia Lawall 2020-06-06 20:36 ` Denis Efremov 2020-06-06 20:46 ` Denis Efremov 1 sibling, 2 replies; 10+ messages in thread From: Julia Lawall @ 2020-06-06 8:24 UTC (permalink / raw) To: Denis Efremov; +Cc: Joe Perches, cocci, linux-kernel On Sat, 30 May 2020, Denis Efremov wrote: > Match GFP_USER allocations with memdup_user.cocci rule. > Commit 6c2c97a24f09 ("memdup_user(): switch to GFP_USER") switched > memdup_user() from GFP_KERNEL to GFP_USER. In most cases it is still > a good idea to use memdup_user() for GFP_KERNEL allocations. The > motivation behind altering memdup_user() to GFP_USER is here: > https://lkml.org/lkml/2018/1/6/333 Should the rule somehow document the cases in which memdup_user should now not be used? julia > > Signed-off-by: Denis Efremov <efremov@linux.com> > --- > scripts/coccinelle/api/memdup_user.cocci | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/scripts/coccinelle/api/memdup_user.cocci b/scripts/coccinelle/api/memdup_user.cocci > index c809ab10bbce..49f487e6a5c8 100644 > --- a/scripts/coccinelle/api/memdup_user.cocci > +++ b/scripts/coccinelle/api/memdup_user.cocci > @@ -20,7 +20,7 @@ expression from,to,size; > identifier l1,l2; > @@ > > -- to = \(kmalloc\|kzalloc\)(size,GFP_KERNEL); > +- to = \(kmalloc\|kzalloc\)(size,\(GFP_KERNEL\|GFP_USER\)); > + to = memdup_user(from,size); > if ( > - to==NULL > @@ -43,7 +43,7 @@ position p; > statement S1,S2; > @@ > > -* to = \(kmalloc@p\|kzalloc@p\)(size,GFP_KERNEL); > +* to = \(kmalloc@p\|kzalloc@p\)(size,\(GFP_KERNEL\|GFP_USER\)); > if (to==NULL || ...) S1 > if (copy_from_user(to, from, size) != 0) > S2 > -- > 2.26.2 > > _______________________________________________ > Cocci mailing list > Cocci@systeme.lip6.fr > https://systeme.lip6.fr/mailman/listinfo/cocci > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Cocci] [PATCH 1/2] Coccinelle: extend memdup_user transformation with GFP_USER 2020-06-06 8:24 ` Julia Lawall @ 2020-06-06 20:36 ` Denis Efremov 2020-06-06 20:46 ` Denis Efremov 1 sibling, 0 replies; 10+ messages in thread From: Denis Efremov @ 2020-06-06 20:36 UTC (permalink / raw) To: Julia Lawall; +Cc: Joe Perches, cocci, linux-kernel On 6/6/20 11:24 AM, Julia Lawall wrote: > > > On Sat, 30 May 2020, Denis Efremov wrote: > >> Match GFP_USER allocations with memdup_user.cocci rule. >> Commit 6c2c97a24f09 ("memdup_user(): switch to GFP_USER") switched >> memdup_user() from GFP_KERNEL to GFP_USER. In most cases it is still >> a good idea to use memdup_user() for GFP_KERNEL allocations. The >> motivation behind altering memdup_user() to GFP_USER is here: >> https://lkml.org/lkml/2018/1/6/333 > > Should the rule somehow document the cases in which memdup_user should now > not be used? As for now, I can't provide a counterexample. GPF_USER is more permissive than GFP_KERNEL. It's completely ok to use GPF_USER with copy_from_user. Given that memdup_user() was "silently" switched to GPF_USER from GPF_KERNEL with no callside fixes, I think it's ok to recommend to use memdup_user for GPF_KERNEL matches with no additional restrictions. Thanks, Denis ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Cocci] [PATCH 1/2] Coccinelle: extend memdup_user transformation with GFP_USER 2020-06-06 8:24 ` Julia Lawall 2020-06-06 20:36 ` Denis Efremov @ 2020-06-06 20:46 ` Denis Efremov 2020-06-06 20:50 ` Julia Lawall 1 sibling, 1 reply; 10+ messages in thread From: Denis Efremov @ 2020-06-06 20:46 UTC (permalink / raw) To: Julia Lawall; +Cc: Joe Perches, cocci, linux-kernel On 6/6/20 11:24 AM, Julia Lawall wrote: > > > On Sat, 30 May 2020, Denis Efremov wrote: > >> Match GFP_USER allocations with memdup_user.cocci rule. >> Commit 6c2c97a24f09 ("memdup_user(): switch to GFP_USER") switched >> memdup_user() from GFP_KERNEL to GFP_USER. In most cases it is still >> a good idea to use memdup_user() for GFP_KERNEL allocations. The >> motivation behind altering memdup_user() to GFP_USER is here: >> https://lkml.org/lkml/2018/1/6/333 > > Should the rule somehow document the cases in which memdup_user should now > not be used? > > julia > > >> >> Signed-off-by: Denis Efremov <efremov@linux.com> >> --- >> scripts/coccinelle/api/memdup_user.cocci | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/scripts/coccinelle/api/memdup_user.cocci b/scripts/coccinelle/api/memdup_user.cocci >> index c809ab10bbce..49f487e6a5c8 100644 >> --- a/scripts/coccinelle/api/memdup_user.cocci >> +++ b/scripts/coccinelle/api/memdup_user.cocci >> @@ -20,7 +20,7 @@ expression from,to,size; >> identifier l1,l2; >> @@ >> >> -- to = \(kmalloc\|kzalloc\)(size,GFP_KERNEL); >> +- to = \(kmalloc\|kzalloc\)(size,\(GFP_KERNEL\|GFP_USER\)); Actually, we can add optional __GFP_NOWARN here to match such cases as: GFP_KERNEL | __GFP_NOWARN However, I don't know how to express it in elegant way. Something like? ( - to = \(kmalloc\|kzalloc\)(size,\(GFP_KERNEL\|GFP_USER\)); | - to = \(kmalloc\|kzalloc\)(size, GFP_KERNEL|__GFP_NOWARN); | - to = \(kmalloc\|kzalloc\)(size, GFP_USER|__GFP_NOWARN); ) Thanks, Denis ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Cocci] [PATCH 1/2] Coccinelle: extend memdup_user transformation with GFP_USER 2020-06-06 20:46 ` Denis Efremov @ 2020-06-06 20:50 ` Julia Lawall 0 siblings, 0 replies; 10+ messages in thread From: Julia Lawall @ 2020-06-06 20:50 UTC (permalink / raw) To: Denis Efremov; +Cc: Joe Perches, cocci, linux-kernel On Sat, 6 Jun 2020, Denis Efremov wrote: > > > On 6/6/20 11:24 AM, Julia Lawall wrote: > > > > > > On Sat, 30 May 2020, Denis Efremov wrote: > > > >> Match GFP_USER allocations with memdup_user.cocci rule. > >> Commit 6c2c97a24f09 ("memdup_user(): switch to GFP_USER") switched > >> memdup_user() from GFP_KERNEL to GFP_USER. In most cases it is still > >> a good idea to use memdup_user() for GFP_KERNEL allocations. The > >> motivation behind altering memdup_user() to GFP_USER is here: > >> https://lkml.org/lkml/2018/1/6/333 > > > > Should the rule somehow document the cases in which memdup_user should now > > not be used? > > > > julia > > > > > >> > >> Signed-off-by: Denis Efremov <efremov@linux.com> > >> --- > >> scripts/coccinelle/api/memdup_user.cocci | 4 ++-- > >> 1 file changed, 2 insertions(+), 2 deletions(-) > >> > >> diff --git a/scripts/coccinelle/api/memdup_user.cocci b/scripts/coccinelle/api/memdup_user.cocci > >> index c809ab10bbce..49f487e6a5c8 100644 > >> --- a/scripts/coccinelle/api/memdup_user.cocci > >> +++ b/scripts/coccinelle/api/memdup_user.cocci > >> @@ -20,7 +20,7 @@ expression from,to,size; > >> identifier l1,l2; > >> @@ > >> > >> -- to = \(kmalloc\|kzalloc\)(size,GFP_KERNEL); > >> +- to = \(kmalloc\|kzalloc\)(size,\(GFP_KERNEL\|GFP_USER\)); > > Actually, we can add optional __GFP_NOWARN here to match such cases as: > GFP_KERNEL | __GFP_NOWARN > > However, I don't know how to express it in elegant way. Something like? > ( > - to = \(kmalloc\|kzalloc\)(size,\(GFP_KERNEL\|GFP_USER\)); > | > - to = \(kmalloc\|kzalloc\)(size, GFP_KERNEL|__GFP_NOWARN); > | > - to = \(kmalloc\|kzalloc\)(size, GFP_USER|__GFP_NOWARN); > ) I guess you could do: \(GFP_KERNEL\|GFP_USER\|\(GFP_KERNEL\|GFP_USER\)|__GFP_NOWARN\) julia ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/2] Coccinelle: extend memdup_user rule with vmemdup_user() 2020-05-30 20:53 [PATCH 0/2] Update memdup_user.cocci Denis Efremov 2020-05-30 20:53 ` [PATCH 1/2] Coccinelle: extend memdup_user transformation with GFP_USER Denis Efremov @ 2020-05-30 20:53 ` Denis Efremov 2020-06-06 9:27 ` [Cocci] " Julia Lawall 1 sibling, 1 reply; 10+ messages in thread From: Denis Efremov @ 2020-05-30 20:53 UTC (permalink / raw) To: Julia Lawall, Joe Perches; +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 | 49 +++++++++++++++++++++++- 1 file changed, 47 insertions(+), 2 deletions(-) diff --git a/scripts/coccinelle/api/memdup_user.cocci b/scripts/coccinelle/api/memdup_user.cocci index 49f487e6a5c8..a50def35136e 100644 --- a/scripts/coccinelle/api/memdup_user.cocci +++ b/scripts/coccinelle/api/memdup_user.cocci @@ -37,6 +37,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; @@ -48,14 +70,37 @@ statement S1,S2; if (copy_from_user(to, from, size) != 0) S2 -@script:python depends on org@ +@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 && r@ p << r.p; @@ coccilib.org.print_todo(p[0], "WARNING opportunity for memdup_user") -@script:python depends on report@ +@script:python depends on report && r@ p << r.p; @@ coccilib.report.print_report(p[0], "WARNING opportunity for memdup_user") + +@script:python depends on org && rv@ +p << rv.p; +@@ + +coccilib.org.print_todo(p[0], "WARNING opportunity for vmemdup_user") + +@script:python depends on report && rv@ +p << rv.p; +@@ + +coccilib.report.print_report(p[0], "WARNING opportunity for vmemdup_user") -- 2.26.2 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Cocci] [PATCH 2/2] Coccinelle: extend memdup_user rule with vmemdup_user() 2020-05-30 20:53 ` [PATCH 2/2] Coccinelle: extend memdup_user rule with vmemdup_user() Denis Efremov @ 2020-06-06 9:27 ` Julia Lawall 0 siblings, 0 replies; 10+ messages in thread From: Julia Lawall @ 2020-06-06 9:27 UTC (permalink / raw) To: Denis Efremov; +Cc: Joe Perches, cocci, linux-kernel On Sat, 30 May 2020, Denis Efremov wrote: > 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 | 49 +++++++++++++++++++++++- > 1 file changed, 47 insertions(+), 2 deletions(-) > > diff --git a/scripts/coccinelle/api/memdup_user.cocci b/scripts/coccinelle/api/memdup_user.cocci > index 49f487e6a5c8..a50def35136e 100644 > --- a/scripts/coccinelle/api/memdup_user.cocci > +++ b/scripts/coccinelle/api/memdup_user.cocci > @@ -37,6 +37,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 > +- ...+> > +- } > + This could protect against modifying vmemdup_user. Probably the original rule should protect against modifying memdup_user as well. julia > @r depends on !patch@ > expression from,to,size; > position p; > @@ -48,14 +70,37 @@ statement S1,S2; > if (copy_from_user(to, from, size) != 0) > S2 > > -@script:python depends on org@ > +@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 && r@ > p << r.p; > @@ > > coccilib.org.print_todo(p[0], "WARNING opportunity for memdup_user") > > -@script:python depends on report@ > +@script:python depends on report && r@ > p << r.p; > @@ > > coccilib.report.print_report(p[0], "WARNING opportunity for memdup_user") > + > +@script:python depends on org && rv@ > +p << rv.p; > +@@ > + > +coccilib.org.print_todo(p[0], "WARNING opportunity for vmemdup_user") > + > +@script:python depends on report && rv@ > +p << rv.p; > +@@ > + > +coccilib.report.print_report(p[0], "WARNING opportunity for vmemdup_user") > -- > 2.26.2 > > _______________________________________________ > Cocci mailing list > Cocci@systeme.lip6.fr > https://systeme.lip6.fr/mailman/listinfo/cocci > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] Coccinelle: extend memdup_user transformation with GFP_USER
@ 2020-05-31 17:36 Markus Elfring
0 siblings, 0 replies; 10+ messages in thread
From: Markus Elfring @ 2020-05-31 17:36 UTC (permalink / raw)
To: Denis Efremov, Gilles Muller, Julia Lawall, Masahiro Yamada,
Michal Marek, Nicolas Palix, Coccinelle
Cc: kernel-janitors, linux-kernel, Joe Perches
> Match GFP_USER allocations with memdup_user.cocci rule.
Can this software extension help also for the clarification of the topic
“Safer source code analysis by "memdup_user.cocci"”?
https://github.com/coccinelle/coccinelle/issues/78
Regards,
Markus
^ permalink raw reply [flat|nested] 10+ messages in threadend of thread, other threads:[~2020-06-06 20:50 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-05-30 20:53 [PATCH 0/2] Update memdup_user.cocci Denis Efremov 2020-05-30 20:53 ` [PATCH 1/2] Coccinelle: extend memdup_user transformation with GFP_USER Denis Efremov 2020-06-02 13:24 ` [Cocci] " Julia Lawall 2020-06-06 8:24 ` Julia Lawall 2020-06-06 20:36 ` Denis Efremov 2020-06-06 20:46 ` Denis Efremov 2020-06-06 20:50 ` Julia Lawall 2020-05-30 20:53 ` [PATCH 2/2] Coccinelle: extend memdup_user rule with vmemdup_user() Denis Efremov 2020-06-06 9:27 ` [Cocci] " Julia Lawall 2020-05-31 17:36 [PATCH 1/2] Coccinelle: 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
Powered by JetHome