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
