linux-kernel - Re: [PATCH 2/2] checkpatch: warn when Reported-by: is not followed by Link:

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]

Date:   Fri, 9 Dec 2022 09:33:04 +0100
From:   Thorsten Leemhuis <linux@...mhuis.info>
To:     Joe Perches <joe@...ches.com>,
        Kai Wasserbäch <kai@....carbon-project.org>,
        linux-kernel@...r.kernel.org
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        Andy Whitcroft <apw@...onical.com>,
        Dwaipayan Ray <dwaipayanray1@...il.com>,
        Lukas Bulwahn <lukas.bulwahn@...il.com>
Subject: Re: [PATCH 2/2] checkpatch: warn when Reported-by: is not followed by
 Link:

On 08.12.22 22:34, Joe Perches wrote:
> On Thu, 2022-12-08 at 22:11 +0100, Thorsten Leemhuis wrote:
>> Joe, many thx for your time and your valuable feedback. I agree with
>> most of it and will look into addressing it tomorrow, but there is one
>> area where I have a different option:
>>
>> On 08.12.22 21:21, Joe Perches wrote:
>>> On Thu, 2022-12-08 at 20:32 +0100, Kai Wasserbäch wrote:
>>>> +
>>>> +			# check if Reported-by: is followed by a Link:
>>>> +			if ($sign_off =~ /^reported-by:$/i) {
>>>> +				if (!defined $lines[$linenr]) {
>>>> +					WARN("BAD_REPORTED_BY_LINK",
>>>> +					     "Reported-by: should be immediately followed by Link: to the report\n" . "$here\n" . $rawline);
>>>> +				} elsif ($rawlines[$linenr] !~ /^\s*link:\s*(.*)/i) {
>>>> +					WARN("BAD_REPORTED_BY_LINK",
>>>> +					     "Reported-by: should be immediately followed by Link: to the report\n" . "$here\n" . $rawline . "\n" .$rawlines[$linenr]);
>>>> +				} elsif ($lines[$linenr] !~ /https?:\/\//i) {
>>>> +					WARN("BAD_REPORTED_BY_LINK",
>>>> +					     "Link: following Reported-by: should contain an URL\n" . "$here\n" . $rawline . "\n" .$rawlines[$linenr]);
>>>
>>> [...]
>>>
>>> Also the actual link line should likely be from lore
>>>
>>> So maybe: 
>>> 				} elsif ($lines[$linenr] !~ m{https?://lore.kernel.org/.+}i) {
>>> 					WARN("BAD_REPORTED_BY_LINK",
>>> 					     "Link: following Reported-by: should use a lore.kernel.org URL\n" . $herecurr . $rawlines[$linenr]);
>>
>> I'm pretty sure that's not want we want, as from regression tracking I
>> known that we have other links that should continue to work here. Links
>> to bugzilla.kernel.org immediately come to my mind, for example. Then
>> there are some subsystems that use issue trackers on github or in gitlab
>> instances (DRM for example), which must work here, too; and we
>> occasionally even have links to bugs in bugzilla instances from RH or
>> SUSE there, as sometimes valuable details for code archeologists can be
>> found there.
> 
> Outside the fact there are relatively few existing uses of Link: after
> Reported-by:,

Well, sure, because many people forgot to place them -- and this patch
is trying to change that for reasons outlined in the commit description.

> it appears that "Fixes:" should also be allowed.

Well, that would defeat the purpose of this patch -- and there is no
need to, as developers can still put Fixes: tag above or below the combo
of Reported-by: and Link:.

> and lore definitely dominates the Link: uses.
> 
> And IMO: the lkml.kernel.org entries should instead use lore.

Well, that might be something nice to have (if Konstantin thinks it's
worth the trouble, as lkml.kernel.org likely needs to stick around
anyway to not break existing links). But that IMHO is something
independent of this patch-set, because the proper solution afaics then
would be to check all Link: tags in the commit message, not only those
next to a Reported-by.

Ciao, Thorsten