Re: [PATCH] hwmon: (lm90) Fix error return value from detect function

[Kees Cook <keescook@chromium.org>]

On Mon, Aug 08, 2022 at 03:15:04AM -0700, Guenter Roeck wrote:
> lm90_detect_nuvoton() is supposed to return NULL if it can not detect
> a chip, or a pointer to the chip name if it does. Under some circumstances
> it returns an error pointer instead. Some versions of gcc interpret an
> ERR_PTR as region of size 0 and generate an error message.
> 
>   In function ‘__fortify_strlen’,
>       inlined from ‘strlcpy’ at ./include/linux/fortify-string.h:159:10,
>       inlined from ‘lm90_detect’ at drivers/hwmon/lm90.c:2550:2:
>   ./include/linux/fortify-string.h:50:33: error:
>       ‘__builtin_strlen’ reading 1 or more bytes from a region of size 0
>      50 | #define __underlying_strlen     __builtin_strlen
>         |                                 ^
>   ./include/linux/fortify-string.h:141:24: note:
>       in expansion of macro ‘__underlying_strlen’
>     141 |                 return __underlying_strlen(p);
>         |                        ^~~~~~~~~~~~~~~~~~~
> 
> Returning NULL instead of ERR_PTR() fixes the problem.
> 
> Fixes: c7cebce984a2 ("hwmon: (lm90) Rework detect function")
> Reported-by: Ingo Molnar <mingo@kernel.org>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: Kees Cook <keescook@chromium.org>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
> It is interesting that some versions of gcc interpret an ERR_PTR this way.
> It did find a real bug, though the error message is quite confusing.
> Would it be possible to enhance the fortify functions to detect a constant
> ERR_PTR at compile time ? I think that might be quite useful.

Yeah, that should be possible. I suspect something like this might work:

	BUILD_BUG_ON(__builtin_constant_p(src) && IS_ERR_VALUE(src));
	BUILD_BUG_ON(__builtin_constant_p(dst) && IS_ERR_VALUE(dst));

Though I'm not sure how it'd play with GCC value range checker.

-- 
Kees Cook