linux-kernel - Re: [PATCH V3 03/16] platform/x86/intel/vsec: Use cleanup.h

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <d3ca8db7c045909c9fff6bc3e96a7b8de1b05f8d.camel@perches.com>
Date:   Mon, 23 Oct 2023 22:15:55 -0700
From:   Joe Perches <joe@...ches.com>
To:     Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>,
        Andy Whitcroft <apw@...onical.com>,
        Dwaipayan Ray <dwaipayanray1@...il.com>,
        Lukas Bulwahn <lukas.bulwahn@...il.com>
Cc:     kernel test robot <lkp@...el.com>,
        LKML <linux-kernel@...r.kernel.org>,
        platform-driver-x86@...r.kernel.org, rajvi.jingar@...ux.intel.com,
        oe-kbuild-all@...ts.linux.dev,
        "David E. Box" <david.e.box@...ux.intel.com>
Subject: Re: [PATCH V3 03/16] platform/x86/intel/vsec: Use cleanup.h

On Fri, 2023-10-13 at 11:14 -0700, Joe Perches wrote:
> On Fri, 2023-10-13 at 13:39 +0300, Ilpo Järvinen wrote:
> > Hi,
> > 
> > I added checkpatch people, please check what looks a false positive below.
> 
> Yeah, thanks I guess.
> The __free uses are very new.
> I'll play around with adding it to $Attributes
> and see what happens.
> 
> 
> > On Thu, 12 Oct 2023, David E. Box wrote:
> > > On Thu, 2023-10-12 at 13:25 +0800, kernel test robot wrote:
> > > > kernel test robot noticed the following build warnings:
> > > > # many are suggestions rather than must-fix
> []
> > > > ERROR:SPACING: need consistent spacing around '*' (ctx:WxV)
> > > > #31: FILE: drivers/platform/x86/intel/vsec.c:159:
> > > > +       struct intel_vsec_device __free(kfree) *intel_vsec_dev = NULL;
> > > .
> > > These looks like false positives.
> > 
> > I agree. If I interpret the error message right checkpatch seems to think
> > that's a multiplication which is not the case here.

So __free is an odd $Attribute as it takes an argument.
I've added it along with the other odd $Attribute __alloc_size
to another variable called $ArgAttribute and $Attribute.

Oddly, I don't understand why perl does not perform
the elimination using

	$foo =~ s/\b$ArgAttribute\b//g;

but does do the elimination using

	$foo =~ s/\b$ArgAttribute//g;

maybe something to do with the closing parenthesis in the match.

So now there is a separate substitution for this in a test.

Comments?

---
 scripts/checkpatch.pl | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 25fdb7fda1128..501383fa31c55 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -488,9 +488,14 @@ our $InitAttributeConst = qr{$InitAttributePrefix(?:initconst\b)};
 our $InitAttributeInit = qr{$InitAttributePrefix(?:init\b)};
 our $InitAttribute = qr{$InitAttributeData|$InitAttributeConst|$InitAttributeInit};
 
+our $ArgAttribute = qr{(?x:
+			__alloc_size\s*\(\s*\d+\s*(?:,\s*\d+\s*)?\)|
+			__free\s*\(\s*\w+\s*\)
+)};
+
 # Notes to $Attribute:
 # We need \b after 'init' otherwise 'initconst' will cause a false positive in a check
-our $Attribute	= qr{
+our $Attribute	= qr{(?x:
 			const|
 			volatile|
 			__percpu|
@@ -516,8 +521,8 @@ our $Attribute	= qr{
 			____cacheline_aligned_in_smp|
 			____cacheline_internodealigned_in_smp|
 			__weak|
-			__alloc_size\s*\(\s*\d+\s*(?:,\s*\d+\s*)?\)
-		  }x;
+			$ArgAttribute
+		  )};
 our $Modifier;
 our $Inline	= qr{inline|__always_inline|noinline|__inline|__inline__};
 our $Member	= qr{->$Ident|\.$Ident|\[[^]]*\]};
@@ -4091,6 +4096,9 @@ sub process {
 			# remove $Attribute/$Sparse uses to simplify comparisons
 			$sl =~ s/\b(?:$Attribute|$Sparse)\b//g;
 			$pl =~ s/\b(?:$Attribute|$Sparse)\b//g;
+			$sl =~ s/\b(?:$ArgAttribute)//g;
+			$pl =~ s/\b(?:$ArgAttribute)//g;
+
 			if (($pl =~ /^\+\s+$Declare\s*$Ident\s*[=,;:\[]/ ||
 			# function pointer declarations
 			     $pl =~ /^\+\s+$Declare\s*\(\s*\*\s*$Ident\s*\)\s*[=,;:\[\(]/ ||