linux-kernel - Re: Fw: [PATCH 0/2] feat: checkpatch: prohibit Buglink: and warn about missing 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]

Message-ID: <23a61dd072ee1d2cc5b54281b0a9dc13e01aa0b8.camel@perches.com>
Date:   Mon, 05 Dec 2022 21:54:13 -0800
From:   Joe Perches <joe@...ches.com>
To:     Thorsten Leemhuis <linux@...mhuis.info>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Kai Wasserbäch <kai@....carbon-project.org>
Subject: Re: Fw: [PATCH 0/2] feat: checkpatch: prohibit Buglink: and warn
 about missing Link:

On Tue, 2022-12-06 at 05:55 +0100, Thorsten Leemhuis wrote:
> Hi Joe! Many thx for looking into this.
> 
> On 06.12.22 02:02, Joe Perches wrote:
> > On Mon, 2022-12-05 at 13:14 -0800, Andrew Morton wrote:
> > > Begin forwarded message:
> > > 
> > > Date: Sun,  4 Dec 2022 12:33:37 +0100
> > > From: Kai Wasserbäch <kai@....carbon-project.org>
> > > To: linux-kernel@...r.kernel.org
> > > Cc: Andrew Morton <akpm@...ux-foundation.org>, Thorsten Leemhuis <linux@...mhuis.info>
> > > Subject: [PATCH 0/2] feat: checkpatch: prohibit Buglink: and warn about missing Link:
> > > 
> > > Thorsten Leemhuis suggested the following two changes to checkpatch, which
> > > I hereby humbly submit for review. Please let me know if any changes
> > > would be required for acceptance.
> > [...]
> > > Suggested-by: Thorsten Leemhuis <linux@...mhuis.info>
> > > Signed-off-by: Kai Wasserbäch <kai@....carbon-project.org>
> > > 
> > > Kai Wasserbäch (2):
> > >   feat: checkpatch: error on usage of a Buglink tag in the commit log
> > 
> > Why, what's wrong with a buglink reference?
> 
> Long story short: Linus doesn't like it:
> 
> ```
> > > BugLink: https://lore.kernel.org/r/20220610205038.GA3050413@paulmck-ThinkPad-P17-Gen-1
> > > BugLink: https://lore.kernel.org/r/CAMdYzYpF4FNTBPZsEFeWRuEwSies36QM_As8osPWZSr2q-viEA@mail.gmail.com
> > [...]
> > please stop making up random tags that make no sense.
> > 
> > Just use "Link:"
> > [...]
> > 
> > It's extra context to the commit, in case somebody wants to know the
> > history. The "bug" part is (and always should be) already explained in
> > the commit message, there's absolutely no point in adding soem extra
> > noise to the "Link:" tag.>
> ```
> That's a quote from
> https://lore.kernel.org/all/CAHk-=wgs38ZrfPvy=nOwVkVzjpM3VFU1zobP37Fwd_h9iAD5JQ@mail.gmail.com/
> 
> In that message he also links to another mail from him, where he wrote:
> ```
> > I think "BugLink:" is silly, because that's just a regular link.
> > What's the upside of saying "Bug" in there? If you're fixing a bug,
> > and are linking to reports and discussions by people, what does that
> > "bug" buy you apart from another ugly CamelCase thing?
> > 
> > In other words, in "BugLink:", neither "Bug" nor "Link" is actually meaningful.
> > 
> > The current "Link:" tag exists AS A GENERIC WAY TO SAY "look, here's
> > more information". That's the point. The word "Link" has no other
> > meaning, and trying to then combine it with other things is worthless.
> ```
> https://lore.kernel.org/all/CAHk-=wgzRUT1fBpuz3xcN+YdsX0SxqOzHWRtj0ReHpUBb5TKbA@mail.gmail.com/
> 
> Our docs say to use Link in this case, too; see
> Documentation/process/submitting-patches.rst
> (http://docs.kernel.org/process/submitting-patches.html) and
> Documentation/process/5.Posting.rst
> (https://docs.kernel.org/process/5.Posting.html)
> 
> And using various tags for the same thing makes it also harder for
> external scripts that look at commits to take further action -- like the
> regression tracking bot I write and use for my work.
> 
> All of that obviously should have been in patch description. Let me
> resubmit that patch with all of that in there, or are you dead against
> this idea now?

No, better commit description would be useful and perhaps a more
generic, "is the thing in front of a URI/URL" a known/supported entry,
instead of using an known invalid test would be a better mechanism.

> > >   feat: checkpatch: Warn about Reported-by: not being followed by a
> > >     Link:
> > 
> > I think this is unnecessary.
> 
> I expect this to cause more discussion, which is why this should be in a
> separate submission. But in the end the reasons are similar as above:
> (1) Linus really want to see those links to make things easier for
> future code archeologists. (2) My regression tracking efforts heavily
> rely on those Links, as they allow to automatically connect reports with
> patches and commits to fix the reported regression; without those the
> tracking is a PITA and doesn't scale.
> 
> Together that IMHO is strong enough reason to warn in this case.

Maybe, I think there might be some value in not intermixing signature
tags and other tags though.