linux-kernel - Re: [PATCH v1 1/1] docs: process: submitting-patches: Clarify the Reported-by usage

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]

Date:   Thu, 3 Mar 2022 16:51:40 +0300
From:   Dan Carpenter <dan.carpenter@...cle.com>
To:     Johan Hovold <johan@...nel.org>
Cc:     Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Matthew Wilcox <willy@...radead.org>, kbuild-all@...ts.01.org,
        Jonathan Corbet <corbet@....net>, linux-doc@...r.kernel.org,
        linux-kernel@...r.kernel.org, Florian Eckert <fe@....tdt.de>
Subject: Re: [PATCH v1 1/1] docs: process: submitting-patches: Clarify the
 Reported-by usage

On Thu, Mar 03, 2022 at 02:27:42PM +0100, Johan Hovold wrote:
> On Thu, Mar 03, 2022 at 12:54:32PM +0300, Dan Carpenter wrote:
> > On Tue, Feb 01, 2022 at 09:51:33AM +0100, Johan Hovold wrote:
> > > On Mon, Jan 31, 2022 at 08:16:42PM +0200, Andy Shevchenko wrote:
> > > > On Mon, Jan 31, 2022 at 05:47:32PM +0100, Johan Hovold wrote:
> > > > > On Mon, Jan 31, 2022 at 05:34:35PM +0200, Andy Shevchenko wrote:
> > > > > > On Mon, Jan 31, 2022 at 04:18:30PM +0100, Johan Hovold wrote:
> > > > > > > On Fri, Jan 28, 2022 at 01:44:20PM +0000, Matthew Wilcox wrote:
> > > > > 
> > > > > > > > I think this misunderstands the problem that Andy is trying to fix.
> > > > > > > > 
> > > > > > > > The situation: I write a patch.  I post it for review.  A bot does
> > > > > > > > something and finds a bug (could be compile-error, could be boot
> > > > > > > > problem).  That bot sends a bug report with a suggestion to add
> > > > > > > > Reported-by:.  That suggestion is inappropriate because the bug never
> > > > > > > > made it upstream, so it looks like the bot reported the "problem"
> > > > > > > > that the patch "fixes".
> > > > > > > > 
> > > > > > > > It's not unique to "new feature" patches.  If I'm fixing a bug and
> > > > > > > > my fix also contains a bug spotted by a bot, adding Reported-by
> > > > > > > > makes it look like the bot spotted the original bug, rather than
> > > > > > > > spotting a bug in the fix.
> > > > > > > > 
> > > > > > > > The best thing to do in this case is nothing.  Do not credit the bot.
> > > > > > > > Maybe add a Checked-by:, but that would be a new trailer and I really
> > > > > > > > don't think we need a new kind of trailer to get wrong.
> > > > > > > 
> > > > > > > It seems like the only way to fix this is to fix the bots. Adding more
> > > > > > > documentation is unlikely to help in this case.
> 
> > > Perhaps I'm missing something, but if you re-read Mathews description
> > > above, it still seems to me like the issue is that the bots are trying
> > > to claim credit for finding things that haven't been merged yet.
> > > 
> > > Your suggestion is to document that the bots should be ignored. My
> > > suggestion is to fix the bots.
> > 
> > Originally the kbuild bot used to not have that notice but adding it
> > meant that kbuild bot got a lot more visibility.  The truth is that
> > managers love metrics and it helps people get paid.
> > 
> > The whole point of kbuild-bot was to search the lists and test code
> > before it gets merged.  If they just waited and tested linux-next they
> > would get their reported by tags because most trees don't rebase.  But
> > we're punishing them for being better at their job.  It's a perverse
> > incentive.
> 
> I hear you. But I also get Matthew's and Andy's point about it not being
> quite right to give an automatic test program Reported-by credit for
> finding, say, an unused variable in a not yet merged patch. And perhaps
> even more so since real reviewers often get no credit at all (but
> perhaps a mention in a cover letter).
> 
> > We should create a new tag for finding bugs during review.
> 
> Yeah, I guess your perverse incentive argument applies equally to human
> reviewers even if I'm also reluctant to going down this path.

We could make the tag more explicit:  Bug-fixes-from:

You would only get credit for crashing bugs.  Even manual reviewers
should get credit.  You would not get credit for style complaints or
noticing typos in the commit message.  If the bug is not severe enough
to merit a backport or a Fixes tag then no credit.

People always complain that this rule is ambiguous but we make those
choices all the time about what to include in the -stable trees.

regards,
dan carpenter