linux-hardening.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v7 0/8] fortify: Add Clang support
@ 2022-02-08 22:53 Kees Cook
  2022-02-08 22:53 ` [PATCH v7 1/8] fortify: Replace open-coded __gnu_inline attribute Kees Cook
                   ` (7 more replies)
  0 siblings, 8 replies; 16+ messages in thread
From: Kees Cook @ 2022-02-08 22:53 UTC (permalink / raw)
  To: Kees Cook
  Cc: Miguel Ojeda, Nick Desaulniers, Nathan Chancellor,
	George Burgess IV, linux-kernel, linux-hardening, llvm

Hi,

This is the updated series for getting Clang to work with
CONFIG_FORTIFY_SOURCE. We went around a few times since v6 on dealing
with -ffreestanding (thank you Nick for figuring out the root cause),
and are dropping X86_32 support until the associated Clang bug is fixed.

I also split up the last patch, since it was doing a bunch of separate
changes, which seemed better as separate patches.

Thanks!

-Kees

v1: https://lore.kernel.org/linux-hardening/20210727205855.411487-61-keescook@chromium.org/
v2: https://lore.kernel.org/linux-hardening/20210818060533.3569517-64-keescook@chromium.org/
v3: https://lore.kernel.org/linux-hardening/20211213223331.135412-18-keescook@chromium.org/
v4: https://lore.kernel.org/linux-hardening/20220130182204.420775-1-keescook@chromium.org/
v5: https://lore.kernel.org/linux-hardening/20220202003033.704951-1-keescook@chromium.org/
v6: https://lore.kernel.org/linux-hardening/20220203173307.1033257-1-keescook@chromium.org/
v7:
 - split last patch into separate logical change patches
 - drop X86_32 support for now


Kees Cook (8):
  fortify: Replace open-coded __gnu_inline attribute
  Compiler Attributes: Add __pass_object_size for Clang
  Compiler Attributes: Add __overloadable for Clang
  Compiler Attributes: Add __diagnose_as for Clang
  fortify: Make pointer arguments const
  fortify: Use __diagnose_as() for better diagnostic coverage
  fortify: Make sure strlen() may still be used as a constant expression
  fortify: Add Clang support

 include/linux/compiler_attributes.h | 39 +++++++++++++++++++
 include/linux/fortify-string.h      | 58 +++++++++++++++++++++--------
 security/Kconfig                    |  5 ++-
 3 files changed, 85 insertions(+), 17 deletions(-)

-- 
2.30.2


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v7 1/8] fortify: Replace open-coded __gnu_inline attribute
  2022-02-08 22:53 [PATCH v7 0/8] fortify: Add Clang support Kees Cook
@ 2022-02-08 22:53 ` Kees Cook
  2022-02-08 22:59   ` Nick Desaulniers
  2022-02-08 22:53 ` [PATCH v7 2/8] Compiler Attributes: Add __pass_object_size for Clang Kees Cook
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 16+ messages in thread
From: Kees Cook @ 2022-02-08 22:53 UTC (permalink / raw)
  To: Kees Cook
  Cc: Miguel Ojeda, Nick Desaulniers, Nathan Chancellor,
	George Burgess IV, linux-kernel, linux-hardening, llvm

Replace open-coded gnu_inline attribute with the normal kernel
convention for attributes: __gnu_inline

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 53123712bb3b..439aad24ab3b 100644
--- a/include/linux/fortify-string.h
+++ b/include/linux/fortify-string.h
@@ -2,7 +2,7 @@
 #ifndef _LINUX_FORTIFY_STRING_H_
 #define _LINUX_FORTIFY_STRING_H_
 
-#define __FORTIFY_INLINE extern __always_inline __attribute__((gnu_inline))
+#define __FORTIFY_INLINE extern __always_inline __gnu_inline
 #define __RENAME(x) __asm__(#x)
 
 void fortify_panic(const char *name) __noreturn __cold;
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v7 2/8] Compiler Attributes: Add __pass_object_size for Clang
  2022-02-08 22:53 [PATCH v7 0/8] fortify: Add Clang support Kees Cook
  2022-02-08 22:53 ` [PATCH v7 1/8] fortify: Replace open-coded __gnu_inline attribute Kees Cook
@ 2022-02-08 22:53 ` Kees Cook
  2022-02-08 23:00   ` Nick Desaulniers
  2022-02-08 22:53 ` [PATCH v7 3/8] Compiler Attributes: Add __overloadable " Kees Cook
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 16+ messages in thread
From: Kees Cook @ 2022-02-08 22:53 UTC (permalink / raw)
  To: Kees Cook
  Cc: Nick Desaulniers, Nathan Chancellor, llvm, Miguel Ojeda,
	George Burgess IV, linux-kernel, linux-hardening

In order to gain greater visibility to type information when using
__builtin_object_size(), Clang has a function attribute "pass_object_size"
that will make size information available for marked arguments in
a function by way of implicit additional function arguments that are
then wired up the __builtin_object_size().

This is needed to implement FORTIFY_SOURCE in Clang, as a workaround
to Clang's __builtin_object_size() having limited visibility[1] into types
across function calls (even inlines).

This attribute has an additional benefit that it can be used even on
non-inline functions to gain argument size information.

[1] https://github.com/llvm/llvm-project/issues/53516

Cc: Nick Desaulniers <ndesaulniers@google.com>
Cc: Nathan Chancellor <nathan@kernel.org>
Cc: llvm@lists.linux.dev
Reviewed-by: Miguel Ojeda <ojeda@kernel.org>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 include/linux/compiler_attributes.h | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/include/linux/compiler_attributes.h b/include/linux/compiler_attributes.h
index 37e260020221..d0c503772061 100644
--- a/include/linux/compiler_attributes.h
+++ b/include/linux/compiler_attributes.h
@@ -263,6 +263,20 @@
  */
 #define __packed                        __attribute__((__packed__))
 
+/*
+ * Note: the "type" argument should match any __builtin_object_size(p, type) usage.
+ *
+ * Optional: not supported by gcc.
+ * Optional: not supported by icc.
+ *
+ * clang: https://clang.llvm.org/docs/AttributeReference.html#pass-object-size-pass-dynamic-object-size
+ */
+#if __has_attribute(__pass_object_size__)
+# define __pass_object_size(type)	__attribute__((__pass_object_size__(type)))
+#else
+# define __pass_object_size(type)
+#endif
+
 /*
  *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-pure-function-attribute
  */
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v7 3/8] Compiler Attributes: Add __overloadable for Clang
  2022-02-08 22:53 [PATCH v7 0/8] fortify: Add Clang support Kees Cook
  2022-02-08 22:53 ` [PATCH v7 1/8] fortify: Replace open-coded __gnu_inline attribute Kees Cook
  2022-02-08 22:53 ` [PATCH v7 2/8] Compiler Attributes: Add __pass_object_size for Clang Kees Cook
@ 2022-02-08 22:53 ` Kees Cook
  2022-02-08 22:53 ` [PATCH v7 4/8] Compiler Attributes: Add __diagnose_as " Kees Cook
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 16+ messages in thread
From: Kees Cook @ 2022-02-08 22:53 UTC (permalink / raw)
  To: Kees Cook
  Cc: Nathan Chancellor, llvm, Miguel Ojeda, Nick Desaulniers,
	George Burgess IV, linux-kernel, linux-hardening

In order for FORTIFY_SOURCE to use __pass_object_size on an "extern
inline" function, as all the fortified string functions are, the functions
must be marked as being overloadable (i.e. different prototypes due
to the implicitly injected object size arguments). This allows the
__pass_object_size versions to take precedence.

Cc: Nathan Chancellor <nathan@kernel.org>
Cc: llvm@lists.linux.dev
Reviewed-by: Miguel Ojeda <ojeda@kernel.org>
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 include/linux/compiler_attributes.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/include/linux/compiler_attributes.h b/include/linux/compiler_attributes.h
index d0c503772061..dcaf55f5d1ae 100644
--- a/include/linux/compiler_attributes.h
+++ b/include/linux/compiler_attributes.h
@@ -257,6 +257,18 @@
  */
 #define __noreturn                      __attribute__((__noreturn__))
 
+/*
+ * Optional: not supported by gcc.
+ * Optional: not supported by icc.
+ *
+ * clang: https://clang.llvm.org/docs/AttributeReference.html#overloadable
+ */
+#if __has_attribute(__overloadable__)
+# define __overloadable			__attribute__((__overloadable__))
+#else
+# define __overloadable
+#endif
+
 /*
  *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Type-Attributes.html#index-packed-type-attribute
  * clang: https://gcc.gnu.org/onlinedocs/gcc/Common-Variable-Attributes.html#index-packed-variable-attribute
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v7 4/8] Compiler Attributes: Add __diagnose_as for Clang
  2022-02-08 22:53 [PATCH v7 0/8] fortify: Add Clang support Kees Cook
                   ` (2 preceding siblings ...)
  2022-02-08 22:53 ` [PATCH v7 3/8] Compiler Attributes: Add __overloadable " Kees Cook
@ 2022-02-08 22:53 ` Kees Cook
  2022-02-08 22:53 ` [PATCH v7 5/8] fortify: Make pointer arguments const Kees Cook
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 16+ messages in thread
From: Kees Cook @ 2022-02-08 22:53 UTC (permalink / raw)
  To: Kees Cook
  Cc: Nathan Chancellor, llvm, Miguel Ojeda, Nick Desaulniers,
	George Burgess IV, linux-kernel, linux-hardening

Clang will perform various compile-time diagnostics on uses of various
functions (e.g. simple bounds-checking on strcpy(), etc). These
diagnostics can be assigned to other functions (for example, new
implementations of the string functions under CONFIG_FORTIFY_SOURCE)
using the "diagnose_as_builtin" attribute. This allows those functions
to retain their compile-time diagnostic warnings.

Cc: Nathan Chancellor <nathan@kernel.org>
Cc: llvm@lists.linux.dev
Reviewed-by: Miguel Ojeda <ojeda@kernel.org>
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 include/linux/compiler_attributes.h | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/include/linux/compiler_attributes.h b/include/linux/compiler_attributes.h
index dcaf55f5d1ae..445e80517cab 100644
--- a/include/linux/compiler_attributes.h
+++ b/include/linux/compiler_attributes.h
@@ -100,6 +100,19 @@
 # define __copy(symbol)
 #endif
 
+/*
+ * Optional: not supported by gcc
+ * Optional: only supported since clang >= 14.0
+ * Optional: not supported by icc
+ *
+ * clang: https://clang.llvm.org/docs/AttributeReference.html#diagnose_as_builtin
+ */
+#if __has_attribute(__diagnose_as_builtin__)
+# define __diagnose_as(builtin...)	__attribute__((__diagnose_as_builtin__(builtin)))
+#else
+# define __diagnose_as(builtin...)
+#endif
+
 /*
  * Don't. Just don't. See commit 771c035372a0 ("deprecate the '__deprecated'
  * attribute warnings entirely and for good") for more information.
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v7 5/8] fortify: Make pointer arguments const
  2022-02-08 22:53 [PATCH v7 0/8] fortify: Add Clang support Kees Cook
                   ` (3 preceding siblings ...)
  2022-02-08 22:53 ` [PATCH v7 4/8] Compiler Attributes: Add __diagnose_as " Kees Cook
@ 2022-02-08 22:53 ` Kees Cook
  2022-02-08 23:02   ` Nick Desaulniers
  2022-02-08 22:53 ` [PATCH v7 6/8] fortify: Use __diagnose_as() for better diagnostic coverage Kees Cook
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 16+ messages in thread
From: Kees Cook @ 2022-02-08 22:53 UTC (permalink / raw)
  To: Kees Cook
  Cc: Miguel Ojeda, Nick Desaulniers, Nathan Chancellor,
	George Burgess IV, linux-kernel, linux-hardening, llvm

In preparation for using Clang's __pass_object_size attribute, make all
the pointer arguments to the fortified string functions const. Nothing
was changing their values anyway, so this added requirement (needed by
__pass_object_size) requires no code changes and has no impact on
the binary instruction output.

Signed-off-by: Kees Cook <keescook@chromium.org>
---
 include/linux/fortify-string.h | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/include/linux/fortify-string.h b/include/linux/fortify-string.h
index 439aad24ab3b..f874ada4b9af 100644
--- a/include/linux/fortify-string.h
+++ b/include/linux/fortify-string.h
@@ -50,7 +50,7 @@ extern char *__underlying_strncpy(char *p, const char *q, __kernel_size_t size)
 #define __underlying_strncpy	__builtin_strncpy
 #endif
 
-__FORTIFY_INLINE char *strncpy(char *p, const char *q, __kernel_size_t size)
+__FORTIFY_INLINE char *strncpy(char * const p, const char *q, __kernel_size_t size)
 {
 	size_t p_size = __builtin_object_size(p, 1);
 
@@ -61,7 +61,7 @@ __FORTIFY_INLINE char *strncpy(char *p, const char *q, __kernel_size_t size)
 	return __underlying_strncpy(p, q, size);
 }
 
-__FORTIFY_INLINE char *strcat(char *p, const char *q)
+__FORTIFY_INLINE char *strcat(char * const p, const char *q)
 {
 	size_t p_size = __builtin_object_size(p, 1);
 
@@ -73,7 +73,7 @@ __FORTIFY_INLINE char *strcat(char *p, const char *q)
 }
 
 extern __kernel_size_t __real_strnlen(const char *, __kernel_size_t) __RENAME(strnlen);
-__FORTIFY_INLINE __kernel_size_t strnlen(const char *p, __kernel_size_t maxlen)
+__FORTIFY_INLINE __kernel_size_t strnlen(const char * const p, __kernel_size_t maxlen)
 {
 	size_t p_size = __builtin_object_size(p, 1);
 	size_t p_len = __compiletime_strlen(p);
@@ -94,7 +94,7 @@ __FORTIFY_INLINE __kernel_size_t strnlen(const char *p, __kernel_size_t maxlen)
 }
 
 /* defined after fortified strnlen to reuse it. */
-__FORTIFY_INLINE __kernel_size_t strlen(const char *p)
+__FORTIFY_INLINE __kernel_size_t strlen(const char * const p)
 {
 	__kernel_size_t ret;
 	size_t p_size = __builtin_object_size(p, 1);
@@ -110,7 +110,7 @@ __FORTIFY_INLINE __kernel_size_t strlen(const char *p)
 
 /* defined after fortified strlen to reuse it */
 extern size_t __real_strlcpy(char *, const char *, size_t) __RENAME(strlcpy);
-__FORTIFY_INLINE size_t strlcpy(char *p, const char *q, size_t size)
+__FORTIFY_INLINE size_t strlcpy(char * const p, const char * const q, size_t size)
 {
 	size_t p_size = __builtin_object_size(p, 1);
 	size_t q_size = __builtin_object_size(q, 1);
@@ -137,7 +137,7 @@ __FORTIFY_INLINE size_t strlcpy(char *p, const char *q, size_t size)
 
 /* defined after fortified strnlen to reuse it */
 extern ssize_t __real_strscpy(char *, const char *, size_t) __RENAME(strscpy);
-__FORTIFY_INLINE ssize_t strscpy(char *p, const char *q, size_t size)
+__FORTIFY_INLINE ssize_t strscpy(char * const p, const char * const q, size_t size)
 {
 	size_t len;
 	/* Use string size rather than possible enclosing struct size. */
@@ -183,7 +183,7 @@ __FORTIFY_INLINE ssize_t strscpy(char *p, const char *q, size_t size)
 }
 
 /* defined after fortified strlen and strnlen to reuse them */
-__FORTIFY_INLINE char *strncat(char *p, const char *q, __kernel_size_t count)
+__FORTIFY_INLINE char *strncat(char * const p, const char * const q, __kernel_size_t count)
 {
 	size_t p_len, copy_len;
 	size_t p_size = __builtin_object_size(p, 1);
@@ -354,7 +354,7 @@ __FORTIFY_INLINE void fortify_memcpy_chk(__kernel_size_t size,
 		memmove)
 
 extern void *__real_memscan(void *, int, __kernel_size_t) __RENAME(memscan);
-__FORTIFY_INLINE void *memscan(void *p, int c, __kernel_size_t size)
+__FORTIFY_INLINE void *memscan(void * const p, int c, __kernel_size_t size)
 {
 	size_t p_size = __builtin_object_size(p, 0);
 
@@ -365,7 +365,7 @@ __FORTIFY_INLINE void *memscan(void *p, int c, __kernel_size_t size)
 	return __real_memscan(p, c, size);
 }
 
-__FORTIFY_INLINE int memcmp(const void *p, const void *q, __kernel_size_t size)
+__FORTIFY_INLINE int memcmp(const void * const p, const void * const q, __kernel_size_t size)
 {
 	size_t p_size = __builtin_object_size(p, 0);
 	size_t q_size = __builtin_object_size(q, 0);
@@ -381,7 +381,7 @@ __FORTIFY_INLINE int memcmp(const void *p, const void *q, __kernel_size_t size)
 	return __underlying_memcmp(p, q, size);
 }
 
-__FORTIFY_INLINE void *memchr(const void *p, int c, __kernel_size_t size)
+__FORTIFY_INLINE void *memchr(const void * const p, int c, __kernel_size_t size)
 {
 	size_t p_size = __builtin_object_size(p, 0);
 
@@ -393,7 +393,7 @@ __FORTIFY_INLINE void *memchr(const void *p, int c, __kernel_size_t size)
 }
 
 void *__real_memchr_inv(const void *s, int c, size_t n) __RENAME(memchr_inv);
-__FORTIFY_INLINE void *memchr_inv(const void *p, int c, size_t size)
+__FORTIFY_INLINE void *memchr_inv(const void * const p, int c, size_t size)
 {
 	size_t p_size = __builtin_object_size(p, 0);
 
@@ -405,7 +405,7 @@ __FORTIFY_INLINE void *memchr_inv(const void *p, int c, size_t size)
 }
 
 extern void *__real_kmemdup(const void *src, size_t len, gfp_t gfp) __RENAME(kmemdup);
-__FORTIFY_INLINE void *kmemdup(const void *p, size_t size, gfp_t gfp)
+__FORTIFY_INLINE void *kmemdup(const void * const p, size_t size, gfp_t gfp)
 {
 	size_t p_size = __builtin_object_size(p, 0);
 
@@ -417,7 +417,7 @@ __FORTIFY_INLINE void *kmemdup(const void *p, size_t size, gfp_t gfp)
 }
 
 /* Defined after fortified strlen to reuse it. */
-__FORTIFY_INLINE char *strcpy(char *p, const char *q)
+__FORTIFY_INLINE char *strcpy(char * const p, const char * const q)
 {
 	size_t p_size = __builtin_object_size(p, 1);
 	size_t q_size = __builtin_object_size(q, 1);
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v7 6/8] fortify: Use __diagnose_as() for better diagnostic coverage
  2022-02-08 22:53 [PATCH v7 0/8] fortify: Add Clang support Kees Cook
                   ` (4 preceding siblings ...)
  2022-02-08 22:53 ` [PATCH v7 5/8] fortify: Make pointer arguments const Kees Cook
@ 2022-02-08 22:53 ` Kees Cook
  2022-02-08 23:04   ` Nick Desaulniers
  2022-02-08 22:53 ` [PATCH v7 7/8] fortify: Make sure strlen() may still be used as a constant expression Kees Cook
  2022-02-08 22:53 ` [PATCH v7 8/8] fortify: Add Clang support Kees Cook
  7 siblings, 1 reply; 16+ messages in thread
From: Kees Cook @ 2022-02-08 22:53 UTC (permalink / raw)
  To: Kees Cook
  Cc: Miguel Ojeda, Nick Desaulniers, Nathan Chancellor,
	George Burgess IV, linux-kernel, linux-hardening, llvm

In preparation for using Clang's __pass_object_size, add __diagnose_as()
attributes to mark the functions as being the same as the indicated
builtins. When __daignose_as() is available, Clang will have a more
complete ability to apply its own diagnostic analysis to callers of these
functions, as if they were the builtins themselves. Without __diagnose_as,
Clang's compile time diagnostic messages won't be as precise as they
could be, but at least users of older toolchains will still benefit from
having fortified routines.

Signed-off-by: Kees Cook <keescook@chromium.org>
---
 include/linux/fortify-string.h | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/include/linux/fortify-string.h b/include/linux/fortify-string.h
index f874ada4b9af..db1ad1c1c79a 100644
--- a/include/linux/fortify-string.h
+++ b/include/linux/fortify-string.h
@@ -50,7 +50,8 @@ extern char *__underlying_strncpy(char *p, const char *q, __kernel_size_t size)
 #define __underlying_strncpy	__builtin_strncpy
 #endif
 
-__FORTIFY_INLINE char *strncpy(char * const p, const char *q, __kernel_size_t size)
+__FORTIFY_INLINE __diagnose_as(__builtin_strncpy, 1, 2, 3)
+char *strncpy(char * const p, const char *q, __kernel_size_t size)
 {
 	size_t p_size = __builtin_object_size(p, 1);
 
@@ -61,7 +62,8 @@ __FORTIFY_INLINE char *strncpy(char * const p, const char *q, __kernel_size_t si
 	return __underlying_strncpy(p, q, size);
 }
 
-__FORTIFY_INLINE char *strcat(char * const p, const char *q)
+__FORTIFY_INLINE __diagnose_as(__builtin_strcat, 1, 2)
+char *strcat(char * const p, const char *q)
 {
 	size_t p_size = __builtin_object_size(p, 1);
 
@@ -94,7 +96,8 @@ __FORTIFY_INLINE __kernel_size_t strnlen(const char * const p, __kernel_size_t m
 }
 
 /* defined after fortified strnlen to reuse it. */
-__FORTIFY_INLINE __kernel_size_t strlen(const char * const p)
+__FORTIFY_INLINE __diagnose_as(__builtin_strlen, 1)
+__kernel_size_t strlen(const char * const p)
 {
 	__kernel_size_t ret;
 	size_t p_size = __builtin_object_size(p, 1);
@@ -183,7 +186,8 @@ __FORTIFY_INLINE ssize_t strscpy(char * const p, const char * const q, size_t si
 }
 
 /* defined after fortified strlen and strnlen to reuse them */
-__FORTIFY_INLINE char *strncat(char * const p, const char * const q, __kernel_size_t count)
+__FORTIFY_INLINE __diagnose_as(__builtin_strncat, 1, 2, 3)
+char *strncat(char * const p, const char * const q, __kernel_size_t count)
 {
 	size_t p_len, copy_len;
 	size_t p_size = __builtin_object_size(p, 1);
@@ -365,7 +369,8 @@ __FORTIFY_INLINE void *memscan(void * const p, int c, __kernel_size_t size)
 	return __real_memscan(p, c, size);
 }
 
-__FORTIFY_INLINE int memcmp(const void * const p, const void * const q, __kernel_size_t size)
+__FORTIFY_INLINE __diagnose_as(__builtin_memcmp, 1, 2, 3)
+int memcmp(const void * const p, const void * const q, __kernel_size_t size)
 {
 	size_t p_size = __builtin_object_size(p, 0);
 	size_t q_size = __builtin_object_size(q, 0);
@@ -381,7 +386,8 @@ __FORTIFY_INLINE int memcmp(const void * const p, const void * const q, __kernel
 	return __underlying_memcmp(p, q, size);
 }
 
-__FORTIFY_INLINE void *memchr(const void * const p, int c, __kernel_size_t size)
+__FORTIFY_INLINE __diagnose_as(__builtin_memchr, 1, 2, 3)
+void *memchr(const void * const p, int c, __kernel_size_t size)
 {
 	size_t p_size = __builtin_object_size(p, 0);
 
@@ -417,7 +423,8 @@ __FORTIFY_INLINE void *kmemdup(const void * const p, size_t size, gfp_t gfp)
 }
 
 /* Defined after fortified strlen to reuse it. */
-__FORTIFY_INLINE char *strcpy(char * const p, const char * const q)
+__FORTIFY_INLINE __diagnose_as(__builtin_strcpy, 1, 2)
+char *strcpy(char * const p, const char * const q)
 {
 	size_t p_size = __builtin_object_size(p, 1);
 	size_t q_size = __builtin_object_size(q, 1);
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v7 7/8] fortify: Make sure strlen() may still be used as a constant expression
  2022-02-08 22:53 [PATCH v7 0/8] fortify: Add Clang support Kees Cook
                   ` (5 preceding siblings ...)
  2022-02-08 22:53 ` [PATCH v7 6/8] fortify: Use __diagnose_as() for better diagnostic coverage Kees Cook
@ 2022-02-08 22:53 ` Kees Cook
  2022-02-08 23:17   ` Nick Desaulniers
  2022-02-08 22:53 ` [PATCH v7 8/8] fortify: Add Clang support Kees Cook
  7 siblings, 1 reply; 16+ messages in thread
From: Kees Cook @ 2022-02-08 22:53 UTC (permalink / raw)
  To: Kees Cook
  Cc: Miguel Ojeda, Nick Desaulniers, Nathan Chancellor,
	George Burgess IV, linux-kernel, linux-hardening, llvm

In preparation for enabling Clang FORTIFY_SOURCE support, redefine
strlen() as a macro that tests for being a constant expression
so that strlen() can still be used in static initializers, which is
lost when adding __pass_object_size and __overloadable.

An example of this usage can be seen here:
	https://lore.kernel.org/all/202201252321.dRmWZ8wW-lkp@intel.com/

Notably, this constant expression feature of strlen() is not available
for architectures that build with -ffreestanding. This means the kernel
currently does not universally expect strlen() to be used this way, but
since there _are_ some build configurations that depend on it, retain
the characteristic for Clang FORTIFY_SOURCE builds too.

Signed-off-by: Kees Cook <keescook@chromium.org>
---
 include/linux/fortify-string.h | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/include/linux/fortify-string.h b/include/linux/fortify-string.h
index db1ad1c1c79a..f77cf22e2d60 100644
--- a/include/linux/fortify-string.h
+++ b/include/linux/fortify-string.h
@@ -2,6 +2,8 @@
 #ifndef _LINUX_FORTIFY_STRING_H_
 #define _LINUX_FORTIFY_STRING_H_
 
+#include <linux/const.h>
+
 #define __FORTIFY_INLINE extern __always_inline __gnu_inline
 #define __RENAME(x) __asm__(#x)
 
@@ -95,9 +97,16 @@ __FORTIFY_INLINE __kernel_size_t strnlen(const char * const p, __kernel_size_t m
 	return ret;
 }
 
-/* defined after fortified strnlen to reuse it. */
+/*
+ * Defined after fortified strnlen to reuse it. However, it must still be
+ * possible for strlen() to be used on compile-time strings for use in
+ * static initializers (i.e. as a constant expression).
+ */
+#define strlen(p)							\
+	__builtin_choose_expr(__is_constexpr(__builtin_strlen(p)),	\
+		__builtin_strlen(p), __fortify_strlen(p))
 __FORTIFY_INLINE __diagnose_as(__builtin_strlen, 1)
-__kernel_size_t strlen(const char * const p)
+__kernel_size_t __fortify_strlen(const char * const p)
 {
 	__kernel_size_t ret;
 	size_t p_size = __builtin_object_size(p, 1);
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v7 8/8] fortify: Add Clang support
  2022-02-08 22:53 [PATCH v7 0/8] fortify: Add Clang support Kees Cook
                   ` (6 preceding siblings ...)
  2022-02-08 22:53 ` [PATCH v7 7/8] fortify: Make sure strlen() may still be used as a constant expression Kees Cook
@ 2022-02-08 22:53 ` Kees Cook
  2022-02-08 23:25   ` Nick Desaulniers
  7 siblings, 1 reply; 16+ messages in thread
From: Kees Cook @ 2022-02-08 22:53 UTC (permalink / raw)
  To: Kees Cook
  Cc: Miguel Ojeda, Nick Desaulniers, Nathan Chancellor,
	George Burgess IV, llvm, linux-kernel, linux-hardening

Enable FORTIFY_SOURCE support for Clang:

Use the new __pass_object_size and __overloadable attributes so that
Clang will have appropriate visibility into argument sizes such that
__builtin_object_size(p, 1) will behave correctly. Additional details
available here:
    https://github.com/llvm/llvm-project/issues/53516
    https://github.com/ClangBuiltLinux/linux/issues/1401

A bug with __builtin_constant_p() of globally defined variables was
fixed in Clang 13 (and backported to 12.0.1), so FORTIFY support must
depend on that version or later. Additional details here:
    https://bugs.llvm.org/show_bug.cgi?id=41459
    commit a52f8a59aef4 ("fortify: Explicitly disable Clang support")

A bug with Clang's -mregparm=3 and -m32 makes some builtins unusable,
so removing -ffreestanding (to gain the needed libcall optimizations
with Clang) cannot be done. Without the libcall optimizations, Clang
cannot provide appropriate FORTIFY coverage, so it must be disabled
for CONFIG_X86_32. Additional details here;
    https://github.com/llvm/llvm-project/issues/53645

Cc: Miguel Ojeda <ojeda@kernel.org>
Cc: Nick Desaulniers <ndesaulniers@google.com>
Cc: Nathan Chancellor <nathan@kernel.org>
Cc: George Burgess IV <gbiv@google.com>
Cc: llvm@lists.linux.dev
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 include/linux/fortify-string.h | 40 ++++++++++++++++++++++------------
 security/Kconfig               |  5 +++--
 2 files changed, 29 insertions(+), 16 deletions(-)

diff --git a/include/linux/fortify-string.h b/include/linux/fortify-string.h
index f77cf22e2d60..295637a66c46 100644
--- a/include/linux/fortify-string.h
+++ b/include/linux/fortify-string.h
@@ -4,7 +4,7 @@
 
 #include <linux/const.h>
 
-#define __FORTIFY_INLINE extern __always_inline __gnu_inline
+#define __FORTIFY_INLINE extern __always_inline __gnu_inline __overloadable
 #define __RENAME(x) __asm__(#x)
 
 void fortify_panic(const char *name) __noreturn __cold;
@@ -52,8 +52,17 @@ extern char *__underlying_strncpy(char *p, const char *q, __kernel_size_t size)
 #define __underlying_strncpy	__builtin_strncpy
 #endif
 
+/*
+ * Clang's use of __builtin_object_size() within inlines needs hinting via
+ * __pass_object_size(). The preference is to only ever use type 1 (member
+ * size, rather than struct size), but there remain some stragglers using
+ * type 0 that will be converted in the future.
+ */
+#define POS	__pass_object_size(1)
+#define POS0	__pass_object_size(0)
+
 __FORTIFY_INLINE __diagnose_as(__builtin_strncpy, 1, 2, 3)
-char *strncpy(char * const p, const char *q, __kernel_size_t size)
+char *strncpy(char * const POS p, const char *q, __kernel_size_t size)
 {
 	size_t p_size = __builtin_object_size(p, 1);
 
@@ -65,7 +74,7 @@ char *strncpy(char * const p, const char *q, __kernel_size_t size)
 }
 
 __FORTIFY_INLINE __diagnose_as(__builtin_strcat, 1, 2)
-char *strcat(char * const p, const char *q)
+char *strcat(char * const POS p, const char *q)
 {
 	size_t p_size = __builtin_object_size(p, 1);
 
@@ -77,7 +86,7 @@ char *strcat(char * const p, const char *q)
 }
 
 extern __kernel_size_t __real_strnlen(const char *, __kernel_size_t) __RENAME(strnlen);
-__FORTIFY_INLINE __kernel_size_t strnlen(const char * const p, __kernel_size_t maxlen)
+__FORTIFY_INLINE __kernel_size_t strnlen(const char * const POS p, __kernel_size_t maxlen)
 {
 	size_t p_size = __builtin_object_size(p, 1);
 	size_t p_len = __compiletime_strlen(p);
@@ -106,7 +115,7 @@ __FORTIFY_INLINE __kernel_size_t strnlen(const char * const p, __kernel_size_t m
 	__builtin_choose_expr(__is_constexpr(__builtin_strlen(p)),	\
 		__builtin_strlen(p), __fortify_strlen(p))
 __FORTIFY_INLINE __diagnose_as(__builtin_strlen, 1)
-__kernel_size_t __fortify_strlen(const char * const p)
+__kernel_size_t __fortify_strlen(const char * const POS p)
 {
 	__kernel_size_t ret;
 	size_t p_size = __builtin_object_size(p, 1);
@@ -122,7 +131,7 @@ __kernel_size_t __fortify_strlen(const char * const p)
 
 /* defined after fortified strlen to reuse it */
 extern size_t __real_strlcpy(char *, const char *, size_t) __RENAME(strlcpy);
-__FORTIFY_INLINE size_t strlcpy(char * const p, const char * const q, size_t size)
+__FORTIFY_INLINE size_t strlcpy(char * const POS p, const char * const POS q, size_t size)
 {
 	size_t p_size = __builtin_object_size(p, 1);
 	size_t q_size = __builtin_object_size(q, 1);
@@ -149,7 +158,7 @@ __FORTIFY_INLINE size_t strlcpy(char * const p, const char * const q, size_t siz
 
 /* defined after fortified strnlen to reuse it */
 extern ssize_t __real_strscpy(char *, const char *, size_t) __RENAME(strscpy);
-__FORTIFY_INLINE ssize_t strscpy(char * const p, const char * const q, size_t size)
+__FORTIFY_INLINE ssize_t strscpy(char * const POS p, const char * const POS q, size_t size)
 {
 	size_t len;
 	/* Use string size rather than possible enclosing struct size. */
@@ -196,7 +205,7 @@ __FORTIFY_INLINE ssize_t strscpy(char * const p, const char * const q, size_t si
 
 /* defined after fortified strlen and strnlen to reuse them */
 __FORTIFY_INLINE __diagnose_as(__builtin_strncat, 1, 2, 3)
-char *strncat(char * const p, const char * const q, __kernel_size_t count)
+char *strncat(char * const POS p, const char * const POS q, __kernel_size_t count)
 {
 	size_t p_len, copy_len;
 	size_t p_size = __builtin_object_size(p, 1);
@@ -367,7 +376,7 @@ __FORTIFY_INLINE void fortify_memcpy_chk(__kernel_size_t size,
 		memmove)
 
 extern void *__real_memscan(void *, int, __kernel_size_t) __RENAME(memscan);
-__FORTIFY_INLINE void *memscan(void * const p, int c, __kernel_size_t size)
+__FORTIFY_INLINE void *memscan(void * const POS0 p, int c, __kernel_size_t size)
 {
 	size_t p_size = __builtin_object_size(p, 0);
 
@@ -379,7 +388,7 @@ __FORTIFY_INLINE void *memscan(void * const p, int c, __kernel_size_t size)
 }
 
 __FORTIFY_INLINE __diagnose_as(__builtin_memcmp, 1, 2, 3)
-int memcmp(const void * const p, const void * const q, __kernel_size_t size)
+int memcmp(const void * const POS0 p, const void * const POS0 q, __kernel_size_t size)
 {
 	size_t p_size = __builtin_object_size(p, 0);
 	size_t q_size = __builtin_object_size(q, 0);
@@ -396,7 +405,7 @@ int memcmp(const void * const p, const void * const q, __kernel_size_t size)
 }
 
 __FORTIFY_INLINE __diagnose_as(__builtin_memchr, 1, 2, 3)
-void *memchr(const void * const p, int c, __kernel_size_t size)
+void *memchr(const void * const POS0 p, int c, __kernel_size_t size)
 {
 	size_t p_size = __builtin_object_size(p, 0);
 
@@ -408,7 +417,7 @@ void *memchr(const void * const p, int c, __kernel_size_t size)
 }
 
 void *__real_memchr_inv(const void *s, int c, size_t n) __RENAME(memchr_inv);
-__FORTIFY_INLINE void *memchr_inv(const void * const p, int c, size_t size)
+__FORTIFY_INLINE void *memchr_inv(const void * const POS0 p, int c, size_t size)
 {
 	size_t p_size = __builtin_object_size(p, 0);
 
@@ -420,7 +429,7 @@ __FORTIFY_INLINE void *memchr_inv(const void * const p, int c, size_t size)
 }
 
 extern void *__real_kmemdup(const void *src, size_t len, gfp_t gfp) __RENAME(kmemdup);
-__FORTIFY_INLINE void *kmemdup(const void * const p, size_t size, gfp_t gfp)
+__FORTIFY_INLINE void *kmemdup(const void * const POS0 p, size_t size, gfp_t gfp)
 {
 	size_t p_size = __builtin_object_size(p, 0);
 
@@ -433,7 +442,7 @@ __FORTIFY_INLINE void *kmemdup(const void * const p, size_t size, gfp_t gfp)
 
 /* Defined after fortified strlen to reuse it. */
 __FORTIFY_INLINE __diagnose_as(__builtin_strcpy, 1, 2)
-char *strcpy(char * const p, const char * const q)
+char *strcpy(char * const POS p, const char * const POS q)
 {
 	size_t p_size = __builtin_object_size(p, 1);
 	size_t q_size = __builtin_object_size(q, 1);
@@ -462,4 +471,7 @@ char *strcpy(char * const p, const char * const q)
 #undef __underlying_strncat
 #undef __underlying_strncpy
 
+#undef POS
+#undef POS0
+
 #endif /* _LINUX_FORTIFY_STRING_H_ */
diff --git a/security/Kconfig b/security/Kconfig
index 0b847f435beb..1d2d71cc1f36 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -177,9 +177,10 @@ config HARDENED_USERCOPY_PAGESPAN
 config FORTIFY_SOURCE
 	bool "Harden common str/mem functions against buffer overflows"
 	depends on ARCH_HAS_FORTIFY_SOURCE
-	# https://bugs.llvm.org/show_bug.cgi?id=50322
 	# https://bugs.llvm.org/show_bug.cgi?id=41459
-	depends on !CC_IS_CLANG
+	depends on !CC_IS_CLANG || CLANG_VERSION >= 120001
+	# https://github.com/llvm/llvm-project/issues/53645
+	depends on !CC_IS_CLANG || !X86_32
 	help
 	  Detect overflows of buffers in common string and memory functions
 	  where the compiler can determine and validate the buffer sizes.
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH v7 1/8] fortify: Replace open-coded __gnu_inline attribute
  2022-02-08 22:53 ` [PATCH v7 1/8] fortify: Replace open-coded __gnu_inline attribute Kees Cook
@ 2022-02-08 22:59   ` Nick Desaulniers
  0 siblings, 0 replies; 16+ messages in thread
From: Nick Desaulniers @ 2022-02-08 22:59 UTC (permalink / raw)
  To: Kees Cook
  Cc: Miguel Ojeda, Nathan Chancellor, George Burgess IV, linux-kernel,
	linux-hardening, llvm

On Tue, Feb 8, 2022 at 2:53 PM Kees Cook <keescook@chromium.org> wrote:
>
> Replace open-coded gnu_inline attribute with the normal kernel
> convention for attributes: __gnu_inline

Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

>
> 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 53123712bb3b..439aad24ab3b 100644
> --- a/include/linux/fortify-string.h
> +++ b/include/linux/fortify-string.h
> @@ -2,7 +2,7 @@
>  #ifndef _LINUX_FORTIFY_STRING_H_
>  #define _LINUX_FORTIFY_STRING_H_
>
> -#define __FORTIFY_INLINE extern __always_inline __attribute__((gnu_inline))
> +#define __FORTIFY_INLINE extern __always_inline __gnu_inline
>  #define __RENAME(x) __asm__(#x)
>
>  void fortify_panic(const char *name) __noreturn __cold;
> --
> 2.30.2
>
>


-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v7 2/8] Compiler Attributes: Add __pass_object_size for Clang
  2022-02-08 22:53 ` [PATCH v7 2/8] Compiler Attributes: Add __pass_object_size for Clang Kees Cook
@ 2022-02-08 23:00   ` Nick Desaulniers
  0 siblings, 0 replies; 16+ messages in thread
From: Nick Desaulniers @ 2022-02-08 23:00 UTC (permalink / raw)
  To: Kees Cook
  Cc: Nathan Chancellor, llvm, Miguel Ojeda, George Burgess IV,
	linux-kernel, linux-hardening

On Tue, Feb 8, 2022 at 2:53 PM Kees Cook <keescook@chromium.org> wrote:
>
> In order to gain greater visibility to type information when using
> __builtin_object_size(), Clang has a function attribute "pass_object_size"
> that will make size information available for marked arguments in
> a function by way of implicit additional function arguments that are
> then wired up the __builtin_object_size().
>
> This is needed to implement FORTIFY_SOURCE in Clang, as a workaround
> to Clang's __builtin_object_size() having limited visibility[1] into types
> across function calls (even inlines).
>
> This attribute has an additional benefit that it can be used even on
> non-inline functions to gain argument size information.
>
> [1] https://github.com/llvm/llvm-project/issues/53516

Thanks for the patch!
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

>
> Cc: Nick Desaulniers <ndesaulniers@google.com>
> Cc: Nathan Chancellor <nathan@kernel.org>
> Cc: llvm@lists.linux.dev
> Reviewed-by: Miguel Ojeda <ojeda@kernel.org>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
>  include/linux/compiler_attributes.h | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
>
> diff --git a/include/linux/compiler_attributes.h b/include/linux/compiler_attributes.h
> index 37e260020221..d0c503772061 100644
> --- a/include/linux/compiler_attributes.h
> +++ b/include/linux/compiler_attributes.h
> @@ -263,6 +263,20 @@
>   */
>  #define __packed                        __attribute__((__packed__))
>
> +/*
> + * Note: the "type" argument should match any __builtin_object_size(p, type) usage.
> + *
> + * Optional: not supported by gcc.
> + * Optional: not supported by icc.
> + *
> + * clang: https://clang.llvm.org/docs/AttributeReference.html#pass-object-size-pass-dynamic-object-size
> + */
> +#if __has_attribute(__pass_object_size__)
> +# define __pass_object_size(type)      __attribute__((__pass_object_size__(type)))
> +#else
> +# define __pass_object_size(type)
> +#endif
> +
>  /*
>   *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-pure-function-attribute
>   */
> --
> 2.30.2
>
>


-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v7 5/8] fortify: Make pointer arguments const
  2022-02-08 22:53 ` [PATCH v7 5/8] fortify: Make pointer arguments const Kees Cook
@ 2022-02-08 23:02   ` Nick Desaulniers
  0 siblings, 0 replies; 16+ messages in thread
From: Nick Desaulniers @ 2022-02-08 23:02 UTC (permalink / raw)
  To: Kees Cook
  Cc: Miguel Ojeda, Nathan Chancellor, George Burgess IV, linux-kernel,
	linux-hardening, llvm

On Tue, Feb 8, 2022 at 2:53 PM Kees Cook <keescook@chromium.org> wrote:
>
> In preparation for using Clang's __pass_object_size attribute, make all
> the pointer arguments to the fortified string functions const. Nothing
> was changing their values anyway, so this added requirement (needed by
> __pass_object_size) requires no code changes and has no impact on
> the binary instruction output.

Gracias Señor!
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
>  include/linux/fortify-string.h | 26 +++++++++++++-------------
>  1 file changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/include/linux/fortify-string.h b/include/linux/fortify-string.h
> index 439aad24ab3b..f874ada4b9af 100644
> --- a/include/linux/fortify-string.h
> +++ b/include/linux/fortify-string.h
> @@ -50,7 +50,7 @@ extern char *__underlying_strncpy(char *p, const char *q, __kernel_size_t size)
>  #define __underlying_strncpy   __builtin_strncpy
>  #endif
>
> -__FORTIFY_INLINE char *strncpy(char *p, const char *q, __kernel_size_t size)
> +__FORTIFY_INLINE char *strncpy(char * const p, const char *q, __kernel_size_t size)
>  {
>         size_t p_size = __builtin_object_size(p, 1);
>
> @@ -61,7 +61,7 @@ __FORTIFY_INLINE char *strncpy(char *p, const char *q, __kernel_size_t size)
>         return __underlying_strncpy(p, q, size);
>  }
>
> -__FORTIFY_INLINE char *strcat(char *p, const char *q)
> +__FORTIFY_INLINE char *strcat(char * const p, const char *q)
>  {
>         size_t p_size = __builtin_object_size(p, 1);
>
> @@ -73,7 +73,7 @@ __FORTIFY_INLINE char *strcat(char *p, const char *q)
>  }
>
>  extern __kernel_size_t __real_strnlen(const char *, __kernel_size_t) __RENAME(strnlen);
> -__FORTIFY_INLINE __kernel_size_t strnlen(const char *p, __kernel_size_t maxlen)
> +__FORTIFY_INLINE __kernel_size_t strnlen(const char * const p, __kernel_size_t maxlen)
>  {
>         size_t p_size = __builtin_object_size(p, 1);
>         size_t p_len = __compiletime_strlen(p);
> @@ -94,7 +94,7 @@ __FORTIFY_INLINE __kernel_size_t strnlen(const char *p, __kernel_size_t maxlen)
>  }
>
>  /* defined after fortified strnlen to reuse it. */
> -__FORTIFY_INLINE __kernel_size_t strlen(const char *p)
> +__FORTIFY_INLINE __kernel_size_t strlen(const char * const p)
>  {
>         __kernel_size_t ret;
>         size_t p_size = __builtin_object_size(p, 1);
> @@ -110,7 +110,7 @@ __FORTIFY_INLINE __kernel_size_t strlen(const char *p)
>
>  /* defined after fortified strlen to reuse it */
>  extern size_t __real_strlcpy(char *, const char *, size_t) __RENAME(strlcpy);
> -__FORTIFY_INLINE size_t strlcpy(char *p, const char *q, size_t size)
> +__FORTIFY_INLINE size_t strlcpy(char * const p, const char * const q, size_t size)
>  {
>         size_t p_size = __builtin_object_size(p, 1);
>         size_t q_size = __builtin_object_size(q, 1);
> @@ -137,7 +137,7 @@ __FORTIFY_INLINE size_t strlcpy(char *p, const char *q, size_t size)
>
>  /* defined after fortified strnlen to reuse it */
>  extern ssize_t __real_strscpy(char *, const char *, size_t) __RENAME(strscpy);
> -__FORTIFY_INLINE ssize_t strscpy(char *p, const char *q, size_t size)
> +__FORTIFY_INLINE ssize_t strscpy(char * const p, const char * const q, size_t size)
>  {
>         size_t len;
>         /* Use string size rather than possible enclosing struct size. */
> @@ -183,7 +183,7 @@ __FORTIFY_INLINE ssize_t strscpy(char *p, const char *q, size_t size)
>  }
>
>  /* defined after fortified strlen and strnlen to reuse them */
> -__FORTIFY_INLINE char *strncat(char *p, const char *q, __kernel_size_t count)
> +__FORTIFY_INLINE char *strncat(char * const p, const char * const q, __kernel_size_t count)
>  {
>         size_t p_len, copy_len;
>         size_t p_size = __builtin_object_size(p, 1);
> @@ -354,7 +354,7 @@ __FORTIFY_INLINE void fortify_memcpy_chk(__kernel_size_t size,
>                 memmove)
>
>  extern void *__real_memscan(void *, int, __kernel_size_t) __RENAME(memscan);
> -__FORTIFY_INLINE void *memscan(void *p, int c, __kernel_size_t size)
> +__FORTIFY_INLINE void *memscan(void * const p, int c, __kernel_size_t size)
>  {
>         size_t p_size = __builtin_object_size(p, 0);
>
> @@ -365,7 +365,7 @@ __FORTIFY_INLINE void *memscan(void *p, int c, __kernel_size_t size)
>         return __real_memscan(p, c, size);
>  }
>
> -__FORTIFY_INLINE int memcmp(const void *p, const void *q, __kernel_size_t size)
> +__FORTIFY_INLINE int memcmp(const void * const p, const void * const q, __kernel_size_t size)
>  {
>         size_t p_size = __builtin_object_size(p, 0);
>         size_t q_size = __builtin_object_size(q, 0);
> @@ -381,7 +381,7 @@ __FORTIFY_INLINE int memcmp(const void *p, const void *q, __kernel_size_t size)
>         return __underlying_memcmp(p, q, size);
>  }
>
> -__FORTIFY_INLINE void *memchr(const void *p, int c, __kernel_size_t size)
> +__FORTIFY_INLINE void *memchr(const void * const p, int c, __kernel_size_t size)
>  {
>         size_t p_size = __builtin_object_size(p, 0);
>
> @@ -393,7 +393,7 @@ __FORTIFY_INLINE void *memchr(const void *p, int c, __kernel_size_t size)
>  }
>
>  void *__real_memchr_inv(const void *s, int c, size_t n) __RENAME(memchr_inv);
> -__FORTIFY_INLINE void *memchr_inv(const void *p, int c, size_t size)
> +__FORTIFY_INLINE void *memchr_inv(const void * const p, int c, size_t size)
>  {
>         size_t p_size = __builtin_object_size(p, 0);
>
> @@ -405,7 +405,7 @@ __FORTIFY_INLINE void *memchr_inv(const void *p, int c, size_t size)
>  }
>
>  extern void *__real_kmemdup(const void *src, size_t len, gfp_t gfp) __RENAME(kmemdup);
> -__FORTIFY_INLINE void *kmemdup(const void *p, size_t size, gfp_t gfp)
> +__FORTIFY_INLINE void *kmemdup(const void * const p, size_t size, gfp_t gfp)
>  {
>         size_t p_size = __builtin_object_size(p, 0);
>
> @@ -417,7 +417,7 @@ __FORTIFY_INLINE void *kmemdup(const void *p, size_t size, gfp_t gfp)
>  }
>
>  /* Defined after fortified strlen to reuse it. */
> -__FORTIFY_INLINE char *strcpy(char *p, const char *q)
> +__FORTIFY_INLINE char *strcpy(char * const p, const char * const q)
>  {
>         size_t p_size = __builtin_object_size(p, 1);
>         size_t q_size = __builtin_object_size(q, 1);
> --
> 2.30.2
>
>


-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v7 6/8] fortify: Use __diagnose_as() for better diagnostic coverage
  2022-02-08 22:53 ` [PATCH v7 6/8] fortify: Use __diagnose_as() for better diagnostic coverage Kees Cook
@ 2022-02-08 23:04   ` Nick Desaulniers
  0 siblings, 0 replies; 16+ messages in thread
From: Nick Desaulniers @ 2022-02-08 23:04 UTC (permalink / raw)
  To: Kees Cook
  Cc: Miguel Ojeda, Nathan Chancellor, George Burgess IV, linux-kernel,
	linux-hardening, llvm

On Tue, Feb 8, 2022 at 2:53 PM Kees Cook <keescook@chromium.org> wrote:
>
> In preparation for using Clang's __pass_object_size, add __diagnose_as()
> attributes to mark the functions as being the same as the indicated
> builtins. When __daignose_as() is available, Clang will have a more
> complete ability to apply its own diagnostic analysis to callers of these
> functions, as if they were the builtins themselves. Without __diagnose_as,
> Clang's compile time diagnostic messages won't be as precise as they
> could be, but at least users of older toolchains will still benefit from
> having fortified routines.

Yes, much easier to review split up. Thanks for taking the time to do so!
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
>  include/linux/fortify-string.h | 21 ++++++++++++++-------
>  1 file changed, 14 insertions(+), 7 deletions(-)
>
> diff --git a/include/linux/fortify-string.h b/include/linux/fortify-string.h
> index f874ada4b9af..db1ad1c1c79a 100644
> --- a/include/linux/fortify-string.h
> +++ b/include/linux/fortify-string.h
> @@ -50,7 +50,8 @@ extern char *__underlying_strncpy(char *p, const char *q, __kernel_size_t size)
>  #define __underlying_strncpy   __builtin_strncpy
>  #endif
>
> -__FORTIFY_INLINE char *strncpy(char * const p, const char *q, __kernel_size_t size)
> +__FORTIFY_INLINE __diagnose_as(__builtin_strncpy, 1, 2, 3)
> +char *strncpy(char * const p, const char *q, __kernel_size_t size)
>  {
>         size_t p_size = __builtin_object_size(p, 1);
>
> @@ -61,7 +62,8 @@ __FORTIFY_INLINE char *strncpy(char * const p, const char *q, __kernel_size_t si
>         return __underlying_strncpy(p, q, size);
>  }
>
> -__FORTIFY_INLINE char *strcat(char * const p, const char *q)
> +__FORTIFY_INLINE __diagnose_as(__builtin_strcat, 1, 2)
> +char *strcat(char * const p, const char *q)
>  {
>         size_t p_size = __builtin_object_size(p, 1);
>
> @@ -94,7 +96,8 @@ __FORTIFY_INLINE __kernel_size_t strnlen(const char * const p, __kernel_size_t m
>  }
>
>  /* defined after fortified strnlen to reuse it. */
> -__FORTIFY_INLINE __kernel_size_t strlen(const char * const p)
> +__FORTIFY_INLINE __diagnose_as(__builtin_strlen, 1)
> +__kernel_size_t strlen(const char * const p)
>  {
>         __kernel_size_t ret;
>         size_t p_size = __builtin_object_size(p, 1);
> @@ -183,7 +186,8 @@ __FORTIFY_INLINE ssize_t strscpy(char * const p, const char * const q, size_t si
>  }
>
>  /* defined after fortified strlen and strnlen to reuse them */
> -__FORTIFY_INLINE char *strncat(char * const p, const char * const q, __kernel_size_t count)
> +__FORTIFY_INLINE __diagnose_as(__builtin_strncat, 1, 2, 3)
> +char *strncat(char * const p, const char * const q, __kernel_size_t count)
>  {
>         size_t p_len, copy_len;
>         size_t p_size = __builtin_object_size(p, 1);
> @@ -365,7 +369,8 @@ __FORTIFY_INLINE void *memscan(void * const p, int c, __kernel_size_t size)
>         return __real_memscan(p, c, size);
>  }
>
> -__FORTIFY_INLINE int memcmp(const void * const p, const void * const q, __kernel_size_t size)
> +__FORTIFY_INLINE __diagnose_as(__builtin_memcmp, 1, 2, 3)
> +int memcmp(const void * const p, const void * const q, __kernel_size_t size)
>  {
>         size_t p_size = __builtin_object_size(p, 0);
>         size_t q_size = __builtin_object_size(q, 0);
> @@ -381,7 +386,8 @@ __FORTIFY_INLINE int memcmp(const void * const p, const void * const q, __kernel
>         return __underlying_memcmp(p, q, size);
>  }
>
> -__FORTIFY_INLINE void *memchr(const void * const p, int c, __kernel_size_t size)
> +__FORTIFY_INLINE __diagnose_as(__builtin_memchr, 1, 2, 3)
> +void *memchr(const void * const p, int c, __kernel_size_t size)
>  {
>         size_t p_size = __builtin_object_size(p, 0);
>
> @@ -417,7 +423,8 @@ __FORTIFY_INLINE void *kmemdup(const void * const p, size_t size, gfp_t gfp)
>  }
>
>  /* Defined after fortified strlen to reuse it. */
> -__FORTIFY_INLINE char *strcpy(char * const p, const char * const q)
> +__FORTIFY_INLINE __diagnose_as(__builtin_strcpy, 1, 2)
> +char *strcpy(char * const p, const char * const q)
>  {
>         size_t p_size = __builtin_object_size(p, 1);
>         size_t q_size = __builtin_object_size(q, 1);
> --
> 2.30.2
>


-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v7 7/8] fortify: Make sure strlen() may still be used as a constant expression
  2022-02-08 22:53 ` [PATCH v7 7/8] fortify: Make sure strlen() may still be used as a constant expression Kees Cook
@ 2022-02-08 23:17   ` Nick Desaulniers
  2022-02-08 23:58     ` Kees Cook
  0 siblings, 1 reply; 16+ messages in thread
From: Nick Desaulniers @ 2022-02-08 23:17 UTC (permalink / raw)
  To: Kees Cook
  Cc: Miguel Ojeda, Nathan Chancellor, George Burgess IV, linux-kernel,
	linux-hardening, llvm

On Tue, Feb 8, 2022 at 2:53 PM Kees Cook <keescook@chromium.org> wrote:
>
> In preparation for enabling Clang FORTIFY_SOURCE support, redefine
> strlen() as a macro that tests for being a constant expression
> so that strlen() can still be used in static initializers, which is
> lost when adding __pass_object_size and __overloadable.
>
> An example of this usage can be seen here:
>         https://lore.kernel.org/all/202201252321.dRmWZ8wW-lkp@intel.com/
>
> Notably, this constant expression feature of strlen() is not available
> for architectures that build with -ffreestanding. This means the kernel
> currently does not universally expect strlen() to be used this way, but
> since there _are_ some build configurations that depend on it, retain
> the characteristic for Clang FORTIFY_SOURCE builds too.
>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
>  include/linux/fortify-string.h | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/fortify-string.h b/include/linux/fortify-string.h
> index db1ad1c1c79a..f77cf22e2d60 100644
> --- a/include/linux/fortify-string.h
> +++ b/include/linux/fortify-string.h
> @@ -2,6 +2,8 @@
>  #ifndef _LINUX_FORTIFY_STRING_H_
>  #define _LINUX_FORTIFY_STRING_H_
>
> +#include <linux/const.h>
> +
>  #define __FORTIFY_INLINE extern __always_inline __gnu_inline
>  #define __RENAME(x) __asm__(#x)
>
> @@ -95,9 +97,16 @@ __FORTIFY_INLINE __kernel_size_t strnlen(const char * const p, __kernel_size_t m
>         return ret;
>  }
>
> -/* defined after fortified strnlen to reuse it. */
> +/*
> + * Defined after fortified strnlen to reuse it. However, it must still be
> + * possible for strlen() to be used on compile-time strings for use in
> + * static initializers (i.e. as a constant expression).
> + */
> +#define strlen(p)                                                      \
> +       __builtin_choose_expr(__is_constexpr(__builtin_strlen(p)),      \

Is `__is_constexpr(p) == __is_constexpr(__builtin_strlen(p))`? i.e.
can we drop the first `__builtin_strlen`? It seems redundant.

So instead, we'd have:

#define strlen(p) __builtin_choose_expr(__is_constexpr(p),
__builtin_strlen(p), __fortify_strlen(p))

Or is there some funny business where p isn't constexpr but strlen(p)
somehow is? I doubt that.  (Or is it that p is constexpr, but
strlen(p) is not?)

(Guess I'm wrong: https://godbolt.org/z/19ffz7vjx)

Ok then.
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

> +               __builtin_strlen(p), __fortify_strlen(p))
>  __FORTIFY_INLINE __diagnose_as(__builtin_strlen, 1)
> -__kernel_size_t strlen(const char * const p)
> +__kernel_size_t __fortify_strlen(const char * const p)
>  {
>         __kernel_size_t ret;
>         size_t p_size = __builtin_object_size(p, 1);
> --
> 2.30.2
>


-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v7 8/8] fortify: Add Clang support
  2022-02-08 22:53 ` [PATCH v7 8/8] fortify: Add Clang support Kees Cook
@ 2022-02-08 23:25   ` Nick Desaulniers
  0 siblings, 0 replies; 16+ messages in thread
From: Nick Desaulniers @ 2022-02-08 23:25 UTC (permalink / raw)
  To: Kees Cook
  Cc: Miguel Ojeda, Nathan Chancellor, George Burgess IV, llvm,
	linux-kernel, linux-hardening

On Tue, Feb 8, 2022 at 2:53 PM Kees Cook <keescook@chromium.org> wrote:
>
> Enable FORTIFY_SOURCE support for Clang:
>
> Use the new __pass_object_size and __overloadable attributes so that
> Clang will have appropriate visibility into argument sizes such that
> __builtin_object_size(p, 1) will behave correctly. Additional details
> available here:
>     https://github.com/llvm/llvm-project/issues/53516
>     https://github.com/ClangBuiltLinux/linux/issues/1401
>
> A bug with __builtin_constant_p() of globally defined variables was
> fixed in Clang 13 (and backported to 12.0.1), so FORTIFY support must
> depend on that version or later. Additional details here:
>     https://bugs.llvm.org/show_bug.cgi?id=41459
>     commit a52f8a59aef4 ("fortify: Explicitly disable Clang support")
>
> A bug with Clang's -mregparm=3 and -m32 makes some builtins unusable,
> so removing -ffreestanding (to gain the needed libcall optimizations
> with Clang) cannot be done. Without the libcall optimizations, Clang
> cannot provide appropriate FORTIFY coverage, so it must be disabled
> for CONFIG_X86_32. Additional details here;
>     https://github.com/llvm/llvm-project/issues/53645

Nice job on this series Kees! Hopefully we can get i386 support working next!
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

>
> Cc: Miguel Ojeda <ojeda@kernel.org>
> Cc: Nick Desaulniers <ndesaulniers@google.com>
> Cc: Nathan Chancellor <nathan@kernel.org>
> Cc: George Burgess IV <gbiv@google.com>
> Cc: llvm@lists.linux.dev
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
>  include/linux/fortify-string.h | 40 ++++++++++++++++++++++------------
>  security/Kconfig               |  5 +++--
>  2 files changed, 29 insertions(+), 16 deletions(-)
>
> diff --git a/include/linux/fortify-string.h b/include/linux/fortify-string.h
> index f77cf22e2d60..295637a66c46 100644
> --- a/include/linux/fortify-string.h
> +++ b/include/linux/fortify-string.h
> @@ -4,7 +4,7 @@
>
>  #include <linux/const.h>
>
> -#define __FORTIFY_INLINE extern __always_inline __gnu_inline
> +#define __FORTIFY_INLINE extern __always_inline __gnu_inline __overloadable
>  #define __RENAME(x) __asm__(#x)
>
>  void fortify_panic(const char *name) __noreturn __cold;
> @@ -52,8 +52,17 @@ extern char *__underlying_strncpy(char *p, const char *q, __kernel_size_t size)
>  #define __underlying_strncpy   __builtin_strncpy
>  #endif
>
> +/*
> + * Clang's use of __builtin_object_size() within inlines needs hinting via
> + * __pass_object_size(). The preference is to only ever use type 1 (member
> + * size, rather than struct size), but there remain some stragglers using
> + * type 0 that will be converted in the future.
> + */
> +#define POS    __pass_object_size(1)
> +#define POS0   __pass_object_size(0)
> +
>  __FORTIFY_INLINE __diagnose_as(__builtin_strncpy, 1, 2, 3)
> -char *strncpy(char * const p, const char *q, __kernel_size_t size)
> +char *strncpy(char * const POS p, const char *q, __kernel_size_t size)
>  {
>         size_t p_size = __builtin_object_size(p, 1);
>
> @@ -65,7 +74,7 @@ char *strncpy(char * const p, const char *q, __kernel_size_t size)
>  }
>
>  __FORTIFY_INLINE __diagnose_as(__builtin_strcat, 1, 2)
> -char *strcat(char * const p, const char *q)
> +char *strcat(char * const POS p, const char *q)
>  {
>         size_t p_size = __builtin_object_size(p, 1);
>
> @@ -77,7 +86,7 @@ char *strcat(char * const p, const char *q)
>  }
>
>  extern __kernel_size_t __real_strnlen(const char *, __kernel_size_t) __RENAME(strnlen);
> -__FORTIFY_INLINE __kernel_size_t strnlen(const char * const p, __kernel_size_t maxlen)
> +__FORTIFY_INLINE __kernel_size_t strnlen(const char * const POS p, __kernel_size_t maxlen)
>  {
>         size_t p_size = __builtin_object_size(p, 1);
>         size_t p_len = __compiletime_strlen(p);
> @@ -106,7 +115,7 @@ __FORTIFY_INLINE __kernel_size_t strnlen(const char * const p, __kernel_size_t m
>         __builtin_choose_expr(__is_constexpr(__builtin_strlen(p)),      \
>                 __builtin_strlen(p), __fortify_strlen(p))
>  __FORTIFY_INLINE __diagnose_as(__builtin_strlen, 1)
> -__kernel_size_t __fortify_strlen(const char * const p)
> +__kernel_size_t __fortify_strlen(const char * const POS p)
>  {
>         __kernel_size_t ret;
>         size_t p_size = __builtin_object_size(p, 1);
> @@ -122,7 +131,7 @@ __kernel_size_t __fortify_strlen(const char * const p)
>
>  /* defined after fortified strlen to reuse it */
>  extern size_t __real_strlcpy(char *, const char *, size_t) __RENAME(strlcpy);
> -__FORTIFY_INLINE size_t strlcpy(char * const p, const char * const q, size_t size)
> +__FORTIFY_INLINE size_t strlcpy(char * const POS p, const char * const POS q, size_t size)
>  {
>         size_t p_size = __builtin_object_size(p, 1);
>         size_t q_size = __builtin_object_size(q, 1);
> @@ -149,7 +158,7 @@ __FORTIFY_INLINE size_t strlcpy(char * const p, const char * const q, size_t siz
>
>  /* defined after fortified strnlen to reuse it */
>  extern ssize_t __real_strscpy(char *, const char *, size_t) __RENAME(strscpy);
> -__FORTIFY_INLINE ssize_t strscpy(char * const p, const char * const q, size_t size)
> +__FORTIFY_INLINE ssize_t strscpy(char * const POS p, const char * const POS q, size_t size)
>  {
>         size_t len;
>         /* Use string size rather than possible enclosing struct size. */
> @@ -196,7 +205,7 @@ __FORTIFY_INLINE ssize_t strscpy(char * const p, const char * const q, size_t si
>
>  /* defined after fortified strlen and strnlen to reuse them */
>  __FORTIFY_INLINE __diagnose_as(__builtin_strncat, 1, 2, 3)
> -char *strncat(char * const p, const char * const q, __kernel_size_t count)
> +char *strncat(char * const POS p, const char * const POS q, __kernel_size_t count)
>  {
>         size_t p_len, copy_len;
>         size_t p_size = __builtin_object_size(p, 1);
> @@ -367,7 +376,7 @@ __FORTIFY_INLINE void fortify_memcpy_chk(__kernel_size_t size,
>                 memmove)
>
>  extern void *__real_memscan(void *, int, __kernel_size_t) __RENAME(memscan);
> -__FORTIFY_INLINE void *memscan(void * const p, int c, __kernel_size_t size)
> +__FORTIFY_INLINE void *memscan(void * const POS0 p, int c, __kernel_size_t size)
>  {
>         size_t p_size = __builtin_object_size(p, 0);
>
> @@ -379,7 +388,7 @@ __FORTIFY_INLINE void *memscan(void * const p, int c, __kernel_size_t size)
>  }
>
>  __FORTIFY_INLINE __diagnose_as(__builtin_memcmp, 1, 2, 3)
> -int memcmp(const void * const p, const void * const q, __kernel_size_t size)
> +int memcmp(const void * const POS0 p, const void * const POS0 q, __kernel_size_t size)
>  {
>         size_t p_size = __builtin_object_size(p, 0);
>         size_t q_size = __builtin_object_size(q, 0);
> @@ -396,7 +405,7 @@ int memcmp(const void * const p, const void * const q, __kernel_size_t size)
>  }
>
>  __FORTIFY_INLINE __diagnose_as(__builtin_memchr, 1, 2, 3)
> -void *memchr(const void * const p, int c, __kernel_size_t size)
> +void *memchr(const void * const POS0 p, int c, __kernel_size_t size)
>  {
>         size_t p_size = __builtin_object_size(p, 0);
>
> @@ -408,7 +417,7 @@ void *memchr(const void * const p, int c, __kernel_size_t size)
>  }
>
>  void *__real_memchr_inv(const void *s, int c, size_t n) __RENAME(memchr_inv);
> -__FORTIFY_INLINE void *memchr_inv(const void * const p, int c, size_t size)
> +__FORTIFY_INLINE void *memchr_inv(const void * const POS0 p, int c, size_t size)
>  {
>         size_t p_size = __builtin_object_size(p, 0);
>
> @@ -420,7 +429,7 @@ __FORTIFY_INLINE void *memchr_inv(const void * const p, int c, size_t size)
>  }
>
>  extern void *__real_kmemdup(const void *src, size_t len, gfp_t gfp) __RENAME(kmemdup);
> -__FORTIFY_INLINE void *kmemdup(const void * const p, size_t size, gfp_t gfp)
> +__FORTIFY_INLINE void *kmemdup(const void * const POS0 p, size_t size, gfp_t gfp)
>  {
>         size_t p_size = __builtin_object_size(p, 0);
>
> @@ -433,7 +442,7 @@ __FORTIFY_INLINE void *kmemdup(const void * const p, size_t size, gfp_t gfp)
>
>  /* Defined after fortified strlen to reuse it. */
>  __FORTIFY_INLINE __diagnose_as(__builtin_strcpy, 1, 2)
> -char *strcpy(char * const p, const char * const q)
> +char *strcpy(char * const POS p, const char * const POS q)
>  {
>         size_t p_size = __builtin_object_size(p, 1);
>         size_t q_size = __builtin_object_size(q, 1);
> @@ -462,4 +471,7 @@ char *strcpy(char * const p, const char * const q)
>  #undef __underlying_strncat
>  #undef __underlying_strncpy
>
> +#undef POS
> +#undef POS0
> +
>  #endif /* _LINUX_FORTIFY_STRING_H_ */
> diff --git a/security/Kconfig b/security/Kconfig
> index 0b847f435beb..1d2d71cc1f36 100644
> --- a/security/Kconfig
> +++ b/security/Kconfig
> @@ -177,9 +177,10 @@ config HARDENED_USERCOPY_PAGESPAN
>  config FORTIFY_SOURCE
>         bool "Harden common str/mem functions against buffer overflows"
>         depends on ARCH_HAS_FORTIFY_SOURCE
> -       # https://bugs.llvm.org/show_bug.cgi?id=50322
>         # https://bugs.llvm.org/show_bug.cgi?id=41459
> -       depends on !CC_IS_CLANG
> +       depends on !CC_IS_CLANG || CLANG_VERSION >= 120001
> +       # https://github.com/llvm/llvm-project/issues/53645
> +       depends on !CC_IS_CLANG || !X86_32
>         help
>           Detect overflows of buffers in common string and memory functions
>           where the compiler can determine and validate the buffer sizes.
> --
> 2.30.2
>
>


-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v7 7/8] fortify: Make sure strlen() may still be used as a constant expression
  2022-02-08 23:17   ` Nick Desaulniers
@ 2022-02-08 23:58     ` Kees Cook
  0 siblings, 0 replies; 16+ messages in thread
From: Kees Cook @ 2022-02-08 23:58 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: Miguel Ojeda, Nathan Chancellor, George Burgess IV, linux-kernel,
	linux-hardening, llvm

On Tue, Feb 08, 2022 at 03:17:13PM -0800, Nick Desaulniers wrote:
> On Tue, Feb 8, 2022 at 2:53 PM Kees Cook <keescook@chromium.org> wrote:
> >
> > In preparation for enabling Clang FORTIFY_SOURCE support, redefine
> > strlen() as a macro that tests for being a constant expression
> > so that strlen() can still be used in static initializers, which is
> > lost when adding __pass_object_size and __overloadable.
> >
> > An example of this usage can be seen here:
> >         https://lore.kernel.org/all/202201252321.dRmWZ8wW-lkp@intel.com/
> >
> > Notably, this constant expression feature of strlen() is not available
> > for architectures that build with -ffreestanding. This means the kernel
> > currently does not universally expect strlen() to be used this way, but
> > since there _are_ some build configurations that depend on it, retain
> > the characteristic for Clang FORTIFY_SOURCE builds too.
> >
> > Signed-off-by: Kees Cook <keescook@chromium.org>
> > ---
> >  include/linux/fortify-string.h | 13 +++++++++++--
> >  1 file changed, 11 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/linux/fortify-string.h b/include/linux/fortify-string.h
> > index db1ad1c1c79a..f77cf22e2d60 100644
> > --- a/include/linux/fortify-string.h
> > +++ b/include/linux/fortify-string.h
> > @@ -2,6 +2,8 @@
> >  #ifndef _LINUX_FORTIFY_STRING_H_
> >  #define _LINUX_FORTIFY_STRING_H_
> >
> > +#include <linux/const.h>
> > +
> >  #define __FORTIFY_INLINE extern __always_inline __gnu_inline
> >  #define __RENAME(x) __asm__(#x)
> >
> > @@ -95,9 +97,16 @@ __FORTIFY_INLINE __kernel_size_t strnlen(const char * const p, __kernel_size_t m
> >         return ret;
> >  }
> >
> > -/* defined after fortified strnlen to reuse it. */
> > +/*
> > + * Defined after fortified strnlen to reuse it. However, it must still be
> > + * possible for strlen() to be used on compile-time strings for use in
> > + * static initializers (i.e. as a constant expression).
> > + */
> > +#define strlen(p)                                                      \
> > +       __builtin_choose_expr(__is_constexpr(__builtin_strlen(p)),      \
> 
> Is `__is_constexpr(p) == __is_constexpr(__builtin_strlen(p))`? i.e.
> can we drop the first `__builtin_strlen`? It seems redundant.
> 
> So instead, we'd have:
> 
> #define strlen(p) __builtin_choose_expr(__is_constexpr(p),
> __builtin_strlen(p), __fortify_strlen(p))
> 
> Or is there some funny business where p isn't constexpr but strlen(p)
> somehow is? I doubt that.  (Or is it that p is constexpr, but
> strlen(p) is not?)
> 
> (Guess I'm wrong: https://godbolt.org/z/19ffz7vjx)

Yeah, as you've discovered ... funny business. :P

> Ok then.
> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

Thanks!

-- 
Kees Cook

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2022-02-08 23:58 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-08 22:53 [PATCH v7 0/8] fortify: Add Clang support Kees Cook
2022-02-08 22:53 ` [PATCH v7 1/8] fortify: Replace open-coded __gnu_inline attribute Kees Cook
2022-02-08 22:59   ` Nick Desaulniers
2022-02-08 22:53 ` [PATCH v7 2/8] Compiler Attributes: Add __pass_object_size for Clang Kees Cook
2022-02-08 23:00   ` Nick Desaulniers
2022-02-08 22:53 ` [PATCH v7 3/8] Compiler Attributes: Add __overloadable " Kees Cook
2022-02-08 22:53 ` [PATCH v7 4/8] Compiler Attributes: Add __diagnose_as " Kees Cook
2022-02-08 22:53 ` [PATCH v7 5/8] fortify: Make pointer arguments const Kees Cook
2022-02-08 23:02   ` Nick Desaulniers
2022-02-08 22:53 ` [PATCH v7 6/8] fortify: Use __diagnose_as() for better diagnostic coverage Kees Cook
2022-02-08 23:04   ` Nick Desaulniers
2022-02-08 22:53 ` [PATCH v7 7/8] fortify: Make sure strlen() may still be used as a constant expression Kees Cook
2022-02-08 23:17   ` Nick Desaulniers
2022-02-08 23:58     ` Kees Cook
2022-02-08 22:53 ` [PATCH v7 8/8] fortify: Add Clang support Kees Cook
2022-02-08 23:25   ` Nick Desaulniers

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).