linux-kernel - Re: [RCF PATCH,v2,2/2] pwm: imx: Configure output to GPIO in disabled state

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] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 15 Oct 2018 10:45:05 +0200
From:   Thierry Reding <thierry.reding@...il.com>
To:     Uwe Kleine-König <u.kleine-koenig@...gutronix.de>
Cc:     Vokáč Michal <Michal.Vokac@...ft.com>,
        Mark Rutland <mark.rutland@....com>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-pwm@...r.kernel.org" <linux-pwm@...r.kernel.org>,
        Lukasz Majewski <l.majewski@...ess.pl>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Rob Herring <robh+dt@...nel.org>,
        "kernel@...gutronix.de" <kernel@...gutronix.de>,
        Fabio Estevam <fabio.estevam@....com>,
        Lothar Waßmann <LW@...o-electronics.de>
Subject: Re: [RCF PATCH,v2,2/2] pwm: imx:
 Configure output to GPIO in disabled state

On Sun, Oct 14, 2018 at 10:24:57PM +0200, Uwe Kleine-König wrote:
> Hello,
> 
> On Fri, Oct 12, 2018 at 06:08:54PM +0200, Uwe Kleine-König wrote:
> > > >> +	if (PTR_ERR(imx_chip->pwm_gpiod) == -EPROBE_DEFER) {
> > > > 
> > > > You must not use PTR_ERR on a value that might not contain an error
> > > > pointer.
> > > 
> > > OK, thank you for valuable info.
> > > So it seems like the I2C folks are in troubles as well:
> > > 
> > > 	https://elixir.bootlin.com/linux/latest/source/drivers/i2c/busses/i2c-imx.c#L996
> > 
> > correct. I cannot find this documented though.
> 
> I found in LDD3[1], chapter 11 on page 295:
> 
> 	If you need the actual error code, it can be extracted with:
> 
> 		long PTR_ERR(const void *ptr);
> 
> 	You should use PTR_ERR only on a value for which IS_ERR returns a true
> 	value; any other value is a valid pointer.
> 
> That is probably where I have my claim from. There is no further
> explanation though, so I'll post a patch adding a comment to the
> definition of PTR_ERR to find out if there is a relevant reason.

Michal's code above does an implicit IS_ERR() by comparing to an actual
error code. It's certainly true that PTR_ERR() on any pointer and then
using that value can be risky because it may not actually be an error.
So if you go and unconditionally print that error code even if it isn't
an error but a valid pointer, you've leaked a kernel address.

However, it's perfectly safe to use PTR_ERR(ptr) == -EPROBE_DEFER (or
for any other error code for that matter).

Thierry

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)