Re: syzcaller patch postings...
On Thu, Feb 22, 2018 at 3:35 PM, David Miller wrote: > From: Dmitry Vyukov > 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.
Re: syzcaller patch postings...
Dmitry Vyukov writes: > On Thu, Feb 22, 2018 at 3:35 PM, David Miller wrote: >> From: Dmitry Vyukov >> 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...
On Thu, Feb 22, 2018 at 3:35 PM, David Miller wrote: > From: Dmitry Vyukov > 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...
On Thu, Feb 22, 2018 at 3:16 PM, Daniel Axtens wrote: > Dmitry Vyukov writes: > >> On Thu, Feb 22, 2018 at 2:31 PM, Daniel Axtens wrote: >>> Dmitry Vyukov writes: >>> On Thu, Feb 22, 2018 at 9:26 AM, Paolo Abeni 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? > > > 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. > 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...
From: Dmitry Vyukov 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 writes: > On Thu, Feb 22, 2018 at 2:31 PM, Daniel Axtens wrote: >> Dmitry Vyukov writes: >> >>> On Thu, Feb 22, 2018 at 9:26 AM, Paolo Abeni 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? 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. > 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...
On Thu, Feb 22, 2018 at 2:31 PM, Daniel Axtens wrote: > Dmitry Vyukov writes: > >> On Thu, Feb 22, 2018 at 9:26 AM, Paolo Abeni 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...
Dmitry Vyukov writes: > On Thu, Feb 22, 2018 at 9:26 AM, Paolo Abeni 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...
On Thu, Feb 22, 2018 at 11:03 AM, Florian Westphal wrote: > Dmitry Vyukov wrote: >> On Thu, Feb 22, 2018 at 9:26 AM, Paolo Abeni 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...
Dmitry Vyukov wrote: > On Thu, Feb 22, 2018 at 9:26 AM, Paolo Abeni 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...
On Thu, Feb 22, 2018 at 9:26 AM, Paolo Abeni 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...
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...
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. 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.
syzcaller patch postings...
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.
