On Tue, Aug 09, 2022 at 03:24:04PM -0700, Kees Cook wrote: > 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> Reviewed-by: 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. Yeah, looks like this doesn't work. These are all only able to check for a single value. The GCC diagnostics depend on its internal value range checking. This tripped because of the (sometimes buggy) "void * cast of a literal value is always a NULL pointer dereference, so its size must always be zero" which we've had to repeatedly work around. In this case, it was a real error, though. :P I'm hoping we can teach future GCC "treat literal casts in range $foo-$bar to be NULL derefs", and we can hand it the ERR_PTR range. -Kees -- Kees Cook