Re: [MAINTAINER SUMMIT] Rethinking the acceptance policy for "trivial" patches

[Kees Cook <keescook@chromium.org>]

On Wed, Apr 21, 2021 at 03:22:09PM -0400, Steven Rostedt wrote:
> On Wed, 21 Apr 2021 11:35:36 -0700
> James Bottomley <James.Bottomley@HansenPartnership.com> wrote:
> 
> > I've long been on record as not really being a fan of trivial patches
> > because they can cause merge issues with current patches and introduce
> > bugs, particularly in older drivers, that don't get detected for a long
> > while.  However, the recent events with the University of Minnesota:
> > 
> > https://lore.kernel.org/lkml/20210421130105.1226686-1-gregkh@linuxfoundation.org/
> > 
> > Have elevated the risk factor around trivial patches claiming to fix
> > bugs to the point where it looks like there's no such thing as a truly
> > trivial patch and they all need reviewing.
> > 
> > Our policy in SCSI for a long time has been no trivial patches accepted
> > to maintained drivers, and I think that would be a good start if
> > adopted kernel wide, but I think the next policy should be no trivial
> > bug fix without a pointer to the actual bug report or report from a
> > trusted static checker.  This would likely mean we have to create a
> > list of trusted static checkers ... obviously 0day and coverity but
> > what else?
> 
> I take a lot of trivial fixes. I found two that I accepted that were from
> umn.edu, and both of them (after a second review) were legitimate fixes.
> One was in Greg's revert patch series, which I asked him to not revert, and
> the other was me looking at all patches I've received with a Cc to umn.edu
> emails, and was from a gmail account (which I'm assuming was someone that
> was part of this group).
> 
> I have no problem with taking a trivial patch if they are really fixing a
> bug. I think what needs to be done here is look at the patches that got in
> that were buggy, and see why they got in.
> 
> Perhaps the answer is to scrutinize trivial patches more. To me, the only
> "trivial" patch is a comment fix, or update to documentation. And even
> then, I spend time reviewing it.
>
> If you don't have time to review a patch, then by all means, don't accept
> it. Perhaps the answer is simply have a higher bar on what you do accept.

Agreed. I and many other folks do trivial patching all over the kernel
(though, yes, we're usually much better at describing tools, methods,
reachability, and impact), but I don't want us to swing too far in the
other direction and allow the UMN situation to have a chilling effect on
legitimate (even if "trivial") improvements.

-- 
Kees Cook