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, 29 Oct 2018 15:55:12 +0000
From:   Vokáč Michal <Michal.Vokac@...ft.com>
To:     Thierry Reding <thierry.reding@...il.com>,
        Uwe Kleine-König 
        <u.kleine-koenig@...gutronix.de>
CC:     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 15.10.2018 10:45, Thierry Reding wrote:
> 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.

Thanks for mentioning the LDD3 Uwe. I almost forgot it exist.
I will dust-off my electronic copy ;)

> 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).

OK, I will leave that as is. And thanks for all the details.
I always learn something new from the comments!

Michal