[PATCH] fortify: further improve Clang style fortify
Adhemerval Zanella Netto
adhemerval.zanella@linaro.org
Thu Mar 27 16:32:30 GMT 2025
On 27/03/25 11:35, Adrian Ratiu wrote:
> glibc >= v2.41 significatly improved clang fortify support,
> so I've rebased the ChromeOS toolchain test suite [1] on
> top of the upstream implementation and semantics.
>
> Doing this I noticed some missing upstream diagnostics, so
> I've added them to get Clang builds on par with GCC.
>
> In addition to the missing ones I also noticed some mismatch
> bugs between __glibc_objsize0 vs __glibc_objsize calls done
> via various macros like:
> __fortify_clang_warning_only_if_bos0_lt2 versus
> __fortify_clang_warning_only_if_bos_lt2
>
> The general rule is that the function argument declaration
> must match its uses to ensure check consistency between the
> __glibc_objsize0 and __glibc_objsize calls.
>
> For example if the arg is defined like
> __fortify_clang_overload_arg (...)
> then a form like the following must be used:
> __fortify_clang_warning_only_if_bos_lt2(...)
>
> If the arg is defined like
> __fortify_clang_overload_arg0 (...)
> then a form like the following must be used:
> __fortify_clang_warning_only_if_bos0_lt2(...)
>
> This rule ensures consistent use of __glibc_objsize0 vs
> __glibc_objsize calls in checks.
>
> While a diagnostic message might be correctly printed
> with the above type of mismatch, LLVM/Clang will not be
> able to signal and terminate a process despite for eg a
> buffer overflow being detected (it will happily continue).
These should be considered real bugs and have proper regression testcases.
Do you have any examples of which fortify wrappers are not being triggered
correctly?
Unfortunately we don't have tests for the compiler warning itself,
just for the runtime behavior. And I recently updated my clang branch [1]
and I don't see any regression with debug fortify tests with clang 18/19/20/main.
I also don't see any issue withg clang 18/19/20/main when used just as test
compiler (TEST_CC/TEST_CXX).
[1] https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/azanella/clang
>
> So the end result of this type of bug fix is that processes
> will get terimated by proper signals when fortify diagnostics
> get triggered.
>
> Based on a patch by George Burges with contributions from
> Andrej Shadura. [2]
>
> Link: https://chromium.googlesource.com/chromiumos/platform2/+/refs/heads/main/cros-toolchain/fortify-tests/clang-fortify-tests.cpp [1]
> Link: https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/refs/heads/staging-infra-release-R135-16209.B/sys-libs/glibc/files/local/glibc-2.39/0005-glibc-add-clang-style-FORTIFY.patch [2]
> Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
> ---
> libio/bits/stdio2.h | 38 ++++++++++++++++++++++-----------
> misc/bits/syslog.h | 6 ++++--
> posix/bits/unistd.h | 5 ++++-
> stdlib/bits/stdlib.h | 5 ++++-
> string/bits/string_fortified.h | 26 +++++++++++++---------
> string/bits/strings_fortified.h | 11 ++++++----
> wcsmbs/bits/wchar2.h | 12 +++++------
> 7 files changed, 67 insertions(+), 36 deletions(-)
>
> diff --git a/libio/bits/stdio2.h b/libio/bits/stdio2.h
> index d2ed65371f..0dca01ec83 100644
> --- a/libio/bits/stdio2.h
> +++ b/libio/bits/stdio2.h
> @@ -33,7 +33,8 @@ __NTH (sprintf (char *__restrict __s, const char *__restrict __fmt, ...))
> }
> #elif __fortify_use_clang
> /* clang does not have __va_arg_pack, so defer to va_arg version. */
> -__fortify_function_error_function __attribute_overloadable__ int
> +__fortify_function_error_function __attribute_overloadable__
> +__attribute__ ((__format__ (__printf__, 2, 3))) int
This seems not really related to the described issues in commit message and
not really clang-specific since gcc also supports __format__ attribute.
So I think we should be a different change.
> __NTH (sprintf (__fortify_clang_overload_arg (char *, __restrict, __s),
> const char *__restrict __fmt, ...))
> {
> @@ -71,9 +72,13 @@ __NTH (snprintf (char *__restrict __s, size_t __n,
> }
> # elif __fortify_use_clang
> /* clang does not have __va_arg_pack, so defer to va_arg version. */
> -__fortify_function_error_function __attribute_overloadable__ int
> +__fortify_function_error_function __attribute_overloadable__
> +__attribute__ ((__format__ (__printf__, 3, 4))) int
> __NTH (snprintf (__fortify_clang_overload_arg (char *, __restrict, __s),
> size_t __n, const char *__restrict __fmt, ...))
> + __fortify_clang_warning_only_if_bos_lt(__n, __s,
> + "call to snprintf may overflow "
> + "the destination buffer")
> {
> __gnuc_va_list __fortify_ap;
> __builtin_va_start (__fortify_ap, __fmt);
> @@ -89,13 +94,14 @@ __NTH (snprintf (__fortify_clang_overload_arg (char *, __restrict, __s),
> __glibc_objsize (str), __VA_ARGS__)
> # endif
>
> -__fortify_function __attribute_overloadable__ int
> +__fortify_function __attribute_overloadable__
> +__attribute__ ((__format__ (__printf__, 3, 0))) int
> __NTH (vsnprintf (__fortify_clang_overload_arg (char *, __restrict, __s),
> size_t __n, const char *__restrict __fmt,
> __gnuc_va_list __ap))
> - __fortify_clang_warning (__fortify_clang_bos_static_lt (__n, __s),
> - "call to vsnprintf may overflow the destination "
> - "buffer")
> + __fortify_clang_warning_only_if_bos_lt(__n, __s,
> + "call to vsnprintf may overflow "
> + "the destination buffer")
> {
> return __builtin___vsnprintf_chk (__s, __n, __USE_FORTIFY_LEVEL - 1,
> __glibc_objsize (__s), __fmt, __ap);
> @@ -119,7 +125,9 @@ printf (const char *__restrict __fmt, ...)
> }
> # elif __fortify_use_clang
> /* clang does not have __va_arg_pack, so defer to va_arg version. */
> -__fortify_function_error_function __attribute_overloadable__ __nonnull ((1)) int
> +__fortify_function_error_function __attribute_overloadable__ __nonnull ((1))
> +__attribute__ ((__format__ (__printf__, 2, 3)))
> +int
> fprintf (__fortify_clang_overload_arg (FILE *, __restrict, __stream),
> const char *__restrict __fmt, ...)
> {
> @@ -131,7 +139,8 @@ fprintf (__fortify_clang_overload_arg (FILE *, __restrict, __stream),
> return __r;
> }
>
> -__fortify_function_error_function __attribute_overloadable__ int
> +__fortify_function_error_function __attribute_overloadable__
> +__attribute__ ((__format__ (__printf__, 1, 2))) int
> printf (__fortify_clang_overload_arg (const char *, __restrict, __fmt), ...)
> {
> __gnuc_va_list __fortify_ap;
> @@ -148,7 +157,8 @@ printf (__fortify_clang_overload_arg (const char *, __restrict, __fmt), ...)
> __fprintf_chk (stream, __USE_FORTIFY_LEVEL - 1, __VA_ARGS__)
> # endif
>
> -__fortify_function __attribute_overloadable__ int
> +__fortify_function __attribute_overloadable__
> +__attribute__ ((__format__ (__printf__, 1, 0))) int
> vprintf (__fortify_clang_overload_arg (const char *, __restrict, __fmt),
> __gnuc_va_list __ap)
> {
> @@ -175,7 +185,8 @@ dprintf (int __fd, const char *__restrict __fmt, ...)
> __va_arg_pack ());
> }
> # elif __fortify_use_clang
> -__fortify_function_error_function __attribute_overloadable__ int
> +__fortify_function_error_function __attribute_overloadable__
> +__attribute__ ((__format__ (__printf__, 2, 3))) int
> dprintf (int __fd, __fortify_clang_overload_arg (const char *, __restrict,
> __fmt), ...)
> {
> @@ -223,7 +234,9 @@ __NTH (obstack_printf (struct obstack *__restrict __obstack,
> __va_arg_pack ());
> }
> # elif __fortify_use_clang
> -__fortify_function_error_function __attribute_overloadable__ int
> +__fortify_function_error_function __attribute_overloadable__
> +__attribute__ ((__format__ (__printf__, 2, 3)))
> +__wur int
> __NTH (asprintf (__fortify_clang_overload_arg (char **, __restrict, __ptr),
> const char *__restrict __fmt, ...))
> {
> @@ -247,7 +260,8 @@ __NTH (__asprintf (__fortify_clang_overload_arg (char **, __restrict, __ptr),
> return __r;
> }
>
> -__fortify_function_error_function __attribute_overloadable__ int
> +__fortify_function_error_function __attribute_overloadable__
> +__attribute__ ((__format__ (__printf__, 2, 3))) int
> __NTH (obstack_printf (__fortify_clang_overload_arg (struct obstack *,
> __restrict, __obstack),
> const char *__restrict __fmt, ...))
> diff --git a/misc/bits/syslog.h b/misc/bits/syslog.h
> index 9907a28cbe..ca3132e09c 100644
> --- a/misc/bits/syslog.h
> +++ b/misc/bits/syslog.h
> @@ -37,7 +37,8 @@ syslog (int __pri, const char *__fmt, ...)
> __syslog_chk (__pri, __USE_FORTIFY_LEVEL - 1, __fmt, __va_arg_pack ());
> }
> #elif __fortify_use_clang && defined __USE_MISC
> -__fortify_function_error_function __attribute_overloadable__ void
> +__fortify_function_error_function __attribute_overloadable__
> +__attribute__ ((__format__ (__printf__, 2, 3))) void
> syslog (int __pri, __fortify_clang_overload_arg (const char *, , __fmt), ...)
> {
> __gnuc_va_list __fortify_ap;
> @@ -52,7 +53,8 @@ syslog (int __pri, __fortify_clang_overload_arg (const char *, , __fmt), ...)
>
>
> #ifdef __USE_MISC
> -__fortify_function __attribute_overloadable__ void
> +__fortify_function __attribute_overloadable__
> +__attribute__ ((__format__ (__printf__, 2, 0))) void
> vsyslog (int __pri, __fortify_clang_overload_arg (const char *, ,__fmt),
> __gnuc_va_list __ap)
> {
> diff --git a/posix/bits/unistd.h b/posix/bits/unistd.h
> index f3428135ec..0af7b5e53c 100644
> --- a/posix/bits/unistd.h
> +++ b/posix/bits/unistd.h
> @@ -79,7 +79,7 @@ pread64 (int __fd, __fortify_clang_overload_arg0 (void *, ,__buf),
> #if defined __USE_XOPEN_EXTENDED || defined __USE_XOPEN2K
> __fortify_function __attribute_overloadable__ __nonnull ((1, 2)) __wur ssize_t
> __NTH (readlink (const char *__restrict __path,
> - __fortify_clang_overload_arg0 (char *, __restrict, __buf),
> + __fortify_clang_overload_arg (char *, __restrict, __buf),
> size_t __len))
> __fortify_clang_warning_only_if_bos_lt (__len, __buf,
> "readlink called with bigger length "
> @@ -122,6 +122,9 @@ __NTH (getcwd (__fortify_clang_overload_arg (char *, , __buf), size_t __size))
> __fortify_function __attribute_overloadable__ __nonnull ((1))
> __attribute_deprecated__ __wur char *
> __NTH (getwd (__fortify_clang_overload_arg (char *,, __buf)))
> + __fortify_clang_warning (__glibc_objsize (__buf) == (size_t) -1,
> + "please use getcwd instead, as getwd "
> + "doesn't specify buffer size")
> {
> if (__glibc_objsize (__buf) != (size_t) -1)
> return __getwd_chk (__buf, __glibc_objsize (__buf));
Ok.
> diff --git a/stdlib/bits/stdlib.h b/stdlib/bits/stdlib.h
> index 12f9cfd58f..58c51faaf8 100644
> --- a/stdlib/bits/stdlib.h
> +++ b/stdlib/bits/stdlib.h
> @@ -129,7 +129,7 @@ __fortify_function __attribute_overloadable__ size_t
> __NTH (mbstowcs (__fortify_clang_overload_arg (wchar_t *, __restrict, __dst),
> const char *__restrict __src,
> size_t __len))
> - __fortify_clang_warning_only_if_bos0_lt2 (__len, __dst, sizeof (wchar_t),
> + __fortify_clang_warning_only_if_bos_lt2 (__len, __dst, sizeof (wchar_t),
> "mbstowcs called with dst buffer "
> "smaller than len * sizeof (wchar_t)")
> {
Ok.
> @@ -159,6 +159,9 @@ __fortify_function __attribute_overloadable__ size_t
> __NTH (wcstombs (__fortify_clang_overload_arg (char *, __restrict, __dst),
> const wchar_t *__restrict __src,
> size_t __len))
> + __fortify_clang_warning_only_if_bos_lt (__len, __dst,
> + "wcstombs called with dst buffer "
> + "smaller than len")
> {
> return __glibc_fortify (wcstombs, __len, sizeof (char),
> __glibc_objsize (__dst),
Ok.
> diff --git a/string/bits/string_fortified.h b/string/bits/string_fortified.h
> index ae14cfbab0..2512bc0517 100644
> --- a/string/bits/string_fortified.h
> +++ b/string/bits/string_fortified.h
> @@ -22,25 +22,29 @@
> # error "Never use <bits/string_fortified.h> directly; include <string.h> instead."
> #endif
>
> -__fortify_function void *
> -__NTH (memcpy (void *__restrict __dest, const void *__restrict __src,
> - size_t __len))
> +__fortify_function __attribute_overloadable__ void *
> +__NTH (memcpy (__fortify_clang_overload_arg0 (void *, __restrict, __dest),
> + const void *__restrict __src, size_t __len))
> + __fortify_clang_warn_if_dest_too_small0 (__dest, __len)
> {
> return __builtin___memcpy_chk (__dest, __src, __len,
> __glibc_objsize0 (__dest));
> }
>
Ok.
> -__fortify_function void *
> -__NTH (memmove (void *__dest, const void *__src, size_t __len))
> +__fortify_function __attribute_overloadable__ void *
> +__NTH (memmove (__fortify_clang_overload_arg0 (void *, __restrict, __dest),
> + const void *__src, size_t __len))
> + __fortify_clang_warn_if_dest_too_small0 (__dest, __len)
> {
> return __builtin___memmove_chk (__dest, __src, __len,
> __glibc_objsize0 (__dest));
> }
>
Ok.
> #ifdef __USE_GNU
> -__fortify_function void *
> -__NTH (mempcpy (void *__restrict __dest, const void *__restrict __src,
> - size_t __len))
> +__fortify_function __attribute_overloadable__ void *
> +__NTH (mempcpy (__fortify_clang_overload_arg0 (void *, __restrict, __dest),
> + const void *__restrict __src, size_t __len))
> + __fortify_clang_warn_if_dest_too_small0 (__dest, __len)
> {
> return __builtin___mempcpy_chk (__dest, __src, __len,
> __glibc_objsize0 (__dest));
Ok.
> @@ -53,8 +57,10 @@ __NTH (mempcpy (void *__restrict __dest, const void *__restrict __src,
> especially problematic if the intended fill value is zero. In this
> case no work is done at all. We detect these problems by referring
> non-existing functions. */
> -__fortify_function void *
> -__NTH (memset (void *__dest, int __ch, size_t __len))
> +__fortify_function __attribute_overloadable__ void *
> +__NTH (memset (__fortify_clang_overload_arg0 (void *, __restrict, __dest),
> + int __ch, size_t __len))
> + __fortify_clang_warn_if_dest_too_small0 (__dest, __len)
> {
> return __builtin___memset_chk (__dest, __ch, __len,
> __glibc_objsize0 (__dest));
Ok.
> diff --git a/string/bits/strings_fortified.h b/string/bits/strings_fortified.h
> index be65d5937d..327288f289 100644
> --- a/string/bits/strings_fortified.h
> +++ b/string/bits/strings_fortified.h
> @@ -19,15 +19,18 @@
> #ifndef __STRINGS_FORTIFIED
> # define __STRINGS_FORTIFIED 1
>
> -__fortify_function void
> -__NTH (bcopy (const void *__src, void *__dest, size_t __len))
> +__fortify_function __attribute_overloadable__ void
> +__NTH (bcopy (const void *__src, __fortify_clang_overload_arg0 (void *, , __dest),
> + size_t __len))
> + __fortify_clang_warn_if_dest_too_small0 (__dest, __len)
> {
> (void) __builtin___memmove_chk (__dest, __src, __len,
> __glibc_objsize0 (__dest));
> }
>
Ok.
> -__fortify_function void
> -__NTH (bzero (void *__dest, size_t __len))
> +__fortify_function __attribute_overloadable__ void
> +__NTH (bzero (__fortify_clang_overload_arg0 (void *, , __dest), size_t __len))
> + __fortify_clang_warn_if_dest_too_small0 (__dest, __len)
> {
> (void) __builtin___memset_chk (__dest, '\0', __len,
> __glibc_objsize0 (__dest));
Ok.
> diff --git a/wcsmbs/bits/wchar2.h b/wcsmbs/bits/wchar2.h
> index c1a1da8166..609cc76986 100644
> --- a/wcsmbs/bits/wchar2.h
> +++ b/wcsmbs/bits/wchar2.h
> @@ -21,7 +21,7 @@
> #endif
>
> __fortify_function __attribute_overloadable__ wchar_t *
> -__NTH (wmemcpy (__fortify_clang_overload_arg (wchar_t *, __restrict, __s1),
> +__NTH (wmemcpy (__fortify_clang_overload_arg0 (wchar_t *, __restrict, __s1),
> const wchar_t *__restrict __s2, size_t __n))
> __fortify_clang_warning_only_if_bos0_lt2 (__n, __s1, sizeof (wchar_t),
> "wmemcpy called with length bigger "
> @@ -33,7 +33,7 @@ __NTH (wmemcpy (__fortify_clang_overload_arg (wchar_t *, __restrict, __s1),
> }
Ok.
>
> __fortify_function __attribute_overloadable__ wchar_t *
> -__NTH (wmemmove (__fortify_clang_overload_arg (wchar_t *, ,__s1),
> +__NTH (wmemmove (__fortify_clang_overload_arg0 (wchar_t *, ,__s1),
> const wchar_t *__s2, size_t __n))
> __fortify_clang_warning_only_if_bos0_lt2 (__n, __s1, sizeof (wchar_t),
> "wmemmove called with length bigger "
> @@ -46,7 +46,7 @@ __NTH (wmemmove (__fortify_clang_overload_arg (wchar_t *, ,__s1),
>
> #ifdef __USE_GNU
> __fortify_function __attribute_overloadable__ wchar_t *
> -__NTH (wmempcpy (__fortify_clang_overload_arg (wchar_t *, __restrict, __s1),
> +__NTH (wmempcpy (__fortify_clang_overload_arg0 (wchar_t *, __restrict, __s1),
> const wchar_t *__restrict __s2, size_t __n))
> __fortify_clang_warning_only_if_bos0_lt2 (__n, __s1, sizeof (wchar_t),
> "wmempcpy called with length bigger "
> @@ -59,7 +59,7 @@ __NTH (wmempcpy (__fortify_clang_overload_arg (wchar_t *, __restrict, __s1),
> #endif
>
> __fortify_function __attribute_overloadable__ wchar_t *
> -__NTH (wmemset (__fortify_clang_overload_arg (wchar_t *, ,__s), wchar_t __c,
> +__NTH (wmemset (__fortify_clang_overload_arg0 (wchar_t *, ,__s), wchar_t __c,
> size_t __n))
> __fortify_clang_warning_only_if_bos0_lt2 (__n, __s, sizeof (wchar_t),
> "wmemset called with length bigger "
> @@ -93,7 +93,7 @@ __NTH (wcpcpy (__fortify_clang_overload_arg (wchar_t *, __restrict, __dest),
> __fortify_function __attribute_overloadable__ wchar_t *
> __NTH (wcsncpy (__fortify_clang_overload_arg (wchar_t *, __restrict, __dest),
> const wchar_t *__restrict __src, size_t __n))
> - __fortify_clang_warning_only_if_bos0_lt2 (__n, __dest, sizeof (wchar_t),
> + __fortify_clang_warning_only_if_bos_lt2 (__n, __dest, sizeof (wchar_t),
> "wcsncpy called with length bigger "
> "than size of destination buffer")
> {
> @@ -105,7 +105,7 @@ __NTH (wcsncpy (__fortify_clang_overload_arg (wchar_t *, __restrict, __dest),
> __fortify_function __attribute_overloadable__ wchar_t *
> __NTH (wcpncpy (__fortify_clang_overload_arg (wchar_t *, __restrict, __dest),
> const wchar_t *__restrict __src, size_t __n))
> - __fortify_clang_warning_only_if_bos0_lt2 (__n, __dest, sizeof (wchar_t),
> + __fortify_clang_warning_only_if_bos_lt2 (__n, __dest, sizeof (wchar_t),
> "wcpncpy called with length bigger "
> "than size of destination buffer")
> {
Ok.
More information about the Libc-alpha
mailing list