mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] checkpatch: Add --strict test for kmalloc/kzalloc with multiply
@ 2014-05-13 23:48 Joe Perches
  2014-05-15 22:58 ` Andrew Morton
  0 siblings, 1 reply; 5+ messages in thread
From: Joe Perches @ 2014-05-13 23:48 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Andy Whitcroft, LKML

Protect against sizeof overflows by preferring
kmalloc_array and kcalloc to kmalloc/kzalloc
with a sizeof multiply.

Signed-off-by: Joe Perches <joe@perches.com>
---
 scripts/checkpatch.pl | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index e7ff52a..33eb71f 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -4378,6 +4378,20 @@ sub process {
 			    "Prefer $3(sizeof(*$1)...) over $3($4...)\n" . $herecurr);
 		}
 
+# check for k[mz]alloc with multiplies that could be kmalloc_array/kcalloc
+		if ($^V && $^V ge 5.10.0 &&
+		    $line =~ /\b($Lval)\s*\=\s*(?:$balanced_parens)?\s*(k[mz]alloc)\s*\(\s*($FuncArg)\s*\*\s*($FuncArg)/) {
+			my $oldfunc = $3;
+			my $a1 = $4;
+			my $a2 = $10;
+			my $newfunc = "kmalloc_array";
+			$newfunc = "kcalloc" if ($oldfunc eq "kzalloc");
+			if ($a1 =~ /^sizeof\s*\S/ || $a2 =~ /^sizeof\s*\S/) {
+				CHK("ALLOC_WITH_MULTIPLY",
+				    "Prefer $newfunc over $oldfunc with multiply\n" . $herecurr);
+			}
+		}
+
 # check for krealloc arg reuse
 		if ($^V && $^V ge 5.10.0 &&
 		    $line =~ /\b($Lval)\s*\=\s*(?:$balanced_parens)?\s*krealloc\s*\(\s*\1\s*,/) {



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

* Re: [PATCH] checkpatch: Add --strict test for kmalloc/kzalloc with multiply
  2014-05-13 23:48 [PATCH] checkpatch: Add --strict test for kmalloc/kzalloc with multiply Joe Perches
@ 2014-05-15 22:58 ` Andrew Morton
  2014-05-15 23:04   ` Joe Perches
  0 siblings, 1 reply; 5+ messages in thread
From: Andrew Morton @ 2014-05-15 22:58 UTC (permalink / raw)
  To: Joe Perches; +Cc: Andy Whitcroft, LKML

On Tue, 13 May 2014 16:48:49 -0700 Joe Perches <joe@perches.com> wrote:

> Protect against sizeof overflows by preferring
> kmalloc_array and kcalloc to kmalloc/kzalloc
> with a sizeof multiply.
> 
> ...
>
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -4378,6 +4378,20 @@ sub process {
>  			    "Prefer $3(sizeof(*$1)...) over $3($4...)\n" . $herecurr);
>  		}
>  
> +# check for k[mz]alloc with multiplies that could be kmalloc_array/kcalloc
> +		if ($^V && $^V ge 5.10.0 &&
> +		    $line =~ /\b($Lval)\s*\=\s*(?:$balanced_parens)?\s*(k[mz]alloc)\s*\(\s*($FuncArg)\s*\*\s*($FuncArg)/) {
> +			my $oldfunc = $3;
> +			my $a1 = $4;
> +			my $a2 = $10;
> +			my $newfunc = "kmalloc_array";
> +			$newfunc = "kcalloc" if ($oldfunc eq "kzalloc");
> +			if ($a1 =~ /^sizeof\s*\S/ || $a2 =~ /^sizeof\s*\S/) {
> +				CHK("ALLOC_WITH_MULTIPLY",
> +				    "Prefer $newfunc over $oldfunc with multiply\n" . $herecurr);
> +			}
> +		}
> +

Why hide this behind --strict?

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

* Re: [PATCH] checkpatch: Add --strict test for kmalloc/kzalloc with multiply
  2014-05-15 22:58 ` Andrew Morton
@ 2014-05-15 23:04   ` Joe Perches
  2014-05-15 23:07     ` Andrew Morton
  0 siblings, 1 reply; 5+ messages in thread
From: Joe Perches @ 2014-05-15 23:04 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Andy Whitcroft, LKML

On Thu, 2014-05-15 at 15:58 -0700, Andrew Morton wrote:
> On Tue, 13 May 2014 16:48:49 -0700 Joe Perches <joe@perches.com> wrote:
> > Protect against sizeof overflows by preferring
> > kmalloc_array and kcalloc to kmalloc/kzalloc
> > with a sizeof multiply.
[]
> > +# check for k[mz]alloc with multiplies that could be kmalloc_array/kcalloc
> > +		if ($^V && $^V ge 5.10.0 &&
> > +		    $line =~ /\b($Lval)\s*\=\s*(?:$balanced_parens)?\s*(k[mz]alloc)\s*\(\s*($FuncArg)\s*\*\s*($FuncArg)/) {
> > +			my $oldfunc = $3;
> > +			my $a1 = $4;
> > +			my $a2 = $10;
> > +			my $newfunc = "kmalloc_array";
> > +			$newfunc = "kcalloc" if ($oldfunc eq "kzalloc");
> > +			if ($a1 =~ /^sizeof\s*\S/ || $a2 =~ /^sizeof\s*\S/) {
> > +				CHK("ALLOC_WITH_MULTIPLY",
> > +				    "Prefer $newfunc over $oldfunc with multiply\n" . $herecurr);
> > +			}
> > +		}
> > +
> 
> Why hide this behind --strict?

Non-obvious CHK/--strict tests are less controversial.

The block above it
"prefer foo = alloc(sizeof(*foo)) over foo = alloc(sizeof(struct bar))"
used CHK so I copied it.

I've no objection to making it WARN instead,



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

* Re: [PATCH] checkpatch: Add --strict test for kmalloc/kzalloc with multiply
  2014-05-15 23:04   ` Joe Perches
@ 2014-05-15 23:07     ` Andrew Morton
  2014-05-15 23:27       ` [PATCH V2] checkpatch: Add warning " Joe Perches
  0 siblings, 1 reply; 5+ messages in thread
From: Andrew Morton @ 2014-05-15 23:07 UTC (permalink / raw)
  To: Joe Perches; +Cc: Andy Whitcroft, LKML

On Thu, 15 May 2014 16:04:46 -0700 Joe Perches <joe@perches.com> wrote:

> On Thu, 2014-05-15 at 15:58 -0700, Andrew Morton wrote:
> > On Tue, 13 May 2014 16:48:49 -0700 Joe Perches <joe@perches.com> wrote:
> > > Protect against sizeof overflows by preferring
> > > kmalloc_array and kcalloc to kmalloc/kzalloc
> > > with a sizeof multiply.
> []
> > > +# check for k[mz]alloc with multiplies that could be kmalloc_array/kcalloc
> > > +		if ($^V && $^V ge 5.10.0 &&
> > > +		    $line =~ /\b($Lval)\s*\=\s*(?:$balanced_parens)?\s*(k[mz]alloc)\s*\(\s*($FuncArg)\s*\*\s*($FuncArg)/) {
> > > +			my $oldfunc = $3;
> > > +			my $a1 = $4;
> > > +			my $a2 = $10;
> > > +			my $newfunc = "kmalloc_array";
> > > +			$newfunc = "kcalloc" if ($oldfunc eq "kzalloc");
> > > +			if ($a1 =~ /^sizeof\s*\S/ || $a2 =~ /^sizeof\s*\S/) {
> > > +				CHK("ALLOC_WITH_MULTIPLY",
> > > +				    "Prefer $newfunc over $oldfunc with multiply\n" . $herecurr);
> > > +			}
> > > +		}
> > > +
> > 
> > Why hide this behind --strict?
> 
> Non-obvious CHK/--strict tests are less controversial.
> 
> The block above it
> "prefer foo = alloc(sizeof(*foo)) over foo = alloc(sizeof(struct bar))"
> used CHK so I copied it.
> 
> I've no objection to making it WARN instead,

I'd prefer that - this is one of my regular comment-on-during-review
things.


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

* [PATCH V2] checkpatch: Add warning for kmalloc/kzalloc with multiply
  2014-05-15 23:07     ` Andrew Morton
@ 2014-05-15 23:27       ` Joe Perches
  0 siblings, 0 replies; 5+ messages in thread
From: Joe Perches @ 2014-05-15 23:27 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Andy Whitcroft, LKML

Protect against sizeof overflows by preferring
kmalloc_array/kcalloc over kmalloc/kzalloc
with a sizeof multiply.

Signed-off-by: Joe Perches <joe@perches.com>
---

V2: Make it a WARN instead.
    Add capability to --fix it too.

 scripts/checkpatch.pl | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index e7ff52a..7774025 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -4378,6 +4378,30 @@ sub process {
 			    "Prefer $3(sizeof(*$1)...) over $3($4...)\n" . $herecurr);
 		}
 
+# check for k[mz]alloc with multiplies that could be kmalloc_array/kcalloc
+		if ($^V && $^V ge 5.10.0 &&
+		    $line =~ /\b($Lval)\s*\=\s*(?:$balanced_parens)?\s*(k[mz]alloc)\s*\(\s*($FuncArg)\s*\*\s*($FuncArg)/) {
+			my $oldfunc = $3;
+			my $a1 = $4;
+			my $a2 = $10;
+			my $newfunc = "kmalloc_array";
+			$newfunc = "kcalloc" if ($oldfunc eq "kzalloc");
+			if ($a1 =~ /^sizeof\s*\S/ || $a2 =~ /^sizeof\s*\S/) {
+				if (WARN("ALLOC_WITH_MULTIPLY",
+					 "Prefer $newfunc over $oldfunc with multiply\n" . $herecurr) &&
+				    $fix) {
+					my $r1 = $a1;
+					my $r2 = $a2;
+					if ($a1 =~ /^sizeof\s*\S/) {
+						$r1 = $a2;
+						$r2 = $a1;
+					}
+					$fixed[$linenr - 1] =~ s/\b($Lval)\s*\=\s*(?:$balanced_parens)?\s*(k[mz]alloc)\s*\(\s*($FuncArg)\s*\*\s*($FuncArg)/$1 . ' = ' . "$newfunc(" . trim($r1) . ', ' . trim($r2)/e;
+
+				}
+			}
+		}
+
 # check for krealloc arg reuse
 		if ($^V && $^V ge 5.10.0 &&
 		    $line =~ /\b($Lval)\s*\=\s*(?:$balanced_parens)?\s*krealloc\s*\(\s*\1\s*,/) {



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

end of thread, other threads:[~2014-05-15 23:27 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-05-13 23:48 [PATCH] checkpatch: Add --strict test for kmalloc/kzalloc with multiply Joe Perches
2014-05-15 22:58 ` Andrew Morton
2014-05-15 23:04   ` Joe Perches
2014-05-15 23:07     ` Andrew Morton
2014-05-15 23:27       ` [PATCH V2] checkpatch: Add warning " Joe Perches

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®