* [PATCH][next] checkpatch: add a couple new alloc functions to alloc with multiplies check
@ 2023-09-12 17:04 Gustavo A. R. Silva
2023-09-12 17:51 ` Joe Perches
0 siblings, 1 reply; 3+ messages in thread
From: Gustavo A. R. Silva @ 2023-09-12 17:04 UTC (permalink / raw)
To: Andy Whitcroft, Joe Perches, Dwaipayan Ray, Lukas Bulwahn
Cc: linux-kernel, Gustavo A. R. Silva, linux-hardening
vmalloc() and vzalloc() functions have now 2-factor multiplication
argument forms vmalloc_array() and vcalloc(), correspondingly.
Add alloc-with-multiplies checks for these new functions.
Link: https://github.com/KSPP/linux/issues/342
Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
---
scripts/checkpatch.pl | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 7d16f863edf1..45265d0eee1b 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -7207,17 +7207,19 @@ sub process {
"Prefer $3(sizeof(*$1)...) over $3($4...)\n" . $herecurr);
}
-# check for (kv|k)[mz]alloc with multiplies that could be kmalloc_array/kvmalloc_array/kvcalloc/kcalloc
+# check for (kv|k|v)[mz]alloc with multiplies that could be kmalloc_array/kvmalloc_array/vmalloc_array/kvcalloc/kcalloc/vcalloc
if ($perl_version_ok &&
defined $stat &&
- $stat =~ /^\+\s*($Lval)\s*\=\s*(?:$balanced_parens)?\s*((?:kv|k)[mz]alloc)\s*\(\s*($FuncArg)\s*\*\s*($FuncArg)\s*,/) {
+ $stat =~ /^\+\s*($Lval)\s*\=\s*(?:$balanced_parens)?\s*((?:kv|k|v)[mz]alloc)\s*\(\s*($FuncArg)\s*\*\s*($FuncArg)\s*,?/) {
my $oldfunc = $3;
my $a1 = $4;
my $a2 = $10;
my $newfunc = "kmalloc_array";
$newfunc = "kvmalloc_array" if ($oldfunc eq "kvmalloc");
+ $newfunc = "vmalloc_array" if ($oldfunc eq "vmalloc");
$newfunc = "kvcalloc" if ($oldfunc eq "kvzalloc");
$newfunc = "kcalloc" if ($oldfunc eq "kzalloc");
+ $newfunc = "vcalloc" if ($oldfunc eq "vzalloc");
my $r1 = $a1;
my $r2 = $a2;
if ($a1 =~ /^sizeof\s*\S/) {
@@ -7233,7 +7235,7 @@ sub process {
"Prefer $newfunc over $oldfunc with multiply\n" . $herectx) &&
$cnt == 1 &&
$fix) {
- $fixed[$fixlinenr] =~ s/\b($Lval)\s*\=\s*(?:$balanced_parens)?\s*((?:kv|k)[mz]alloc)\s*\(\s*($FuncArg)\s*\*\s*($FuncArg)/$1 . ' = ' . "$newfunc(" . trim($r1) . ', ' . trim($r2)/e;
+ $fixed[$fixlinenr] =~ s/\b($Lval)\s*\=\s*(?:$balanced_parens)?\s*((?:kv|k|v)[mz]alloc)\s*\(\s*($FuncArg)\s*\*\s*($FuncArg)/$1 . ' = ' . "$newfunc(" . trim($r1) . ', ' . trim($r2)/e;
}
}
}
--
2.34.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH][next] checkpatch: add a couple new alloc functions to alloc with multiplies check
2023-09-12 17:04 [PATCH][next] checkpatch: add a couple new alloc functions to alloc with multiplies check Gustavo A. R. Silva
@ 2023-09-12 17:51 ` Joe Perches
2023-09-15 3:31 ` Kees Cook
0 siblings, 1 reply; 3+ messages in thread
From: Joe Perches @ 2023-09-12 17:51 UTC (permalink / raw)
To: Gustavo A. R. Silva, Andy Whitcroft, Dwaipayan Ray, Lukas Bulwahn
Cc: linux-kernel, linux-hardening
On Tue, 2023-09-12 at 11:04 -0600, Gustavo A. R. Silva wrote:
> vmalloc() and vzalloc() functions have now 2-factor multiplication
> argument forms vmalloc_array() and vcalloc(), correspondingly.
> Add alloc-with-multiplies checks for these new functions.
>
> Link: https://github.com/KSPP/linux/issues/342
> Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
> ---
> scripts/checkpatch.pl | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 7d16f863edf1..45265d0eee1b 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -7207,17 +7207,19 @@ sub process {
> "Prefer $3(sizeof(*$1)...) over $3($4...)\n" . $herecurr);
> }
>
> -# check for (kv|k)[mz]alloc with multiplies that could be kmalloc_array/kvmalloc_array/kvcalloc/kcalloc
> +# check for (kv|k|v)[mz]alloc with multiplies that could be kmalloc_array/kvmalloc_array/vmalloc_array/kvcalloc/kcalloc/vcalloc
> if ($perl_version_ok &&
> defined $stat &&
> - $stat =~ /^\+\s*($Lval)\s*\=\s*(?:$balanced_parens)?\s*((?:kv|k)[mz]alloc)\s*\(\s*($FuncArg)\s*\*\s*($FuncArg)\s*,/) {
> + $stat =~ /^\+\s*($Lval)\s*\=\s*(?:$balanced_parens)?\s*((?:kv|k|v)[mz]alloc)\s*\(\s*($FuncArg)\s*\*\s*($FuncArg)\s*,?/) {
> my $oldfunc = $3;
> my $a1 = $4;
> my $a2 = $10;
> my $newfunc = "kmalloc_array";
> $newfunc = "kvmalloc_array" if ($oldfunc eq "kvmalloc");
> + $newfunc = "vmalloc_array" if ($oldfunc eq "vmalloc");
> $newfunc = "kvcalloc" if ($oldfunc eq "kvzalloc");
> $newfunc = "kcalloc" if ($oldfunc eq "kzalloc");
> + $newfunc = "vcalloc" if ($oldfunc eq "vzalloc");
> my $r1 = $a1;
> my $r2 = $a2;
> if ($a1 =~ /^sizeof\s*\S/) {
> @@ -7233,7 +7235,7 @@ sub process {
> "Prefer $newfunc over $oldfunc with multiply\n" . $herectx) &&
> $cnt == 1 &&
> $fix) {
> - $fixed[$fixlinenr] =~ s/\b($Lval)\s*\=\s*(?:$balanced_parens)?\s*((?:kv|k)[mz]alloc)\s*\(\s*($FuncArg)\s*\*\s*($FuncArg)/$1 . ' = ' . "$newfunc(" . trim($r1) . ', ' . trim($r2)/e;
> + $fixed[$fixlinenr] =~ s/\b($Lval)\s*\=\s*(?:$balanced_parens)?\s*((?:kv|k|v)[mz]alloc)\s*\(\s*($FuncArg)\s*\*\s*($FuncArg)/$1 . ' = ' . "$newfunc(" . trim($r1) . ', ' . trim($r2)/e;
> }
> }
> }
Seems ok. Dunno how many more of these there might be like
devm_ variants, but maybe it'd be better style to use a hash
with replacement instead of the repeated if block
Something like:
---
scripts/checkpatch.pl | 26 ++++++++++++++++++++------
1 file changed, 20 insertions(+), 6 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 7d16f863edf1c..bacb63518be14 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -854,6 +854,23 @@ foreach my $entry (keys %deprecated_apis) {
}
$deprecated_apis_search = "(?:${deprecated_apis_search})";
+our %alloc_with_multiply_apis = (
+ "kmalloc" => "kmalloc_array",
+ "kvmalloc" => "kvmalloc_array",
+ "vmalloc" => "vmalloc_array",
+ "kvzalloc" => "kvcalloc",
+ "kzalloc" => "kcalloc",
+ "vzalloc" => "vcalloc",
+);
+
+#Create a search pattern for all these strings to speed up a loop below
+our $alloc_with_multiply_search = "";
+foreach my $entry (keys %alloc_with_multiply_apis) {
+ $alloc_with_multiply_search .= '|' if ($alloc_with_multiply_search ne "");
+ $alloc_with_multiply_search .= $entry;
+}
+$alloc_with_multiply_search = "(?:${alloc_with_multiply_search})";
+
our $mode_perms_world_writable = qr{
S_IWUGO |
S_IWOTH |
@@ -7210,14 +7227,11 @@ sub process {
# check for (kv|k)[mz]alloc with multiplies that could be kmalloc_array/kvmalloc_array/kvcalloc/kcalloc
if ($perl_version_ok &&
defined $stat &&
- $stat =~ /^\+\s*($Lval)\s*\=\s*(?:$balanced_parens)?\s*((?:kv|k)[mz]alloc)\s*\(\s*($FuncArg)\s*\*\s*($FuncArg)\s*,/) {
+ $stat =~ /^\+\s*($Lval)\s*\=\s*(?:$balanced_parens)?\s*($alloc_with_multiply_search)\s*\(\s*($FuncArg)\s*\*\s*($FuncArg)\s*,/) {
my $oldfunc = $3;
+ my $newfunc = $alloc_with_multiply_apis{$oldfunc};
my $a1 = $4;
my $a2 = $10;
- my $newfunc = "kmalloc_array";
- $newfunc = "kvmalloc_array" if ($oldfunc eq "kvmalloc");
- $newfunc = "kvcalloc" if ($oldfunc eq "kvzalloc");
- $newfunc = "kcalloc" if ($oldfunc eq "kzalloc");
my $r1 = $a1;
my $r2 = $a2;
if ($a1 =~ /^sizeof\s*\S/) {
@@ -7233,7 +7247,7 @@ sub process {
"Prefer $newfunc over $oldfunc with multiply\n" . $herectx) &&
$cnt == 1 &&
$fix) {
- $fixed[$fixlinenr] =~ s/\b($Lval)\s*\=\s*(?:$balanced_parens)?\s*((?:kv|k)[mz]alloc)\s*\(\s*($FuncArg)\s*\*\s*($FuncArg)/$1 . ' = ' . "$newfunc(" . trim($r1) . ', ' . trim($r2)/e;
+ $fixed[$fixlinenr] =~ s/\b($Lval)\s*\=\s*(?:$balanced_parens)?\s*($alloc_with_multiply_search)\s*\(\s*($FuncArg)\s*\*\s*($FuncArg)/$1 . ' = ' . "$newfunc(" . trim($r1) . ', ' . trim($r2)/e;
}
}
}
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH][next] checkpatch: add a couple new alloc functions to alloc with multiplies check
2023-09-12 17:51 ` Joe Perches
@ 2023-09-15 3:31 ` Kees Cook
0 siblings, 0 replies; 3+ messages in thread
From: Kees Cook @ 2023-09-15 3:31 UTC (permalink / raw)
To: Joe Perches
Cc: Gustavo A. R. Silva, Andy Whitcroft, Dwaipayan Ray, Lukas Bulwahn,
linux-kernel, linux-hardening
On Tue, Sep 12, 2023 at 10:51:22AM -0700, Joe Perches wrote:
> On Tue, 2023-09-12 at 11:04 -0600, Gustavo A. R. Silva wrote:
> > vmalloc() and vzalloc() functions have now 2-factor multiplication
> > argument forms vmalloc_array() and vcalloc(), correspondingly.
>
> > Add alloc-with-multiplies checks for these new functions.
> >
> > Link: https://github.com/KSPP/linux/issues/342
> > Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
> > ---
> > scripts/checkpatch.pl | 8 +++++---
> > 1 file changed, 5 insertions(+), 3 deletions(-)
> >
> > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > index 7d16f863edf1..45265d0eee1b 100755
> > --- a/scripts/checkpatch.pl
> > +++ b/scripts/checkpatch.pl
> > @@ -7207,17 +7207,19 @@ sub process {
> > "Prefer $3(sizeof(*$1)...) over $3($4...)\n" . $herecurr);
> > }
> >
> > -# check for (kv|k)[mz]alloc with multiplies that could be kmalloc_array/kvmalloc_array/kvcalloc/kcalloc
> > +# check for (kv|k|v)[mz]alloc with multiplies that could be kmalloc_array/kvmalloc_array/vmalloc_array/kvcalloc/kcalloc/vcalloc
> > if ($perl_version_ok &&
> > defined $stat &&
> > - $stat =~ /^\+\s*($Lval)\s*\=\s*(?:$balanced_parens)?\s*((?:kv|k)[mz]alloc)\s*\(\s*($FuncArg)\s*\*\s*($FuncArg)\s*,/) {
> > + $stat =~ /^\+\s*($Lval)\s*\=\s*(?:$balanced_parens)?\s*((?:kv|k|v)[mz]alloc)\s*\(\s*($FuncArg)\s*\*\s*($FuncArg)\s*,?/) {
> > my $oldfunc = $3;
> > my $a1 = $4;
> > my $a2 = $10;
> > my $newfunc = "kmalloc_array";
> > $newfunc = "kvmalloc_array" if ($oldfunc eq "kvmalloc");
> > + $newfunc = "vmalloc_array" if ($oldfunc eq "vmalloc");
> > $newfunc = "kvcalloc" if ($oldfunc eq "kvzalloc");
> > $newfunc = "kcalloc" if ($oldfunc eq "kzalloc");
> > + $newfunc = "vcalloc" if ($oldfunc eq "vzalloc");
> > my $r1 = $a1;
> > my $r2 = $a2;
> > if ($a1 =~ /^sizeof\s*\S/) {
> > @@ -7233,7 +7235,7 @@ sub process {
> > "Prefer $newfunc over $oldfunc with multiply\n" . $herectx) &&
> > $cnt == 1 &&
> > $fix) {
> > - $fixed[$fixlinenr] =~ s/\b($Lval)\s*\=\s*(?:$balanced_parens)?\s*((?:kv|k)[mz]alloc)\s*\(\s*($FuncArg)\s*\*\s*($FuncArg)/$1 . ' = ' . "$newfunc(" . trim($r1) . ', ' . trim($r2)/e;
> > + $fixed[$fixlinenr] =~ s/\b($Lval)\s*\=\s*(?:$balanced_parens)?\s*((?:kv|k|v)[mz]alloc)\s*\(\s*($FuncArg)\s*\*\s*($FuncArg)/$1 . ' = ' . "$newfunc(" . trim($r1) . ', ' . trim($r2)/e;
> > }
> > }
> > }
>
> Seems ok. Dunno how many more of these there might be like
> devm_ variants, but maybe it'd be better style to use a hash
> with replacement instead of the repeated if block
>
> Something like:
> ---
> scripts/checkpatch.pl | 26 ++++++++++++++++++++------
> 1 file changed, 20 insertions(+), 6 deletions(-)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 7d16f863edf1c..bacb63518be14 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -854,6 +854,23 @@ foreach my $entry (keys %deprecated_apis) {
> }
> $deprecated_apis_search = "(?:${deprecated_apis_search})";
>
> +our %alloc_with_multiply_apis = (
> + "kmalloc" => "kmalloc_array",
> + "kvmalloc" => "kvmalloc_array",
> + "vmalloc" => "vmalloc_array",
> + "kvzalloc" => "kvcalloc",
> + "kzalloc" => "kcalloc",
> + "vzalloc" => "vcalloc",
> +);
Yeah, this seems more readable. Gustavo can you send a v2 with this
rework?
Thanks!
-Kees
> +
> +#Create a search pattern for all these strings to speed up a loop below
> +our $alloc_with_multiply_search = "";
> +foreach my $entry (keys %alloc_with_multiply_apis) {
> + $alloc_with_multiply_search .= '|' if ($alloc_with_multiply_search ne "");
> + $alloc_with_multiply_search .= $entry;
> +}
> +$alloc_with_multiply_search = "(?:${alloc_with_multiply_search})";
> +
> our $mode_perms_world_writable = qr{
> S_IWUGO |
> S_IWOTH |
> @@ -7210,14 +7227,11 @@ sub process {
> # check for (kv|k)[mz]alloc with multiplies that could be kmalloc_array/kvmalloc_array/kvcalloc/kcalloc
> if ($perl_version_ok &&
> defined $stat &&
> - $stat =~ /^\+\s*($Lval)\s*\=\s*(?:$balanced_parens)?\s*((?:kv|k)[mz]alloc)\s*\(\s*($FuncArg)\s*\*\s*($FuncArg)\s*,/) {
> + $stat =~ /^\+\s*($Lval)\s*\=\s*(?:$balanced_parens)?\s*($alloc_with_multiply_search)\s*\(\s*($FuncArg)\s*\*\s*($FuncArg)\s*,/) {
> my $oldfunc = $3;
> + my $newfunc = $alloc_with_multiply_apis{$oldfunc};
> my $a1 = $4;
> my $a2 = $10;
> - my $newfunc = "kmalloc_array";
> - $newfunc = "kvmalloc_array" if ($oldfunc eq "kvmalloc");
> - $newfunc = "kvcalloc" if ($oldfunc eq "kvzalloc");
> - $newfunc = "kcalloc" if ($oldfunc eq "kzalloc");
> my $r1 = $a1;
> my $r2 = $a2;
> if ($a1 =~ /^sizeof\s*\S/) {
> @@ -7233,7 +7247,7 @@ sub process {
> "Prefer $newfunc over $oldfunc with multiply\n" . $herectx) &&
> $cnt == 1 &&
> $fix) {
> - $fixed[$fixlinenr] =~ s/\b($Lval)\s*\=\s*(?:$balanced_parens)?\s*((?:kv|k)[mz]alloc)\s*\(\s*($FuncArg)\s*\*\s*($FuncArg)/$1 . ' = ' . "$newfunc(" . trim($r1) . ', ' . trim($r2)/e;
> + $fixed[$fixlinenr] =~ s/\b($Lval)\s*\=\s*(?:$balanced_parens)?\s*($alloc_with_multiply_search)\s*\(\s*($FuncArg)\s*\*\s*($FuncArg)/$1 . ' = ' . "$newfunc(" . trim($r1) . ', ' . trim($r2)/e;
> }
> }
> }
>
--
Kees Cook
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-09-15 3:31 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-09-12 17:04 [PATCH][next] checkpatch: add a couple new alloc functions to alloc with multiplies check Gustavo A. R. Silva
2023-09-12 17:51 ` Joe Perches
2023-09-15 3:31 ` Kees Cook
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox