On Thu, Feb 24, 2022 at 09:19:24AM +0100, Thorsten Leemhuis wrote: > On 24.02.22 01:14, Kees Cook wrote: > > As a follow-up to the UMN incident[1], the TAB took the responsibility > > to document Researcher Guidelines so there would be a common place to > > point for describing our expectations as a developer community. > > > > Document best practices researchers should follow to participate > > successfully with the Linux developer community. > > > > [1] https://lore.kernel.org/lkml/202105051005.49BFABCE@keescook/ > > > > Co-developed-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> > > Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> > > Co-developed-by: Jonathan Corbet <corbet@xxxxxxx> > > Signed-off-by: Jonathan Corbet <corbet@xxxxxxx> > > Co-developed-by: Stefano Zacchiroli <zack@xxxxxxxxxx> > > Signed-off-by: Stefano Zacchiroli <zack@xxxxxxxxxx> > > Co-developed-by: Steven Rostedt <rostedt@xxxxxxxxxxx> > > Signed-off-by: Steven Rostedt <rostedt@xxxxxxxxxxx> > > Acked-by: Steve Rostedt <rostedt@xxxxxxxxxxx> > > Acked-by: Laura Abbott <labbott@xxxxxxxxxx> > > Reviewed-by: Julia Lawall <julia.lawall@xxxxxxxx> > > Reviewed-by: Wenwen Wang <wenwen@xxxxxxxxxx> > > Signed-off-by: Kees Cook <keescook@xxxxxxxxxxxx> > > --- > > Documentation/admin-guide/index.rst | 1 + > > .../admin-guide/researcher-guidelines.rst | 141 ++++++++++++++++++ > > Hmm, the intro for "Documentation/admin-guide/" states that "The > following manuals are written for users of the kernel", but the added > text afaics providing nothing regular users care about. So wouldn't it > be better if this lived below Documentation/process/ ? It might not a > really good fit either, but I'd say it's the better place. > > But well, the best person to know is Jonathan, who is listed as a > Co-developer above, so maybe I'm wrong my suggestion is a bad one. I started in process/ and eventually settled on admin-guide/ since that's basically the "front page". But I agree, there isn't an obviously correct place for it. > > +Researcher Guidelines > > ++++++++++++++++++++++ > > + > > +The Linux kernel community > > Nitpicking: wondering if this maybe should be something like "The Linux > kernel developer community" (or "Linux kernel's..."?). The idea was to lot limit this to strictly developers. (i.e. cast a wide net.) > [...] > > +Passive research that is based entirely on publicly available sources, > > +including posts to public mailing lists and commits to public > > +repositories, is clearly permissible. Though, as with any research, > > +standard ethics must still be followed. > > + > > +Active research on developer behavior, > > Nitpicking: when reading this for the first time I here wondered what is > precisely meant by "Active research". Is this maybe an established term > in academia I'm simply not aware of? If not, maybe simply writing > something like "Other research on developer behavior" or "Research on > developer behavior where you engage in development" avoid this. > > > however, must be done with the > > +explicit agreement of, and full disclosure to, the individual developers > > +involved. Developers cannot be interacted with/experimented on without > > +consent; this, too, is standard research ethics. > > + > > +To help clarify: > > Follow up to above remark: If "Active research" stays above, I'd say it > might be worth repeating the term here to make clear what's being clarified. Hm, yeah, it was a "passive" / "active" comparison, in the sense of trying to describe what is examining subject's artifacts (passive) and interacting with subjects (active). I don't think it's an academic term, but rather an engineering term? > > sending patches to developers *is* interacting > > +with them, but they have already consented to receiving *good faith > > +contributions*. Sending intentionally flawed/vulnerable patches or > > +contributing misleading information to discussions is not consented > > +to. Such communication can be damaging to the developer (e.g. draining > > +time, effort, and morale) and damaging to the project by eroding > > +the entire developer community's trust in the contributor (and the > > +contributor's organization as a whole), undermining efforts to provide > > +constructive feedback to contributors, and putting end users at risk of > > +software flaws. > > Nitpicking again: Quite a long sentence. Maybe split it up with > something like a "s!, undermining !; such an approach would thus undermine" Yeah, I can tweak this. That did bother me a little too. > > +Participation in the development of Linux itself by researchers, as > > +with anyone, is welcomed and encouraged. Research into Linux code is > > +a common practice, especially when it comes to developing or running > > +analysis tools that produce actionable results. > > + > > +When engaging with the developer community, sending a patch has > > +traditionally been the best way to make an impact. Linux already has > > +plenty of known bugs -- what's much more helpful is having vetted fixes. > > +Before contributing, carefully read the documentation on > > +:doc:`submitting patches <../process/submitting-patches>`, > > +:doc:`reporting issues <reporting-issues>`, and > > +:doc:`handling serious flaws <security-bugs>`. > > Wonder if Documentation/process/{development-process.rst,5.Posting.rst} > should be linked as well. I wasn't sure what the balance should be between not enough and too much information. :) > Additionally not my area of expertise, but from what I'd noticed it > seems it's better to avoid the :doc:`foo` tag if there is no strict need > (and I don't think there is one in this case). For the background see here: > > https://lore.kernel.org/all/cover.1623824363.git.mchehab+huawei@xxxxxxxxxx/ > > Most of those patches got applied meanwhile afaik. Oh! Thanks for pointing that out; I'll drop all of that. > [...] > > +If you are a first time contributor it is recommended that the patch > > +itself be vetted by others privately before being posted to public lists. > > +(This is required if you have been explicitly told your patches need > > +more careful internal review.) These people are expected to have their > > +"Reviewed-by" tag included in the resulting patch. Finding another > > +developer familiar with Linux contribution, especially within your own > > +organization, and having them help with reviews before sending them to > > +the public mailing lists tends to significantly improve the quality of the > > +resulting patches, and there by reduces the burden on other developers. > > I like the section, but I wonder why it's needed here. Is there anything > specific to patches produced from research in it there I missed when > reading it? If not: Wouldn't it be better to include that section as a > TLDR in Documentation/process/submitting-patches.rst and point there > instead? We already have at least two places describing how to submit > patches, creating yet another one (even if it's just in such a brief > version) somehow feels slightly wrong to me. Yeah, there is some redundancy here, but I wanted to have an example specifically tuned to the scenario of really fleshing out the parts we'd expect from a researcher to help show what context developers are expecting. Thanks for the review! -Kees -- Kees Cook
