linux-kernel - Re: [patch 2/2] scripts/spdxcheck.py: Handle special quotation mark comments

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <d9eb35b2594d9cfedd8525dc32a18b40c60451b0.camel@perches.com>
Date:   Sun, 20 Jan 2019 10:39:55 -0800
From:   Joe Perches <joe@...ches.com>
To:     Sven Eckelmann <sven@...fation.org>,
        Thomas Gleixner <tglx@...utronix.de>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        Kuninori Morimoto <kuninori.morimoto.gx@...esas.com>,
        Simon Horman <horms+renesas@...ge.net.au>,
        Yoshinori Sato <ysato@...rs.sourceforge.jp>,
        Rich Felker <dalias@...c.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Kate Stewart <kstewart@...uxfoundation.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Jonathan Corbet <corbet@....net>,
        b.a.t.m.a.n@...ts.open-mesh.org
Subject: Re: [patch 2/2] scripts/spdxcheck.py: Handle special quotation mark
 comments

On Sun, 2019-01-20 at 16:32 +0100, Sven Eckelmann wrote:
> On Wednesday, 16 January 2019 11.26.53 CET Thomas Gleixner wrote:
> > The SuperH boot code files use a magic format for the SPDX identifier
> > comment:
> > 
> >   LIST "SPDX-License-Identifier: .... "
> > 
> > The trailing quotation mark is not stripped before the token parser is
> > invoked and causes the scan to fail. Handle it gracefully.
> [...]
> 
> This patch introduces a false positive when checking files with an ANSI-C 
> style /* SPDX-License-Identifier: .... */ comment line.
> 
>     $ ./scripts/checkpatch.pl -q -f include/linux/bug.h
>     WARNING: 'SPDX-License-Identifier: GPL-2.0 */' is not supported in LICENSES/...
>     #1: FILE: include/linux/bug.h:1:
>     +/* SPDX-License-Identifier: GPL-2.0 */
>     
>     total: 0 errors, 1 warnings, 79 lines checked
> 
> checkpatch.pl is already stripping the "/* " prefix and only sends the 
> remaining "SPDX-License-Identifier: GPL-2.0 */" via stdin to
> scripts/spdxcheck.py. Thus the newly introduced check
> 
> > +                # Remove trailing comment closure
> > +                if line.startswith('/*'):
> > +                    expr = expr.rstrip('*/').strip()
> 
> doesn't match and thus the code doesn't remove the " */" at the end of the 
> line.

Well, maybe checkpatch should remove the comment trailer.
---
Miscellanea:

o Indent a block properly too

 scripts/checkpatch.pl | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 155fa9305166..e0b542008256 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3029,8 +3029,10 @@ sub process {
 				$checklicenseline = 2;
 			} elsif ($rawline =~ /^\+/) {
 				my $comment = "";
+				my $trailer = "";
 				if ($realfile =~ /\.(h|s|S)$/) {
 					$comment = '/*';
+					$trailer = '*/';
 				} elsif ($realfile =~ /\.(c|dts|dtsi)$/) {
 					$comment = '//';
 				} elsif (($checklicenseline == 2) || $realfile =~ /\.(sh|pl|py|awk|tc)$/) {
@@ -3044,11 +3046,12 @@ sub process {
 					 WARN("SPDX_LICENSE_TAG",
 					      "Missing or malformed SPDX-License-Identifier tag in line $checklicenseline\n" . $herecurr);
 				} elsif ($rawline =~ /(SPDX-License-Identifier: .*)/) {
-					 my $spdx_license = $1;
-					 if (!is_SPDX_License_valid($spdx_license)) {
-						  WARN("SPDX_LICENSE_TAG",
-						       "'$spdx_license' is not supported in LICENSES/...\n" . $herecurr);
-					 }
+					my $spdx_license = rtrim($1);
+					$spdx_license =~ s/\s*\Q$trailer\E$//;
+					if (!is_SPDX_License_valid($spdx_license)) {
+						WARN("SPDX_LICENSE_TAG",
+						     "'$spdx_license' is not supported in LICENSES/...\n" . $herecurr);
+					}
 				}
 			}
 		}