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@xxxxxxxxxx>
> Cc: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
> Cc: Kees Cook <keescook@xxxxxxxxxxxx>
> Signed-off-by: Guenter Roeck <linux@xxxxxxxxxxxx>
> ---
> 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
