syzcaller patch postings...

syzcaller patch postings...

[David Miller]

I have to mention this now before it gets out of control.

I would like to ask that syzkaller stop posting the patch it is
testing when it posts to netdev.

This creates a lot of confusion and I have to manually change the
status in patchwork of every patch syzcaller posts in this way.

I would suggest to post a link to the patch in the archives or
even better, the patchwork link.

Thanks.

Re: syzcaller patch postings...

[Florian Westphal]

David Miller <davem@davemloft.net> wrote:
> I have to mention this now before it gets out of control.
> 
> I would like to ask that syzkaller stop posting the patch it is
> testing when it posts to netdev.

Same for netfilter-devel.

I could not get a reproducer to trigger and asked syzbot to test
the patch (great feature, thanks!) -- i did not cc any mailing list.

So I was very surprised syzbot announced test result by adding
back all CCs from original report.  I think its better to
announce the result to patch author only.

Re: syzcaller patch postings...

[Paolo Abeni]

On Wed, 2018-02-21 at 16:47 -0500, David Miller wrote:
> I have to mention this now before it gets out of control.
> 
> I would like to ask that syzkaller stop posting the patch it is
> testing when it posts to netdev.

There is an open issue on this topic:

https://github.com/google/syzkaller/issues/526

The current behaviour is that syzbot replies to all get_maintainer.pl
recipients after testing a patch, regardless of the test submission
recipient list, the idea was instead to respect such list.

Cheers,

Paolo

Re: syzcaller patch postings...

[Dmitry Vyukov]

On Thu, Feb 22, 2018 at 9:26 AM, Paolo Abeni <pabeni@redhat.com> wrote:
> On Wed, 2018-02-21 at 16:47 -0500, David Miller wrote:
>> I have to mention this now before it gets out of control.
>>
>> I would like to ask that syzkaller stop posting the patch it is
>> testing when it posts to netdev.
>
> There is an open issue on this topic:
>
> https://github.com/google/syzkaller/issues/526
>
> The current behaviour is that syzbot replies to all get_maintainer.pl
> recipients after testing a patch, regardless of the test submission
> recipient list, the idea was instead to respect such list.


Hi David, Florian, Paolo,

Didn't realize it triggers patchwork. This wasn't intentional, sorry.

Do I understand it correctly that if syzbot replies to the CC list
that was in the testing request, it will resolve the problem? So if
netdev wasn't in CC, it will not be added to CC.

I will go and fix it now.

Re: syzcaller patch postings...

[Florian Westphal]

Dmitry Vyukov <dvyukov@google.com> wrote:
> On Thu, Feb 22, 2018 at 9:26 AM, Paolo Abeni <pabeni@redhat.com> wrote:
> > On Wed, 2018-02-21 at 16:47 -0500, David Miller wrote:
> >> I have to mention this now before it gets out of control.
> >>
> >> I would like to ask that syzkaller stop posting the patch it is
> >> testing when it posts to netdev.
> >
> > There is an open issue on this topic:
> >
> > https://github.com/google/syzkaller/issues/526
> >
> > The current behaviour is that syzbot replies to all get_maintainer.pl
> > recipients after testing a patch, regardless of the test submission
> > recipient list, the idea was instead to respect such list.
>
> 
> Hi David, Florian, Paolo,
> 
> Didn't realize it triggers patchwork. This wasn't intentional, sorry.
> 
> Do I understand it correctly that if syzbot replies to the CC list
> that was in the testing request, it will resolve the problem? So if
> netdev wasn't in CC, it will not be added to CC.

Yes, thats at least my expected/desired behaviour.
This way I can even CC some other person (maintainer, reporter etc)
to have them informed about test result too.

> I will go and fix it now.

Thank you Dmitry!

Re: syzcaller patch postings...

[Dmitry Vyukov]

On Thu, Feb 22, 2018 at 11:03 AM, Florian Westphal <fw@strlen.de> wrote:
> Dmitry Vyukov <dvyukov@google.com> wrote:
>> On Thu, Feb 22, 2018 at 9:26 AM, Paolo Abeni <pabeni@redhat.com> wrote:
>> > On Wed, 2018-02-21 at 16:47 -0500, David Miller wrote:
>> >> I have to mention this now before it gets out of control.
>> >>
>> >> I would like to ask that syzkaller stop posting the patch it is
>> >> testing when it posts to netdev.
>> >
>> > There is an open issue on this topic:
>> >
>> > https://github.com/google/syzkaller/issues/526
>> >
>> > The current behaviour is that syzbot replies to all get_maintainer.pl
>> > recipients after testing a patch, regardless of the test submission
>> > recipient list, the idea was instead to respect such list.
>>
>>
>> Hi David, Florian, Paolo,
>>
>> Didn't realize it triggers patchwork. This wasn't intentional, sorry.
>>
>> Do I understand it correctly that if syzbot replies to the CC list
>> that was in the testing request, it will resolve the problem? So if
>> netdev wasn't in CC, it will not be added to CC.
>
> Yes, thats at least my expected/desired behaviour.
> This way I can even CC some other person (maintainer, reporter etc)
> to have them informed about test result too.
>
>> I will go and fix it now.
>
> Thank you Dmitry!


This now should be fixed by
https://github.com/google/syzkaller/commit/7daaa06d53f0f496aa1a87656d16c81ebff37f73
I've also added a note to the doc referenced from bug report emails:
https://github.com/google/syzkaller/commit/7daaa06d53f0f496aa1a87656d16c81ebff37f73#diff-5b3b5ff5f03b01e1d31ec93aafd2f3d5

Re: syzcaller patch postings...

[Daniel Axtens]

Dmitry Vyukov <dvyukov@google.com> writes:

> On Thu, Feb 22, 2018 at 9:26 AM, Paolo Abeni <pabeni@redhat.com> wrote:
>> On Wed, 2018-02-21 at 16:47 -0500, David Miller wrote:
>>> I have to mention this now before it gets out of control.
>>>
>>> I would like to ask that syzkaller stop posting the patch it is
>>> testing when it posts to netdev.
>>
>> There is an open issue on this topic:
>>
>> https://github.com/google/syzkaller/issues/526
>>
>> The current behaviour is that syzbot replies to all get_maintainer.pl
>> recipients after testing a patch, regardless of the test submission
>> recipient list, the idea was instead to respect such list.
>
>
> Hi David, Florian, Paolo,
>
> Didn't realize it triggers patchwork. This wasn't intentional, sorry.

A little-publicised and incorrectly-documented(!) feature of Patchwork
is that it supports some email headers. In particular, if you include an
"X-Patchwork-Hint: ignore" header, the mail will not be parsed by
Patchwork.

This will stop it being recorded as a patch. Unfortunately it will also
stop it being recorded as a comment - I don't know if that's an issue in
this case. Maybe we can set you up with Patchwork 2's new checks
infrastructure instead.

>
> Do I understand it correctly that if syzbot replies to the CC list
> that was in the testing request, it will resolve the problem? So if
> netdev wasn't in CC, it will not be added to CC.
>
> I will go and fix it now.

Re: syzcaller patch postings...

[Dmitry Vyukov]

On Thu, Feb 22, 2018 at 2:31 PM, Daniel Axtens <dja@axtens.net> wrote:
> Dmitry Vyukov <dvyukov@google.com> writes:
>
>> On Thu, Feb 22, 2018 at 9:26 AM, Paolo Abeni <pabeni@redhat.com> wrote:
>>> On Wed, 2018-02-21 at 16:47 -0500, David Miller wrote:
>>>> I have to mention this now before it gets out of control.
>>>>
>>>> I would like to ask that syzkaller stop posting the patch it is
>>>> testing when it posts to netdev.
>>>
>>> There is an open issue on this topic:
>>>
>>> https://github.com/google/syzkaller/issues/526
>>>
>>> The current behaviour is that syzbot replies to all get_maintainer.pl
>>> recipients after testing a patch, regardless of the test submission
>>> recipient list, the idea was instead to respect such list.
>>
>>
>> Hi David, Florian, Paolo,
>>
>> Didn't realize it triggers patchwork. This wasn't intentional, sorry.
>
> A little-publicised and incorrectly-documented(!) feature of Patchwork
> is that it supports some email headers. In particular, if you include an
> "X-Patchwork-Hint: ignore" header, the mail will not be parsed by
> Patchwork.
>
> This will stop it being recorded as a patch. Unfortunately it will also
> stop it being recorded as a comment - I don't know if that's an issue in
> this case. Maybe we can set you up with Patchwork 2's new checks
> infrastructure instead.

Nice. But unfortunately the current mailing technology we use allows
very limited set of headers and no custom headers:
https://cloud.google.com/appengine/docs/standard/go/mail/mail-with-headers-attachments
So while possible, it would require very significant rework...

What's the Patchwork 2's new checks infrastructure?
If it will still remain a problem (hopefully not), then maybe it's
possible to blacklist syzbot address from creating new patches. syzbot
can do a lot, but so far does not also generate fixes for the bugs it
discovers :)

Re: syzcaller patch postings...

[Daniel Axtens]

Dmitry Vyukov <dvyukov@google.com> writes:

> On Thu, Feb 22, 2018 at 2:31 PM, Daniel Axtens <dja@axtens.net> wrote:
>> Dmitry Vyukov <dvyukov@google.com> writes:
>>
>>> On Thu, Feb 22, 2018 at 9:26 AM, Paolo Abeni <pabeni@redhat.com> wrote:
>>>> On Wed, 2018-02-21 at 16:47 -0500, David Miller wrote:
>>>>> I have to mention this now before it gets out of control.
>>>>>
>>>>> I would like to ask that syzkaller stop posting the patch it is
>>>>> testing when it posts to netdev.
>>>>
>>>> There is an open issue on this topic:
>>>>
>>>> https://github.com/google/syzkaller/issues/526
>>>>
>>>> The current behaviour is that syzbot replies to all get_maintainer.pl
>>>> recipients after testing a patch, regardless of the test submission
>>>> recipient list, the idea was instead to respect such list.
>>>
>>>
>>> Hi David, Florian, Paolo,
>>>
>>> Didn't realize it triggers patchwork. This wasn't intentional, sorry.
>>
>> A little-publicised and incorrectly-documented(!) feature of Patchwork
>> is that it supports some email headers. In particular, if you include an
>> "X-Patchwork-Hint: ignore" header, the mail will not be parsed by
>> Patchwork.
>>
>> This will stop it being recorded as a patch. Unfortunately it will also
>> stop it being recorded as a comment - I don't know if that's an issue in
>> this case. Maybe we can set you up with Patchwork 2's new checks
>> infrastructure instead.
>
> Nice. But unfortunately the current mailing technology we use allows
> very limited set of headers and no custom headers:
> https://cloud.google.com/appengine/docs/standard/go/mail/mail-with-headers-attachments
> So while possible, it would require very significant rework...

Ah, oh well, nevermind.

> What's the Patchwork 2's new checks infrastructure?

<puts on patchwork maintainer hat>
It's probably more a long-term thing than an immediate fix, but...
The checks API is designed to integrate reporting of CI/testing results
into Patchwork. It allows - through a REST API - an arbitrary process
(like your checking) to report success/warning/failure against a
patch. In your case you could report success = patch prevents bug, and
failure = bug still exists with patch. It's still slightly a
work-in-progress: at the moment you need an API key from a maintainer to
post checks. But it does look pretty in the web frontend:
e.g. https://patchwork.ozlabs.org/patch/871346/ - and the number of
successful/warning/failed tests shows up on the patch list page.

There's currently only one project (that I know of) out there that uses
the checks API - Snowpatch: https://github.com/ruscur/snowpatch

If, at any point in the future, you want to explore this, let me know as
I'd be *very* happy to help with the implementation and if needed push
features into Patchwork that make it easier/better.
</hat>

> If it will still remain a problem (hopefully not), then maybe it's
> possible to blacklist syzbot address from creating new patches. syzbot
> can do a lot, but so far does not also generate fixes for the bugs it
> discovers :)

In immediate practical terms, that might be the easiest. They all come
from the same email address, right?

Regards,
Daniel

Re: syzcaller patch postings...

[Dmitry Vyukov]

On Thu, Feb 22, 2018 at 3:16 PM, Daniel Axtens <dja@axtens.net> wrote:
> Dmitry Vyukov <dvyukov@google.com> writes:
>
>> On Thu, Feb 22, 2018 at 2:31 PM, Daniel Axtens <dja@axtens.net> wrote:
>>> Dmitry Vyukov <dvyukov@google.com> writes:
>>>
>>>> On Thu, Feb 22, 2018 at 9:26 AM, Paolo Abeni <pabeni@redhat.com> wrote:
>>>>> On Wed, 2018-02-21 at 16:47 -0500, David Miller wrote:
>>>>>> I have to mention this now before it gets out of control.
>>>>>>
>>>>>> I would like to ask that syzkaller stop posting the patch it is
>>>>>> testing when it posts to netdev.
>>>>>
>>>>> There is an open issue on this topic:
>>>>>
>>>>> https://github.com/google/syzkaller/issues/526
>>>>>
>>>>> The current behaviour is that syzbot replies to all get_maintainer.pl
>>>>> recipients after testing a patch, regardless of the test submission
>>>>> recipient list, the idea was instead to respect such list.
>>>>
>>>>
>>>> Hi David, Florian, Paolo,
>>>>
>>>> Didn't realize it triggers patchwork. This wasn't intentional, sorry.
>>>
>>> A little-publicised and incorrectly-documented(!) feature of Patchwork
>>> is that it supports some email headers. In particular, if you include an
>>> "X-Patchwork-Hint: ignore" header, the mail will not be parsed by
>>> Patchwork.
>>>
>>> This will stop it being recorded as a patch. Unfortunately it will also
>>> stop it being recorded as a comment - I don't know if that's an issue in
>>> this case. Maybe we can set you up with Patchwork 2's new checks
>>> infrastructure instead.
>>
>> Nice. But unfortunately the current mailing technology we use allows
>> very limited set of headers and no custom headers:
>> https://cloud.google.com/appengine/docs/standard/go/mail/mail-with-headers-attachments
>> So while possible, it would require very significant rework...
>
> Ah, oh well, nevermind.
>
>> What's the Patchwork 2's new checks infrastructure?
>
> <puts on patchwork maintainer hat>
> It's probably more a long-term thing than an immediate fix, but...
> The checks API is designed to integrate reporting of CI/testing results
> into Patchwork. It allows - through a REST API - an arbitrary process
> (like your checking) to report success/warning/failure against a
> patch. In your case you could report success = patch prevents bug, and
> failure = bug still exists with patch. It's still slightly a
> work-in-progress: at the moment you need an API key from a maintainer to
> post checks. But it does look pretty in the web frontend:
> e.g. https://patchwork.ozlabs.org/patch/871346/ - and the number of
> successful/warning/failed tests shows up on the patch list page.
>
> There's currently only one project (that I know of) out there that uses
> the checks API - Snowpatch: https://github.com/ruscur/snowpatch
>
> If, at any point in the future, you want to explore this, let me know as
> I'd be *very* happy to help with the implementation and if needed push
> features into Patchwork that make it easier/better.
> </hat>

Interesting.

So far syzbot does not test all patches, it only tests by an explicit
developer request (and that patch is not necessary on Patchwork yet,
e.g. it can be just a debugging patch that add more checks and debug
output and it still meant to fail).
But long term we probably would like to test all fixes for syzbot
bugs, so I will keep this in mind.
Is there any kind of push/poll api to get list of patches? Or
otherwise how are external systems are meant to know that there is
something to test?


>> If it will still remain a problem (hopefully not), then maybe it's
>> possible to blacklist syzbot address from creating new patches. syzbot
>> can do a lot, but so far does not also generate fixes for the bugs it
>> discovers :)
>
> In immediate practical terms, that might be the easiest. They all come
> from the same email address, right?

More or less. It would be "syzbot\+[0-9a-f]+@syzkaller\.appspotmail\.com".

Re: syzcaller patch postings...

[David Miller]

From: Dmitry Vyukov <dvyukov@google.com>
Date: Thu, 22 Feb 2018 10:58:07 +0100

> Do I understand it correctly that if syzbot replies to the CC list
> that was in the testing request, it will resolve the problem? So if
> netdev wasn't in CC, it will not be added to CC.
> 
> I will go and fix it now.

I don't want syzbot to send the patch to netdev, even if it
was in the CC: list.

And again this goes for netfilter-devel and linux-wireless as
well.

There is no reason whatsoever for syzbot to ever post an already
posted patch back to the list again, even if it was on the CC:
list.

In fact netdev will be on that CC: list most of the time.

Re: syzcaller patch postings...

[Dmitry Vyukov]

On Thu, Feb 22, 2018 at 3:35 PM, David Miller <davem@davemloft.net> wrote:
> From: Dmitry Vyukov <dvyukov@google.com>
> Date: Thu, 22 Feb 2018 10:58:07 +0100
>
>> Do I understand it correctly that if syzbot replies to the CC list
>> that was in the testing request, it will resolve the problem? So if
>> netdev wasn't in CC, it will not be added to CC.
>>
>> I will go and fix it now.
>
> I don't want syzbot to send the patch to netdev, even if it
> was in the CC: list.
>
> And again this goes for netfilter-devel and linux-wireless as
> well.
>
> There is no reason whatsoever for syzbot to ever post an already
> posted patch back to the list again, even if it was on the CC:
> list.
>
> In fact netdev will be on that CC: list most of the time.

But if the list on CC the first time, then the patch is already on
Patchwork, right? When syzbot replies it will add In-Reply-To, so this
should be treated as a comment to the existing patch? Will it still
cause problems?
How does Patchwork understand that an email contains a patch? Perhaps
if we make these emails appear as they don't contain a patch, it will
solve the problem.

Re: syzcaller patch postings...

[Daniel Axtens]

Dmitry Vyukov <dvyukov@google.com> writes:

> On Thu, Feb 22, 2018 at 3:35 PM, David Miller <davem@davemloft.net> wrote:
>> From: Dmitry Vyukov <dvyukov@google.com>
>> Date: Thu, 22 Feb 2018 10:58:07 +0100
>>
>>> Do I understand it correctly that if syzbot replies to the CC list
>>> that was in the testing request, it will resolve the problem? So if
>>> netdev wasn't in CC, it will not be added to CC.
>>>
>>> I will go and fix it now.
>>
>> I don't want syzbot to send the patch to netdev, even if it
>> was in the CC: list.
>>
>> And again this goes for netfilter-devel and linux-wireless as
>> well.
>>
>> There is no reason whatsoever for syzbot to ever post an already
>> posted patch back to the list again, even if it was on the CC:
>> list.
>>
>> In fact netdev will be on that CC: list most of the time.
>
> But if the list on CC the first time, then the patch is already on
> Patchwork, right? When syzbot replies it will add In-Reply-To, so this
> should be treated as a comment to the existing patch? Will it still
> cause problems?

You would think that it would treat it as a comment, but it doesn't.  We
treat something as a comment if and only if the subject begins with some
variant of Re:.  An I-R-T is not enough: I think the reasoning was that
people sometimes post their v2 series as replies to their v1 series,
which is bad; but we erred on the side of not losing patches.

There's not really a solid conceptual framework for this - in part
because parsing the sheer variety of mail people post is really, really
hard. Suggestions on a better algorithm are of course welcome.

> How does Patchwork understand that an email contains a patch? Perhaps
> if we make these emails appear as they don't contain a patch, it will
> solve the problem.

We have a pretty sophisticated parser that looks for things that look
like the output of diff. [FWIW, the algorithm is at
https://github.com/getpatchwork/patchwork/blob/master/patchwork/parser.py#L688 ]

You *could* try to trick it: looking at the code, probably the simplest
way would be to replace '---' in '--- a/foo/bar.c' with something else,
but I'm really quite uncomfortable with that.

Regards,
Daniel

Re: syzcaller patch postings...

[Dmitry Vyukov]

On Thu, Feb 22, 2018 at 3:35 PM, David Miller <davem@davemloft.net> wrote:
> From: Dmitry Vyukov <dvyukov@google.com>
> Date: Thu, 22 Feb 2018 10:58:07 +0100
>
>> Do I understand it correctly that if syzbot replies to the CC list
>> that was in the testing request, it will resolve the problem? So if
>> netdev wasn't in CC, it will not be added to CC.
>>
>> I will go and fix it now.
>
> I don't want syzbot to send the patch to netdev, even if it
> was in the CC: list.
>
> And again this goes for netfilter-devel and linux-wireless as
> well.
>
> There is no reason whatsoever for syzbot to ever post an already
> posted patch back to the list again, even if it was on the CC:
> list.
>
> In fact netdev will be on that CC: list most of the time.


Hi David,

We've found a simple and reasonable solution with Daniel.
If we prefix subjects of these replies with "Re: " then Patchwork will
never treat them as new patches (always as a comment).
This is now implemented and deployed for syzbot:
https://github.com/google/syzkaller/commit/334641584880cd238fc32dc6f436e7e10efdf3de
So we now have 2 lines of defense for the problem never happening again.

Thanks for bringing it up.