On 15.10.2018 10:45, Thierry Reding wrote:

On Sun, Oct 14, 2018 at 10:24:57PM +0200, Uwe Kleine-König wrote:

quoted

Hello,

On Fri, Oct 12, 2018 at 06:08:54PM +0200, Uwe Kleine-König wrote:

quoted

quoted

quoted

quoted

+	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