linux-kernel - Re: [PATCH v2] misc: atmel-ssc: Handle return value of clk_prepare_enable and clk

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20170605131319.l3a7aw4xvgwls2ry@piout.net>
Date:   Mon, 5 Jun 2017 15:13:19 +0200
From:   Alexandre Belloni <alexandre.belloni@...e-electrons.com>
To:     Greg KH <gregkh@...uxfoundation.org>
Cc:     Arvind Yadav <arvind.yadav.cs@...il.com>,
        nicolas.ferre@...rochip.com, linux-kernel@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v2] misc: atmel-ssc: Handle return value of
 clk_prepare_enable and clk_prepare

On 05/06/2017 at 14:05:15 +0200, Greg KH wrote:
> On Mon, Jun 05, 2017 at 11:36:52AM +0200, Alexandre Belloni wrote:
> > On 05/06/2017 at 14:53:30 +0530, Arvind Yadav wrote:
> > > Hi,
> > > 
> > > Yes, Patch v1 was wrong that's why i have push v2.
> > > clk_prepare and clk_prepare_enable can fail. There
> > 
> > No this is not true, they will never fail for the SSC.
> 
> How is anyone supposed to know this?  Just check the functions correctly
> and move on, if they can never fail, wonderful, that's a code path that
> is not going to be run.
> 

Anyone able (and taking the time) to read code can know that.

I'm really not okay with having useless code paths. Many people are
already complaining (rightfully) that the kernel is bloated, let's try
to not make the situation worse and keep in mind that Linux can run on
tiny SoCs.

> > > is not harm to check it's return value. It'll not impact present
> > > functionality.
> > > 
> > 
> > It does impact boot time, this patch adds 112 bytes to a compressed
> > kernel for no reason.
> 
> It solves the issue that if someone copy/paste from this code, they will
> implement something incorrectly.  You can spare the 112 bytes (which
> really, seems very large for just 2 error cases, are you sure that's
> right?  Just drop the string if you care about size here.
> 

I'm sure about the size, I've measured it on top of v4.12-rc1 before
replying.

I don't think anyone will copy/paste from this code but if that happen
and is missed by the maintainer, I'm pretty sure someone will be quick
to run his preferred static analysis tool and send a patch (hopefully,
testing it really compiles first).


> thanks,
> 
> greg k-h

-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com