| Message ID | 20221025231627.never.000-kees@kernel.org (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | e9a40e1585d792751d3a122392695e5a53032809 |
| Headers | show |
| Series | fortify: Do not cast to "unsigned char" | expand |
On Tue, Oct 25, 2022 at 4:17 PM Kees Cook <keescook@chromium.org> wrote: > > Do not cast to "unsigned char", as this needlessly creates type problems > when attempting builds without -Wno-pointer-sign[1]. The intent of the > cast is to drop possible "const" types. > > [1] https://lore.kernel.org/lkml/CAHk-=wgz3Uba8w7kdXhsqR1qvfemYL+OFQdefJnkeqXG8qZ_pA@mail.gmail.com/ > > Suggested-by: Linus Torvalds <torvalds@linux-foundation.org> > Fixes: 3009f891bb9f ("fortify: Allow strlen() and strnlen() to pass compile-time known lengths") > Cc: Nick Desaulniers <ndesaulniers@google.com> > Cc: linux-hardening@vger.kernel.org > Signed-off-by: Kees Cook <keescook@chromium.org> > --- > include/linux/fortify-string.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/include/linux/fortify-string.h b/include/linux/fortify-string.h > index f3dd5d1a6a25..09a032f6ce6b 100644 > --- a/include/linux/fortify-string.h > +++ b/include/linux/fortify-string.h > @@ -18,7 +18,7 @@ void __write_overflow_field(size_t avail, size_t wanted) __compiletime_warning(" > > #define __compiletime_strlen(p) \ > ({ \ > - unsigned char *__p = (unsigned char *)(p); \ > + char *__p = (char *)(p); \ If the intent of __p is to avoid repeated application of side effects from the evaluation of the macro parameter p, this could also be: __auto_type __p = (p); I think this is nice because it doesn't strip potential const qualifiers from the macro parameter. > size_t __ret = SIZE_MAX; \ > size_t __p_size = __member_size(p); \ > if (__p_size != SIZE_MAX && \ > -- > 2.34.1 >
On Wed, Oct 26, 2022 at 11:26 AM Nick Desaulniers <ndesaulniers@google.com> wrote: > > If the intent of __p is to avoid repeated application of side effects > from the evaluation of the macro parameter p, this could also be: Not the only intent. The code also does __builtin_constant_p(*__p)) which basically checks for "is this a compile-time constant string" (yes, I realize that it only checks the first character, but it ends up being the same thing). That would fail horribly with __auto_type and people using "void *". It is true that we could probably use __auto_type in a lot of other places where we currently use __typeof__(ptr) _p_ = (ptr) but our "typeof" pattern is (a) much more common because of historical reasons (b) much more generic because it often uses a different ptr type (ie macros that have a value and a pointer, like "put_user()", the type comes from the pointer, but the initializer comes from the value, so "__auto_type" ends up being completely the wrong thing). We do have a couple of "__auto_type" uses, but because it can't replace our __typeof__ users in general anyway, I'm not convinced we should strive to make it hugely more common (even if the __auto_type model probably can replace a lot of them). Linus
diff --git a/include/linux/fortify-string.h b/include/linux/fortify-string.h index f3dd5d1a6a25..09a032f6ce6b 100644 --- a/include/linux/fortify-string.h +++ b/include/linux/fortify-string.h @@ -18,7 +18,7 @@ void __write_overflow_field(size_t avail, size_t wanted) __compiletime_warning(" #define __compiletime_strlen(p) \ ({ \ - unsigned char *__p = (unsigned char *)(p); \ + char *__p = (char *)(p); \ size_t __ret = SIZE_MAX; \ size_t __p_size = __member_size(p); \ if (__p_size != SIZE_MAX && \
Do not cast to "unsigned char", as this needlessly creates type problems when attempting builds without -Wno-pointer-sign[1]. The intent of the cast is to drop possible "const" types. [1] https://lore.kernel.org/lkml/CAHk-=wgz3Uba8w7kdXhsqR1qvfemYL+OFQdefJnkeqXG8qZ_pA@mail.gmail.com/ Suggested-by: Linus Torvalds <torvalds@linux-foundation.org> Fixes: 3009f891bb9f ("fortify: Allow strlen() and strnlen() to pass compile-time known lengths") Cc: Nick Desaulniers <ndesaulniers@google.com> Cc: linux-hardening@vger.kernel.org Signed-off-by: Kees Cook <keescook@chromium.org> --- include/linux/fortify-string.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)