Hi, So, after looking at v4 at little longer I decided that it is just too invasive. After spending time researching the primary issue that needed to be worked around (__builtin_object_size(p, 1) not working from inlines), I got some help from gbiv to use some Clang-specific attributes to get the same effect. I think the result is much less invasive, and it even lets us easily expand size verification coverage into non-inlines if we ever want to. Please take a look. :) -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: - rewritten to use Clang attributes Kees Cook (4): Compiler Attributes: Add Clang's __pass_object_size Compiler Attributes: Add __overloadable Compiler Attributes: Add __diagnose_as fortify: Add Clang support include/linux/compiler_attributes.h | 29 ++++++++++++++++ include/linux/fortify-string.h | 52 ++++++++++++++++++++--------- security/Kconfig | 2 +- 3 files changed, 67 insertions(+), 16 deletions(-) -- 2.30.2
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 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: Miguel Ojeda <ojeda@kernel.org> Cc: Nick Desaulniers <ndesaulniers@google.com> Cc: Nathan Chancellor <nathan@kernel.org> Cc: llvm@lists.linux.dev Signed-off-by: Kees Cook <keescook@chromium.org> --- include/linux/compiler_attributes.h | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/include/linux/compiler_attributes.h b/include/linux/compiler_attributes.h index 37e260020221..cc751e0770f5 100644 --- a/include/linux/compiler_attributes.h +++ b/include/linux/compiler_attributes.h @@ -263,6 +263,17 @@ */ #define __packed __attribute__((__packed__)) +/* + * clang: https://clang.llvm.org/docs/AttributeReference.html#pass-object-size-pass-dynamic-object-size + * + * The "type" argument should match the __builtin_object_size(p, type) usage. + */ +#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
In order for FORTIFY_SOURCE to use __pass_object_size on an "inline extern" function, as all the fortified string functions are, the functions must be marked as being overloadable (i.e. different prototypes). This allows the __pass_object_size versions to take precedence. Cc: Miguel Ojeda <ojeda@kernel.org> Cc: Nick Desaulniers <ndesaulniers@google.com> Cc: Nathan Chancellor <nathan@kernel.org> Cc: llvm@lists.linux.dev Signed-off-by: Kees Cook <keescook@chromium.org> --- include/linux/compiler_attributes.h | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/include/linux/compiler_attributes.h b/include/linux/compiler_attributes.h index cc751e0770f5..51e063347fd6 100644 --- a/include/linux/compiler_attributes.h +++ b/include/linux/compiler_attributes.h @@ -257,6 +257,15 @@ */ #define __noreturn __attribute__((__noreturn__)) +/* + * 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
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: Miguel Ojeda <ojeda@kernel.org> Cc: Nick Desaulniers <ndesaulniers@google.com> Cc: Nathan Chancellor <nathan@kernel.org> Cc: llvm@lists.linux.dev Signed-off-by: Kees Cook <keescook@chromium.org> --- include/linux/compiler_attributes.h | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/include/linux/compiler_attributes.h b/include/linux/compiler_attributes.h index 51e063347fd6..b4ae0d771302 100644 --- a/include/linux/compiler_attributes.h +++ b/include/linux/compiler_attributes.h @@ -100,6 +100,15 @@ # define __copy(symbol) #endif +/* + * 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
Enable FORTIFY_SOURCE support for Clang: Use the new __pass_object_size and __overloadable attributes so that Clang will have appropriate visibility into argument types such that __builtin_object_size(p, 1) will behave correctly. Additional details here: https://github.com/llvm/llvm-project/issues/53516 https://github.com/ClangBuiltLinux/linux/issues/1401 Use the new __diagnose_as attribute to make sure no compile-time diagnostic warnings are lost due to the effectively renamed string functions. Redefine strlen() as a macro that tests for being a constant expression so that strlen() can still be used in static initializers, which was lost when adding __pass_object_size and __overloadable. Finally, a bug with __builtin_constant_p() of globally defined variables was fixed in Clang 13, 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") 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 | 52 ++++++++++++++++++++++++---------- security/Kconfig | 2 +- 2 files changed, 38 insertions(+), 16 deletions(-) diff --git a/include/linux/fortify-string.h b/include/linux/fortify-string.h index c45159dbdaa1..5482536d3197 100644 --- a/include/linux/fortify-string.h +++ b/include/linux/fortify-string.h @@ -2,7 +2,9 @@ #ifndef _LINUX_FORTIFY_STRING_H_ #define _LINUX_FORTIFY_STRING_H_ -#define __FORTIFY_INLINE extern __always_inline __attribute__((gnu_inline)) +#include <linux/const.h> + +#define __FORTIFY_INLINE extern __always_inline __attribute__((gnu_inline)) __overloadable #define __RENAME(x) __asm__(#x) void fortify_panic(const char *name) __noreturn __cold; @@ -50,7 +52,11 @@ 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) +#define BOS const __pass_object_size(1) +#define BOS0 const __pass_object_size(0) + +__FORTIFY_INLINE __diagnose_as(__builtin_strncpy, 1, 2, 3) +char *strncpy(char * BOS p, const char *q, __kernel_size_t size) { size_t p_size = __builtin_object_size(p, 1); @@ -61,7 +67,8 @@ __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 __diagnose_as(__builtin_strcat, 1, 2) +char *strcat(char * BOS p, const char *q) { size_t p_size = __builtin_object_size(p, 1); @@ -73,7 +80,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 * BOS p, __kernel_size_t maxlen) { size_t p_size = __builtin_object_size(p, 1); size_t p_len = __compiletime_strlen(p); @@ -93,8 +100,16 @@ __FORTIFY_INLINE __kernel_size_t strnlen(const char *p, __kernel_size_t maxlen) return ret; } -/* defined after fortified strnlen to reuse it. */ -__FORTIFY_INLINE __kernel_size_t strlen(const char *p) +/* + * 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 __fortify_strlen(const char * BOS p) { __kernel_size_t ret; size_t p_size = __builtin_object_size(p, 1); @@ -110,7 +125,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 * BOS p, const char * BOS q, size_t size) { size_t p_size = __builtin_object_size(p, 1); size_t q_size = __builtin_object_size(q, 1); @@ -137,7 +152,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 * BOS p, const char * BOS q, size_t size) { size_t len; /* Use string size rather than possible enclosing struct size. */ @@ -183,7 +198,8 @@ __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 __diagnose_as(__builtin_strncat, 1, 2, 3) +char *strncat(char * BOS p, const char * BOS q, __kernel_size_t count) { size_t p_len, copy_len; size_t p_size = __builtin_object_size(p, 1); @@ -354,7 +370,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 * BOS0 p, int c, __kernel_size_t size) { size_t p_size = __builtin_object_size(p, 0); @@ -365,7 +381,8 @@ __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 __diagnose_as(__builtin_memcmp, 1, 2, 3) +int memcmp(const void * BOS0 p, const void * BOS0 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 +398,8 @@ __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 __diagnose_as(__builtin_memchr, 1, 2, 3) +void *memchr(const void * BOS0 p, int c, __kernel_size_t size) { size_t p_size = __builtin_object_size(p, 0); @@ -393,7 +411,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 * BOS0 p, int c, size_t size) { size_t p_size = __builtin_object_size(p, 0); @@ -405,7 +423,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 * BOS0 p, size_t size, gfp_t gfp) { size_t p_size = __builtin_object_size(p, 0); @@ -417,7 +435,8 @@ __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 __diagnose_as(__builtin_strcpy, 1, 2) +char *strcpy(char * BOS p, const char * BOS q) { size_t p_size = __builtin_object_size(p, 1); size_t q_size = __builtin_object_size(q, 1); @@ -446,4 +465,7 @@ __FORTIFY_INLINE char *strcpy(char *p, const char *q) #undef __underlying_strncat #undef __underlying_strncpy +#undef BOS0 +#undef BOS + #endif /* _LINUX_FORTIFY_STRING_H_ */ diff --git a/security/Kconfig b/security/Kconfig index 0b847f435beb..1a25a567965f 100644 --- a/security/Kconfig +++ b/security/Kconfig @@ -179,7 +179,7 @@ config FORTIFY_SOURCE 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 >= 130000 help Detect overflows of buffers in common string and memory functions where the compiler can determine and validate the buffer sizes. -- 2.30.2
On Wed, Feb 2, 2022 at 1:30 AM Kees Cook <keescook@chromium.org> wrote: > > +/* > + * clang: https://clang.llvm.org/docs/AttributeReference.html#pass-object-size-pass-dynamic-object-size For attributes that are not supported under all compilers, we have the "Optional" lines in the comment. From a quick look in Godbolt, `__pass_object_size__` and `__overloadable__` are supported for all Clang >= 11 but not GCC/ICC. Thus, could you please add to the comment: * Optional: not supported by gcc. * Optional: not supported by icc. to those two patches? For `__diagnose_as_builtin__`, I only see it on Clang trunk, so I assume >= 14, thus could you please add: * Optional: only supported since clang >= 14. ? Thanks! > + * The "type" argument should match the __builtin_object_size(p, type) usage. This should go above on top of the comment (it is true there is one case that does not follow it, but that one has to be cleaned up). Also, this bit seems to be explained in the Clang documentation (i.e. not kernel-specific). Do you think we need it here? Cheers, Miguel
On Wed, Feb 2, 2022 at 2:11 AM Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote: > > For `__diagnose_as_builtin__`, I only see it on Clang trunk, so I > assume >= 14, thus could you please add: > > * Optional: only supported since clang >= 14. ...and the other two too: > * Optional: not supported by gcc. > * Optional: not supported by icc. Cheers, Miguel
On Wed, Feb 02, 2022 at 02:11:51AM +0100, Miguel Ojeda wrote: > On Wed, Feb 2, 2022 at 1:30 AM Kees Cook <keescook@chromium.org> wrote: > > > > +/* > > + * clang: https://clang.llvm.org/docs/AttributeReference.html#pass-object-size-pass-dynamic-object-size > > For attributes that are not supported under all compilers, we have the > "Optional" lines in the comment. From a quick look in Godbolt, > `__pass_object_size__` and `__overloadable__` are supported for all > Clang >= 11 but not GCC/ICC. Thus, could you please add to the > comment: > > * Optional: not supported by gcc. > * Optional: not supported by icc. > > to those two patches? > > For `__diagnose_as_builtin__`, I only see it on Clang trunk, so I > assume >= 14, thus could you please add: > > * Optional: only supported since clang >= 14. > > ? > > Thanks! Gotcha, I will adjust these. > > + * The "type" argument should match the __builtin_object_size(p, type) usage. > > This should go above on top of the comment (it is true there is one > case that does not follow it, but that one has to be cleaned up). > > Also, this bit seems to be explained in the Clang documentation (i.e. > not kernel-specific). Do you think we need it here? It's the kind of detail I feel like I might forget a month from now, so I added it as a bit of a hint. If you think it's too redundant, I can leave it off. -- Kees Cook
On Wed, Feb 2, 2022 at 10:09 PM Kees Cook <keescook@chromium.org> wrote:
>
> It's the kind of detail I feel like I might forget a month from now, so
> I added it as a bit of a hint. If you think it's too redundant, I can
> leave it off.
If you think it is valuable having it nearby (e.g. when inspecting the
definition of the attribute in a call site), then it is fine having
them -- I am all for documentation (and we could consider having a
small summary/one-liner for all attributes). But, in general, I would
avoid repeating all the compiler docs (or, if needed, we could fix
those upstream if they are not clear etc.).
Thanks!
Cheers,
Miguel
On Tue, Feb 1, 2022 at 4:30 PM Kees Cook <keescook@chromium.org> wrote: > > --- a/security/Kconfig > +++ b/security/Kconfig > @@ -179,7 +179,7 @@ config FORTIFY_SOURCE > 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 >= 130000 Are these comments still relevant, and is the clang version still correct? In https://lore.kernel.org/llvm/CANiq72n1d7ouKNi+pbsy7chsg0DfCXxez27qqtS9XE1n3m5=8Q@mail.gmail.com/ Miguel notes that diagnose_as only exists in clang-14+. If this series relies on diagnose_as, then should this version check be for clang-14+ rather than clang-13+? https://bugs.llvm.org/show_bug.cgi?id=50322 is still open, but doesn't signify why there's a version check. It makes sense if there's no version check, but I'm not sure it's still relevant to this Kconfig option after your series. https://bugs.llvm.org/show_bug.cgi?id=41459 was fixed in clang-13, but it was also backported to the clang 12.0.1 release. Is it still relevant if we're gated on diagnose_as from clang-14? Perhaps a single comment, about the diagnose_as attribute or a link to https://reviews.llvm.org/rGbc5f2d12cadce765620efc56a1ca815221db47af or whatever, and updating the version check to be against clang-14 would be more precise? -- Thanks, ~Nick Desaulniers
On Tue, Feb 1, 2022 at 4:30 PM Kees Cook <keescook@chromium.org> wrote:
>
> +#define BOS const __pass_object_size(1)
> +#define BOS0 const __pass_object_size(0)
A dumb bikeshed, but would you mind naming these BOS1 and BOS0, and
perhaps consider adding a comment or pointer or link to something that
describes why we use the two different modes? I recognize that the
code already uses the two different modes already without comments,
but this might be a nice place to point folks like myself to so that
in a month or so when I forget what the difference is between modes
(again), we have a shorter trail of breadcrumbs.
--
Thanks,
~Nick Desaulniers
On Wed, Feb 02, 2022 at 01:22:09PM -0800, Nick Desaulniers wrote: > On Tue, Feb 1, 2022 at 4:30 PM Kees Cook <keescook@chromium.org> wrote: > > > > --- a/security/Kconfig > > +++ b/security/Kconfig > > @@ -179,7 +179,7 @@ config FORTIFY_SOURCE > > 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 >= 130000 > > Are these comments still relevant, and is the clang version still correct? Oh, good call. I thought the version was still correct (more below), but yes, the comments need adjusting. > In https://lore.kernel.org/llvm/CANiq72n1d7ouKNi+pbsy7chsg0DfCXxez27qqtS9XE1n3m5=8Q@mail.gmail.com/ > Miguel notes that diagnose_as only exists in clang-14+. If this > series relies on diagnose_as, then should this version check be for > clang-14+ rather than clang-13+? It doesn't rely on it; this is just taking advantage of an improvement. > https://bugs.llvm.org/show_bug.cgi?id=50322 is still open, but doesn't > signify why there's a version check. It makes sense if there's no > version check, but I'm not sure it's still relevant to this Kconfig > option after your series. With __overloadable, this probably ended up going away. > https://bugs.llvm.org/show_bug.cgi?id=41459 was fixed in clang-13, but > it was also backported to the clang 12.0.1 release. Is it still > relevant if we're gated on diagnose_as from clang-14? Ah-ha! I missed that this got backported. Looks like 12.0.1 and later have this fixed. That's excellent! > Perhaps a single comment, about the diagnose_as attribute or a link to > https://reviews.llvm.org/rGbc5f2d12cadce765620efc56a1ca815221db47af or > whatever, and updating the version check to be against clang-14 would > be more precise? Yup, I will rework this after double-checking 12.0.1 builds. Thanks! -- Kees Cook
On Wed, Feb 02, 2022 at 01:27:11PM -0800, Nick Desaulniers wrote:
> On Tue, Feb 1, 2022 at 4:30 PM Kees Cook <keescook@chromium.org> wrote:
> >
> > +#define BOS const __pass_object_size(1)
> > +#define BOS0 const __pass_object_size(0)
>
> A dumb bikeshed, but would you mind naming these BOS1 and BOS0, and
> perhaps consider adding a comment or pointer or link to something that
> describes why we use the two different modes? I recognize that the
> code already uses the two different modes already without comments,
> but this might be a nice place to point folks like myself to so that
> in a month or so when I forget what the difference is between modes
> (again), we have a shorter trail of breadcrumbs.
Sure, I can do that. My expectation was to entirely eliminate mode 0
usage in the future.
Though now that things are so close, I'll just do some builds with the
last few users switched over. But maybe memcmp() was a pain? I'll go
check...
--
Kees Cook
On Tue, Feb 1, 2022 at 4:30 PM Kees Cook <keescook@chromium.org> wrote:
>
> diff --git a/include/linux/fortify-string.h b/include/linux/fortify-string.h
> index c45159dbdaa1..5482536d3197 100644
> --- a/include/linux/fortify-string.h
> +++ b/include/linux/fortify-string.h
> @@ -2,7 +2,9 @@
> #ifndef _LINUX_FORTIFY_STRING_H_
> #define _LINUX_FORTIFY_STRING_H_
>
> -#define __FORTIFY_INLINE extern __always_inline __attribute__((gnu_inline))
> +#include <linux/const.h>
> +
> +#define __FORTIFY_INLINE extern __always_inline __attribute__((gnu_inline)) __overloadable
Sorry, I just noticed this line (already) uses a mix of open coding
__attribute__ and not. Would you mind also please changing the open
coded gnu_inline to simply __gnu_inline to make the entire line
consistent?
--
Thanks,
~Nick Desaulniers
On Thu, Feb 3, 2022 at 11:13 PM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> Sorry, I just noticed this line (already) uses a mix of open coding
> __attribute__ and not. Would you mind also please changing the open
> coded gnu_inline to simply __gnu_inline to make the entire line
> consistent?
+1
Cheers,
Miguel
On Thu, Feb 03, 2022 at 02:13:33PM -0800, Nick Desaulniers wrote:
> On Tue, Feb 1, 2022 at 4:30 PM Kees Cook <keescook@chromium.org> wrote:
> >
> > diff --git a/include/linux/fortify-string.h b/include/linux/fortify-string.h
> > index c45159dbdaa1..5482536d3197 100644
> > --- a/include/linux/fortify-string.h
> > +++ b/include/linux/fortify-string.h
> > @@ -2,7 +2,9 @@
> > #ifndef _LINUX_FORTIFY_STRING_H_
> > #define _LINUX_FORTIFY_STRING_H_
> >
> > -#define __FORTIFY_INLINE extern __always_inline __attribute__((gnu_inline))
> > +#include <linux/const.h>
> > +
> > +#define __FORTIFY_INLINE extern __always_inline __attribute__((gnu_inline)) __overloadable
>
> Sorry, I just noticed this line (already) uses a mix of open coding
> __attribute__ and not. Would you mind also please changing the open
> coded gnu_inline to simply __gnu_inline to make the entire line
> consistent?
Ah yeah, thanks. Fixed.
--
Kees Cook