From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Roy Spliet" Subject: Re: nouveau nvaa clock missing break? Date: Fri, 27 Dec 2013 11:05:26 +0100 Message-ID: <8191a78f885837fe5c89d9ffb6dd251a@mail.eclipso.de> References: Reply-To: "Roy Spliet" Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Return-path: In-Reply-To: Content-Disposition: inline Sender: linux-kernel-owner@vger.kernel.org To: Kees Cook Cc: bskeggs@redhat.com, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org Hello Kees, Thanks for your feedback. Let me first reassure you that this fall-thro= ugh is indeed intentional: vdiv should be set regardless of the clock s= ource. A comment line would definitely be in place though for future no= uveau hackers. In read_pll, post_div can never be zero in valid execution. If the regi= ster-read returns zero, the GPU has either fallen off the bus, or crash= ed because someone set a divider of 0. I agree that this is not very ro= bust and will probably lead to a divide-by-zero on hardware failure. Ch= ecking it doesn't harm I bet! I will propose a patch addressing these issues as soon as I find some f= ree time. Thanks again for the feedback. Yours, Roy --- Urspr=C3=BCngliche Nachricht --- Von: Kees Cook Datum: 20:51:22 26-12-2013 An: Roy Spliet Betreff: nouveau nvaa clock missing break? > Hi, > > Just curious if this code is missing a "break" or not... > > /drivers/gpu/drm/nouveau/core/subdev/clock/nvaa.c: 373 in nvaa_clock_= prog() > > > switch (priv->vsrc) { > case nv_clk_src_cclk: > mast |=3D 0x00400000; > default: > nv_wr32(clk, 0x4600, priv->vdiv); > } > > Coverity noticed it as CID 1135671. If it's intentional, it might be > nice to add a "fall through" comment there. > > Coverity also complained about read_pll (CID 1135670) where post_div > could (unlikely) be 0 and used for a divide-by-zero. > > Thanks! > > -Kees > > -- > Kees Cook > Chrome OS Security >