linux-kernel - Re: [PATCH v4 2/3] 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]

Message-ID: <17fdf6f1-60ab-bfde-afc8-5afef6cc797b@leemhuis.info>
Date:   Thu, 2 Mar 2023 10:04:30 +0100
From:   Thorsten Leemhuis <linux@...mhuis.info>
To:     Dmitry Vyukov <dvyukov@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>
Cc:     Joe Perches <joe@...ches.com>, Andy Whitcroft <apw@...onical.com>,
        Dwaipayan Ray <dwaipayanray1@...il.com>,
        Lukas Bulwahn <lukas.bulwahn@...il.com>,
        Kai Wasserbäch <kai@....carbon-project.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        linux-kernel@...r.kernel.org, Aleksandr Nogikh <nogikh@...gle.com>,
        Taras Madan <tarasmadan@...gle.com>,
        syzkaller <syzkaller@...glegroups.com>,
        Theodore Ts'o <tytso@....edu>
Subject: Re: [PATCH v4 2/3] checkpatch: warn when Reported-by: is not followed
 by Link:

On 02.03.23 09:27, Dmitry Vyukov wrote:
> On Thu, 2 Mar 2023 at 06:40, Jakub Kicinski <kuba@...nel.org> wrote:
>> On Thu, 2 Mar 2023 06:17:22 +0100 Thorsten Leemhuis wrote:
>>> On 02.03.23 05:46, Jakub Kicinski wrote:
>>>> On Fri, 20 Jan 2023 13:35:19 +0100 Thorsten Leemhuis wrote:
>>>>> Encourage patch authors to link to reports by issuing a warning, if
>>>>> a Reported-by: is not accompanied by a link to the report. Those links
>>>>> are often extremely useful for any code archaeologist that wants to know
>>>>> more about the backstory of a change than the commit message provides.
>>>>> That includes maintainers higher up in the patch-flow hierarchy, which
>>>>> is why Linus asks developers to add such links [1, 2, 3]. To quote [1]:
>>>>
>>>> Is it okay if we exclude syzbot reports from this rule?
>>>> If full syzbot report ID is provided - it's as good as a link.
>>>
>>> Hmmm. Not sure. Every special case makes things harder for humans and
>>> software that looks at a commits downstream. Clicking on a link also
>>> makes things easy for code archaeologists that might look into the issue
>>> months or years later (which might not even know how to find the report
>>> and potential discussions on lore from the syzbot report ID).
>>
>> No other system comes close to syzbot in terms of reporting meaningful
>> bugs, IMHO special casing it doesn't risk creep.
>>
>> Interestingly other bots attach links which are 100% pointless noise:
>>
>> Reported-by: Abaci Robot <abaci@...ux.alibaba.com>
>> Link: https://bugzilla.openanolis.cn/show_bug.cgi?id=4174
>>
>> Oh, eh. Let's see how noisy this check is once the merge window is over.
>>
>>> Hence, wouldn't it be better to ask the syzbot folks to change their
>>> reporting slightly and suggest something like this instead in their
>>> reports (the last line is the new one):
>>>
>>> ```
>>> IMPORTANT: if you fix the issue, please add the following tag to the commit:
>>> Reported-by: syzbot+bba886ab504fcafecafe@...kaller.appspotmail.com
>>> Link: https://lore.kernel.org/r/cafecaca0cafecaca0cafecaca0@google.com/
>>> ```
>>>
>>> This might not be to hard if they known the message-id in advance. Maybe
>>> they could even use the syzbot report ID as msg-id to make things even
>>> easier. And for developers not much would change afaics, they just need
>>> to copy and paste two lines instead of one.
>>
>> Dmitry, WDYT?
> 
> Adding a Link to syzbot reports should be relatively trivial.

Sounds good.

> Ted proposed to use Link _instead_ of Reported-by:
> https://github.com/google/syzkaller/issues/3596
>> in fact, it might be nice if we could encourage upstream developers
>> put in the commit trailer:
>> Link: https://syzkaller.appspot.com/bug?id=5266d464285a03cee9dbfda7d2452a72c3c2ae7c
>> in addition to, or better yet, instead of:
>> Reported-by: syzbot+15cd994e273307bf5cfa@...kaller.appspotmail.com
> 
> We could also use a link in the Reported-by tag, e.g.:
> 
> Reported-by: https://syzkaller.appspot.com/b/5266d464285a03cee9db
> 
> Some folks parse Reported-by to collect stats.
> 
> What is better?

Here are my thoughts:

* we should definitely have a "Link:" to the report in lore, as that's
the long-term archive under our own control and also where discussions
happen after the report was posted; but I'm biased here, as such a tag
would make tracking with regzbot a no-brainer ;)

* "Reported-by:" IMHO should stay for the hat tip and stats aspects; I
don't care if it includes the syzbot report ID or not (omitting it might
be good for the stats aspects and is more friendly to the eyes, but
those are just details)

* a Link: to the syzkaller web ui might be nice, too -- and likely is
the easiest thing to look out for on the syzbot server side

IOW something like this maybe:

Reported-by: syzbot+cafecafecaca0cafecafe@...kaller.appspotmail.com
Link: https://lore.kernel.org/r/cafecafecaca0cafecafe@google.com/
Link: https://syzkaller.appspot.com/b/cafecafecaca0cafecafe

Something like the following would look more normal, but of course is
only possible if syzbot starts out to look for such Link: tags (not sure
if the msgid is valid here, but you get the idea):

Reported-by: syzbot@...kaller.appspotmail.com
Link:
https://lore.kernel.org/r/syzbot+cafecafecaca0cafecafe-syzkaller-appspotmail-com@google.com/

Ciao, Thorsten