linux-kernel - Re: [PATCH v7] checkpatch: warn for non-standard fixes tag style

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: <YyLtEza0VhSbflvH@oden.dyn.berto.se>
Date:   Thu, 15 Sep 2022 11:14:59 +0200
From:   Niklas Söderlund 
        <niklas.soderlund@...igine.com>
To:     Joe Perches <joe@...ches.com>
Cc:     Dwaipayan Ray <dwaipayanray1@...il.com>,
        Lukas Bulwahn <lukas.bulwahn@...il.com>,
        Jonathan Corbet <corbet@....net>,
        Andy Whitcroft <apw@...onical.com>, linux-doc@...r.kernel.org,
        linux-kernel@...r.kernel.org,
        Philippe Schenker <philippe.schenker@...adex.com>,
        Stephen Rothwell <sfr@...b.auug.org.au>,
        oss-drivers@...igine.com, Simon Horman <simon.horman@...igine.com>,
        Louis Peens <louis.peens@...igine.com>
Subject: Re: [PATCH v7] checkpatch: warn for non-standard fixes tag style

Hi Joe,

On 2022-09-14 09:09:25 -0700, Joe Perches wrote:
> On Wed, 2022-09-14 at 12:02 +0200, Niklas Söderlund wrote:
> > Add a warning for fixes tags that does not follow community conventions.
> []
> > * Changes since v6
> > - Update first check to make sure that there is a likely SHA1 of some
> >   minimum length after the fixes line.
> 
> https://lore.kernel.org/lkml/2febb7893346b6234983453de7c037536e479bfc.camel@perches.com/
> 
> The goal here should be to identify a line that looks like a commit
> reference.
> 
> So find lines that starts with 'fixes' and have a SHA1 commit id as
> broadly as reasonable.
> 
> Did you run the grep pattern and look at the results?
> 
> One grep pattern to verify the non canonical fixes format that
> are mistakenly used is:
> 
> $ git log --since=5-years-ago --no-merges --grep='^\s*fixes' -i --format=email -P | \
>   grep -P -i '^\s*fixes' | \
>   grep -P -v '^Fixes: [0-9a-f]{12,12}\s*\(".*")'
> 
> []
> 
> There are many different styles.
> Parenthesea are sometimes not used.

I understand this, and I did have a look at it.

> 
> > +			if ($line =~ /(\s*fixes:?)\s+([0-9a-f]{5,})\s+($balanced_parens)/i) {
> 
> How about some pattern like
> 
> 	/fixes\s*:?\s*(?:commit:?\s*)?[0-9a-f]{5,}/i
> 
> or maybe even more broadly:
> 
> 	/fixes\b.*\b[0-9a-f]{5,}\b/i

Maybe I misunderstand your comment, but this is what I do in this patch?

    if (!$in_header_lines &&
        $line =~ /^\s*fixes:?\s*(?:commit\s*)?[0-9a-f]{5,}\b/i) {

        ...

        if ($line =~ /(\s*fixes:?)\s+([0-9a-f]{5,})\s+($balanced_parens)/i) {
            ...
        }
    }

This will catch and warn about such tags but not attempt to break out 
it's components in order to suggest a potentially more correct fix. Is 
it this second filter you would like me to change?

-- 
Kind Regards,
Niklas Söderlund