Hi Alex, Using typeof(*p) for the existing k/kv object helpers makes sense. vmalloc_array() and vcalloc() take a count and a byte size, so their fix needs to retain sizeof(...).
I suggest handling the k/kv change separately. While testing this patch, I also found that the new vmalloc --fix path could copy masked strings or comments from checkpatch's parsed statement. The prepared v2 uses the original source instead; the regression tests cover those cases and check the existing k/kv behavior. Kees, would you prefer the typeof() change here or as a follow-up? Thanks, Iurii On Mon, 19 Oct 2026 09:19:48 +0200, Alejandro Colomar <[email protected]> wrote: > Hi Iurii, Kees, > > > Date: 2026-10-14 20:20:06-0400 > > From: Iurii Trukhin <[email protected]> > > > > vmalloc() and vzalloc() have 2-factor multiplication argument forms > > vmalloc_array() and vcalloc() in <linux/vmalloc.h> that protect against > > integer overflow during size calculation. > > > > Commit 070580b0b174 ("checkpatch: Suggest kmalloc_obj family for sizeof > > allocations") updated ALLOC_WITH_MULTIPLY for the (kv|k)[mz]alloc() > > family to suggest kmalloc_objs(type, count, gfp), while vmalloc_array() > > and vcalloc() retain the (count, size) signature. > > > > Extend ALLOC_WITH_MULTIPLY to warn when vmalloc() or vzalloc() is called > > with a 2-factor sizeof() multiplication and suggest vmalloc_array() or > > vcalloc() respectively, including --fix support. Also include the 'v' > > prefix in ALLOC_ARRAY_ARGS so swapped sizeof() first arguments to > > vmalloc_array() and vcalloc() are flagged. > > > > Link: https://github.com/KSPP/linux/issues/342 > > Signed-off-by: Iurii Trukhin <[email protected]> > > --- > > scripts/checkpatch.pl | 19 +++++++++++++------ > > 1 file changed, 13 insertions(+), 6 deletions(-) > > > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl > > index 8a7787d228a6..651a46f1fc40 100755 > > --- a/scripts/checkpatch.pl > > +++ b/scripts/checkpatch.pl > > @@ -7355,10 +7355,11 @@ sub process { > > } > > > > > > -# check for (kv|k)[mz]alloc with multiplies that could be > > kmalloc_objs/kvmalloc_objs/kzalloc_objs/kvzalloc_objs > > +# check for (kv|k|v)[mz]alloc with multiplies that could be > > +# > > kmalloc_objs/kvmalloc_objs/kzalloc_objs/kvzalloc_objs/vmalloc_array/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; > > @@ -7366,6 +7367,8 @@ sub process { > > $newfunc = "kvmalloc_objs" if ($oldfunc eq "kvmalloc"); > > $newfunc = "kvzalloc_objs" if ($oldfunc eq "kvzalloc"); > > $newfunc = "kzalloc_objs" if ($oldfunc eq "kzalloc"); > > + $newfunc = "vmalloc_array" if ($oldfunc eq "vmalloc"); > > + $newfunc = "vcalloc" if ($oldfunc eq "vzalloc"); > > my $r1 = $a1; > > my $r2 = $a2; > > if ($a1 =~ /^sizeof\s*\S/) { > > @@ -7381,9 +7384,13 @@ sub process { > > "Prefer $newfunc over $oldfunc with > > multiply\n" . $herectx) && > > $cnt == 1 && > > $fix) { > > - my $sized = trim($r2); > > - $sized =~ > > s/^sizeof\s*\S\(?([^\)]*)\)?$/$1/; > > - $fixed[$fixlinenr] =~ > > s/\b($Lval)\s*\=\s*(?:$balanced_parens)?\s*((?:kv|k)[mz]alloc)\s*\(\s*($FuncArg)\s*\*\s*($FuncArg)/$1 > > . ' = ' . "$newfunc(" . $sized . ', ' . trim($r1)/e; > > + if ($oldfunc =~ /^v/) { > > + $fixed[$fixlinenr] =~ > > s/\b($Lval)\s*\=\s*(?:$balanced_parens)?\s*(v[mz]alloc)\s*\(\s*($FuncArg)\s*\*\s*($FuncArg)/$1 > > . ' = ' . "$newfunc(" . trim($r1) . ', ' . trim($r2)/e; > > + } else { > > + my $sized = trim($r2); > > + $sized =~ > > s/^sizeof\s*\S\(?([^\)]*)\)?$/$1/; > > + $fixed[$fixlinenr] =~ > > s/\b($Lval)\s*\=\s*(?:$balanced_parens)?\s*((?:kv|k)[mz]alloc)\s*\(\s*($FuncArg)\s*\*\s*($FuncArg)/$1 > > . ' = ' . "$newfunc(" . $sized . ', ' . trim($r1)/e; > > + } > > Could the suggested fix add a typeof() to the first parameter? The only > reason (AFAIK) Kees avoided adding typeof() in the first parameter was > to minimize churn, but if someone is going to fix new code, it would be > better if they do it good. The good thing about using types explicitly > is that it's safer. > > That is, it should suggest > kmalloc_objs(typeof(*p), count, gfp) > instead of > kmalloc_objs(*p, count, gfp) > > > What do you think? > > > Have a lovely day! > Alex > > > } > > } > > } > > @@ -7397,7 +7404,7 @@ sub process { > > } > > > > # check for alloc argument mismatch > > - if ($line =~ > > /\b((?:devm_)?((?:k|kv)?(calloc|malloc_array)(?:_node)?))\s*\(\s*sizeof\b/) > > { > > + if ($line =~ > > /\b((?:devm_)?((?:k|kv|v)?(calloc|malloc_array)(?:_node)?))\s*\(\s*sizeof\b/) > > { > > WARN("ALLOC_ARRAY_ARGS", > > "$1 uses number as first arg, sizeof is generally > > wrong\n" . > > $herecurr); > > } > > -- > > 2.56.0 > > > > -- > <https://www.alejandro-colomar.es>
