Re: [PATCH] checkpatch: add vmalloc() and vzalloc() to alloc with multiplies check

[Iurii Trukhin]

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
<alx+linux-hardening@xxxxxxxxxx> wrote:
> Hi Iurii, Kees,
>
> > Date: 2026-10-14 20:20:06-0400
> > From: Iurii Trukhin <yuri@xxxxxxxxxxx>
> >
> > 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 <yuri@xxxxxxxxxxx>
> > ---
> >  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>