From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 6A03E71 for ; Fri, 23 Apr 2021 17:17:43 +0000 (UTC) Received: by mail.kernel.org (Postfix) with ESMTPSA id 1730D613CB; Fri, 23 Apr 2021 17:17:41 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1619198262; bh=nZYMSwFNjqKZ4K5QofpzPBFqlFmB0XKmoHmBbvCabcc=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=Z3UBY6th0DL6fS7EfPdBJPRs/DEBOg5sumeGOzlIPzh/9lSQgUXC/xlBHx0SXr4Fq lFS3Lnxx0ua0QiKhUIE2NeOpMF04PJcQijZbdvqjX1BEAvvBEhK0hWgiZTGZ63rjpi rrFoKv6QvUbRo9UO+ThKbOhAcXzhuL/4zovy5IJcFC6tVW+RHzVggWTI10q+Ez0XBi bgED7y0zVtYrTBOThUwdCavH4+4njFn28aKmcDGO48QnFVlYAX/LLZu9g5NJjV1LF6 0H+8v0LIdmBnBvLUiMLUt4eljTcwKtynVJr6qEVD4auaHmh0TB/jf+UK2tdKMJYxxn kVAZeL+0YNWwA== Date: Fri, 23 Apr 2021 20:17:38 +0300 From: Leon Romanovsky To: Mauro Carvalho Chehab Cc: Shuah Khan , Geert Uytterhoeven , James Morris , Julia Lawall , Stephen Hemminger , Roland Dreier , Steven Rostedt , James Bottomley , ksummit@lists.linux.dev Subject: Re: [MAINTAINER SUMMIT] Rethinking the acceptance policy for "trivial" patches Message-ID: References: <20210421132824.13a70f6c@hermes.local> <20210422115511.60d1f735@coco.lan> <24762711-0252-f7d2-4e41-3eb1e27955ea@linuxfoundation.org> <20210423110643.4b28c29b@coco.lan> X-Mailing-List: ksummit@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210423110643.4b28c29b@coco.lan> On Fri, Apr 23, 2021 at 11:06:43AM +0200, Mauro Carvalho Chehab wrote: > Em Thu, 22 Apr 2021 09:38:03 -0600 > Shuah Khan escreveu: > > > On 4/22/21 6:18 AM, Leon Romanovsky wrote: > > > On Thu, Apr 22, 2021 at 11:55:11AM +0200, Mauro Carvalho Chehab wrote: > > >> Em Thu, 22 Apr 2021 09:34:38 +0200 > > >> Geert Uytterhoeven escreveu: > > >> > > >>> On Wed, Apr 21, 2021 at 11:50 PM James Morris wrote: > > >>>> On Wed, 21 Apr 2021, Julia Lawall wrote: > > >>>>> The apology states that they didn't detect any vulnerabilities. They > > >>>>> found three non exploitable bugs and submitted incorrect patches for them. > > >>>>> When the patches received some positive feedback, they explained that the > > >>>>> patches were incorrect and provided a proper fix. > > >>>>> > > >>>>> So they damaged trust, but not actually the Linux kernel... > > >>>> > > >>>> The issue is that there was no consent and no coordination, so we don't > > >>>> know the scope of the experiment and whether it was still continuing. > > >>>> > > >>>> We are this not able to trust anything the group said about what they'd > > >>>> done or not done, until now [1]. > > >>>> > > >>>> In all probability there is nothing further amiss but we would not have > > >>>> expected them to use fake gmail accounts to submit bugs to the kernel > > >>>> either. > > >>>> > > >>>> It's now on us to audit all of their known contributions, because we don't > > >>>> know the scope of the experiment, which was based on the use of deception, > > >>>> and we can't make any assumptions based on what they have said. > > >>>> > > >>>> We also need the identity of the 'random' gmail accounts they used, > > >>>> although this should be handled by a small trusted group in private, as it > > >>>> will lead to privacy issues for kernel maintainers who responded to them > > >>>> in public. > > >>> > > >>> What do we gain by blindly reverting all[1] umn.edu patches, and > > >>> ignoring future patches from umn.edu? > > >>> I think all of this is moot: other people may be doing the same thing, > > >>> or even "in worse faith". The only thing that helps is making sure > > >>> patches get reviewed[2] before being applied. > > >>> > > >>> [1] Judging from the new review comments, many of the 190 patches > > >>> to be reverted were real fixes. > > >> > > >> The reverted ones for media (29 patches) didn't contain any malicious code. > > >> One was useless (because the media core already fixes the pointed issue), > > >> but the other ones were valid patches. > > > > > > I'm sorry that I didn't check all media commits, but this random commit > > > 467a37fba93f ("media: dvb: Add check on sp8870_readreg") has a bug and > > > broke sp8870 (I don't know what is it). > > > > > > diff --git a/drivers/media/dvb-frontends/sp8870.c b/drivers/media/dvb-frontends/sp8870.c > > > index 8d31cf3f4f07..270a3c559e08 100644 > > > --- a/drivers/media/dvb-frontends/sp8870.c > > > +++ b/drivers/media/dvb-frontends/sp8870.c > > > @@ -293,7 +293,9 @@ static int sp8870_set_frontend_parameters(struct dvb_frontend *fe) > > > sp8870_writereg(state, 0xc05, reg0xc05); > > > > > > // read status reg in order to clear pending irqs > > > - sp8870_readreg(state, 0x200); > > > + err = sp8870_readreg(state, 0x200); > > > + if (err) > > > + return err; > > > > > > // system controller start > > > sp8870_microcontroller_start(state); > > > > > > > > > 67 static int sp8870_readreg (struct sp8870_state* state, u16 reg) > > > 68 { > > > 69 int ret; > > > <...> > > > 77 if (ret != 2) { > > > 78 dprintk("%s: readreg error (ret == %i)\n", __func__, ret); > > > 79 return -1; > > > 80 } > > > 81 > > > 82 return (b1[0] << 8 | b1[1]); > > > 83 } > > > > > > The valid check should be if (err < 0); > > > > > > > Correct. Like all the other callers of sp8870_readreg() do with > > its return. Non-zero return is valid for this routine. > > This particular patch is completely broken and should be reverted. This is exactly the point, many patches from @umn are broken and the right way to check them is to checkout to the time when they were introduced. I just wanted to show you that you claim about validity of patches is not accurate. "The reverted ones for media (29 patches) didn't contain any malicious code. One was useless (because the media core already fixes the pointed issue), out the other ones were valid patches." Thanks