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

[Thierry Reding]

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

Attachment: signature.asc
Description: PGP signature