Re: [PATCH v3 16/16] Add --enable-fortify-source option

[Siddhesh Poyarekar <siddhesh@gotplt.org>]

On 2023-06-28 04:42, Frédéric Bérat wrote:
> It is now possible to enable fortification through a configure option.
> The level may be given as parameter, if none is provided, the configure
> script will determine what is the highest level possible that can be set
> considering GCC built-ins availability and set it.
> If level is explicitly set to 3, configure checks if the compiler
> supports the built-in function necessary for it or raise an error if it
> isn't.
> 
> The result of the configure checks is a new variables, ${fortify_source}
> that can be used to appropriately populate CFLAGS.
> 
> Updated NEWS and INSTALL.
> 
> Adding dedicated x86_64 variant that enables the configuration.

Adhemerval, do you still think we should drop this and only look at 
CFLAGS?  I am still not a 100% convinced that we should only look at 
CFLAGS (it gives much less control which makes me uneasy) but I see your 
point.  We'll be setting CFLAGS in Fedora anyway (which I guess will be 
true for Ubuntu, Gentoo, Debian, etc. too) and the pre-commit CI will 
likely have _FORTIFY_SOURCE disabled so we may have adequate coverage.

Thanks,
Sid

> ---
>   INSTALL                      |  6 +++
>   Makeconfig                   |  9 ++++-
>   NEWS                         |  7 ++--
>   config.make.in               |  1 +
>   configure                    | 77 ++++++++++++++++++++++++++++++++++++
>   configure.ac                 | 42 ++++++++++++++++++--
>   manual/install.texi          |  6 +++
>   scripts/build-many-glibcs.py |  4 +-
>   8 files changed, 144 insertions(+), 8 deletions(-)
> 
> diff --git a/INSTALL b/INSTALL
> index 6d51475536..44daf64ebd 100644
> --- a/INSTALL
> +++ b/INSTALL
> @@ -276,6 +276,12 @@ if 'CFLAGS' is specified it must enable optimization.  For example:
>        the GNU C Library.  The default value refers to the main
>        bug-reporting information for the GNU C Library.
>   
> +'--enable-fortify-source'
> +'--enable-fortify-source=LEVEL'
> +     Use -D_FORTIFY_SOURCE='LEVEL' to control code hardening, if not
> +     provided, 'LEVEL' defaults to highest possible value for your
> +     system, based on the supported 'CC' features.
> +
>      To build the library and related programs, type 'make'.  This will
>   produce a lot of output, some of which may look like errors from 'make'
>   but aren't.  Look for error messages from 'make' containing '***'.
> diff --git a/Makeconfig b/Makeconfig
> index f6396b3e0c..84e5043b14 100644
> --- a/Makeconfig
> +++ b/Makeconfig
> @@ -902,6 +902,11 @@ define elide-stack-protector
>   $(if $(filter $(@F),$(patsubst %,%$(1),$(2))), $(no-stack-protector))
>   endef
>   
> +# We might want to compile with fortify-source
> +ifneq ($(fortify-source),)
> ++fortify-source=$(fortify-source)
> +endif
> +
>   # Some routine can't be fortified like the ones used by fortify
>   define elide-fortify-source
>   $(if $(filter $(@F),$(patsubst %,%$(1),$(2))), $(no-fortify-source))
> @@ -973,7 +978,9 @@ endif	# $(+cflags) == ""
>   # loader, cannot be fortified. Lastly debug is the fortification routines
>   # themselves and they cannot be fortified.
>   do-fortify = $(filter-out elf dlfcn csu debug,$(subdir))
> -ifneq ($(do-fortify),$(subdir))
> +ifeq ($(do-fortify),$(subdir))
> ++cflags += $(+fortify-source)
> +else
>   +cflags += $(no-fortify-source)
>   endif
>   
> diff --git a/NEWS b/NEWS
> index 027506a44c..b586f0bad5 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -48,6 +48,10 @@ Major new features:
>   * The strlcpy and strlcat functions have been added.  They are derived
>     from OpenBSD, and are expected to be added to a future POSIX version.
>   
> +* A new configure option, "--enable-fortify-source", can be used to build GLIBC
> +  with _FORTIFY_SOURCE. The level of fortification can either be provided, or
> +  is set to the highest value supported by the compiler.
> +
>   Deprecated and removed features, and other changes affecting compatibility:
>   
>   * In the Linux kernel for the hppa/parisc architecture some of the
> @@ -502,9 +506,6 @@ Major new features:
>   * The audit libraries will avoid unnecessary slowdown if it is not required
>     PLT tracking (by not implementing the la_pltenter or la_pltexit callbacks).
>   
> -* Glibc now supports to be built with _FORTIFY_SOURCE. The value is undefined
> -  for parts of the library that can't be built with it.
> -
>   Deprecated and removed features, and other changes affecting compatibility:
>   
>   * On x86-64, the LD_PREFER_MAP_32BIT_EXEC environment variable support
> diff --git a/config.make.in b/config.make.in
> index 75ad9765aa..d487a4f4e9 100644
> --- a/config.make.in
> +++ b/config.make.in
> @@ -64,6 +64,7 @@ have-fpie = @libc_cv_fpie@
>   have-ssp = @libc_cv_ssp@
>   stack-protector = @stack_protector@
>   no-stack-protector = @no_stack_protector@
> +fortify-source = @fortify_source@
>   no-fortify-source = @no_fortify_source@
>   have-selinux = @have_selinux@
>   have-libaudit = @have_libaudit@
> diff --git a/configure b/configure
> index 7a15f8d3e6..fa4a1c2346 100755
> --- a/configure
> +++ b/configure
> @@ -611,7 +611,10 @@ libc_cv_gcc_unwind_find_fde
>   libc_extra_cppflags
>   libc_extra_cflags
>   libc_cv_cxx_thread_local
> +fortify_source
>   no_fortify_source
> +libc_cv_fortify_source
> +enable_fortify_source
>   have_selinux
>   have_libcap
>   have_libaudit
> @@ -782,6 +785,7 @@ enable_pt_chown
>   enable_mathvec
>   enable_cet
>   enable_scv
> +enable_fortify_source
>   with_cpu
>   '
>         ac_precious_vars='build_alias
> @@ -1452,6 +1456,10 @@ Optional Features:
>                             (CET), x86 only
>     --disable-scv           syscalls will not use scv instruction, even if the
>                             kernel supports it, powerpc only
> +  --enable-fortify-source[=1|2|3]
> +                          Use -D_FORTIFY_SOURCE=[1|2|3] to control code
> +                          hardening, defaults to highest possible value for
> +                          your system
>   
>   Optional Packages:
>     --with-PACKAGE[=ARG]    use PACKAGE [ARG=yes]
> @@ -3717,6 +3725,18 @@ if test "$use_scv" != "no"; then :
>   
>   fi
>   
> +# Check whether --enable-fortify-source was given.
> +if test "${enable_fortify_source+set}" = set; then :
> +  enableval=$enable_fortify_source; enable_fortify_source=$enableval
> +else
> +  enable_fortify_source=no
> +fi
> +
> +case "$enable_fortify_source" in
> +1|2|3|no|yes) ;;
> +*) as_fn_error $? "Not a valid argument for --enable-fortify-source: \"$enable_fortify_source\"" "$LINENO" 5;;
> +esac
> +
>   # We keep the original values in `$config_*' and never modify them, so we
>   # can write them unchanged into config.make.  Everything else uses
>   # $machine, $vendor, and $os, and changes them whenever convenient.
> @@ -6353,8 +6373,65 @@ $as_echo "#define HAVE_LIBCAP 1" >>confdefs.h
>   fi
>   
>   
> +fortify_source=""
>   no_fortify_source="-Wp,-U_FORTIFY_SOURCE"
>   
> +{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for __builtin_dynamic_object_size" >&5
> +$as_echo_n "checking for __builtin_dynamic_object_size... " >&6; }
> +if ${libc_cv___builtin_dynamic_object_size+:} false; then :
> +  $as_echo_n "(cached) " >&6
> +else
> +
> +    cat confdefs.h - <<_ACEOF >conftest.$ac_ext
> +/* end confdefs.h.  */
> +
> +int
> +main ()
> +{
> +__builtin_dynamic_object_size("", 0)
> +  ;
> +  return 0;
> +}
> +_ACEOF
> +if ac_fn_c_try_link "$LINENO"; then :
> +  libc_cv___builtin_dynamic_object_size=yes
> +         if test "$enable_fortify_source" = yes; then :
> +  enable_fortify_source=3
> +fi
> +else
> +  libc_cv___builtin_dynamic_object_size=no
> +         if test "$enable_fortify_source" = yes; then :
> +  enable_fortify_source=2
> +fi
> +fi
> +rm -f core conftest.err conftest.$ac_objext \
> +    conftest$ac_exeext conftest.$ac_ext
> +
> +fi
> +{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $libc_cv___builtin_dynamic_object_size" >&5
> +$as_echo "$libc_cv___builtin_dynamic_object_size" >&6; }
> +
> +case $enable_fortify_source in #(
> +  1|2) :
> +    libc_cv_fortify_source=yes ;; #(
> +  3) :
> +    if test "$libc_cv___builtin_dynamic_object_size" = yes; then :
> +  libc_cv_fortify_source=yes
> +else
> +  as_fn_error $? "Compiler doesn't provide necessary support for _FORTIFY_SOURCE=3" "$LINENO" 5
> +fi ;; #(
> +  *) :
> +    libc_cv_fortify_source=no ;;
> +esac
> +
> +if test "$libc_cv_fortify_source" = yes; then :
> +  fortify_source="${no_fortify_source},-D_FORTIFY_SOURCE=${enable_fortify_source}"
> +
> +fi
> +
> +
> +
> +
>   
>   
>   { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether the assembler requires one version per symbol" >&5
> diff --git a/configure.ac b/configure.ac
> index ebc04d49e6..ec4de6e551 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -466,6 +466,17 @@ AC_ARG_ENABLE([scv],
>   
>   AS_IF([[test "$use_scv" != "no"]],[AC_DEFINE(USE_PPC_SCV)])
>   
> +dnl Build glibc with _FORTIFY_SOURCE
> +AC_ARG_ENABLE(fortify-source,
> +              AS_HELP_STRING([--enable-fortify-source@<:@=1|2|3@:>@],
> +                             [Use -D_FORTIFY_SOURCE=[1|2|3] to control code hardening, defaults to highest possible value for your system]),
> +              [enable_fortify_source=$enableval],
> +              [enable_fortify_source=no])
> +case "$enable_fortify_source" in
> +1|2|3|no|yes) ;;
> +*) AC_MSG_ERROR([Not a valid argument for --enable-fortify-source: "$enable_fortify_source"]);;
> +esac
> +
>   # We keep the original values in `$config_*' and never modify them, so we
>   # can write them unchanged into config.make.  Everything else uses
>   # $machine, $vendor, and $os, and changes them whenever convenient.
> @@ -1559,12 +1570,37 @@ if test "x$have_selinux" = xyes; then
>   fi
>   AC_SUBST(have_selinux)
>   
> -dnl Create a variable that can be used to control were _FORTIFY_SOURCE is set.
> -dnl This will allow users to enable fortification through FLAGS or compiler
> -dnl defaults macro definitions.
> +dnl Check if we support the requested _FORTIFY_SOURCE level
> +dnl If not, then don't use it.
> +dnl Note that _FORTIFY_SOURCE may have been set through FLAGS too.
> +dnl _FORTIFY_SOURCE value will be selectively disabled for function that can't
> +dnl support it
> +fortify_source=""
>   no_fortify_source="-Wp,-U_FORTIFY_SOURCE"
>   
> +AC_CACHE_CHECK([for __builtin_dynamic_object_size], [libc_cv___builtin_dynamic_object_size], [
> +    AC_LINK_IFELSE([AC_LANG_PROGRAM([], [__builtin_dynamic_object_size("", 0)])],
> +        [libc_cv___builtin_dynamic_object_size=yes
> +         AS_IF([test "$enable_fortify_source" = yes], [enable_fortify_source=3])],
> +        [libc_cv___builtin_dynamic_object_size=no
> +         AS_IF([test "$enable_fortify_source" = yes], [enable_fortify_source=2])])
> +])
> +
> +AS_CASE([$enable_fortify_source],
> +        [1|2], [libc_cv_fortify_source=yes],
> +        [3], [AS_IF([test "$libc_cv___builtin_dynamic_object_size" = yes],
> +                    [libc_cv_fortify_source=yes],
> +                    [AC_MSG_ERROR([Compiler doesn't provide necessary support for _FORTIFY_SOURCE=3])])],
> +        [libc_cv_fortify_source=no])
> +
> +AS_IF([test "$libc_cv_fortify_source" = yes],
> +      [fortify_source="${no_fortify_source},-D_FORTIFY_SOURCE=${enable_fortify_source}"]
> +      )
> +
> +AC_SUBST(enable_fortify_source)
> +AC_SUBST(libc_cv_fortify_source)
>   AC_SUBST(no_fortify_source)
> +AC_SUBST(fortify_source)
>   
>   dnl Starting with binutils 2.35, GAS can attach multiple symbol versions
>   dnl to one symbol (PR 23840).
> diff --git a/manual/install.texi b/manual/install.texi
> index a44a552d1f..26b64062a0 100644
> --- a/manual/install.texi
> +++ b/manual/install.texi
> @@ -303,6 +303,12 @@ Specify the URL that users should visit if they wish to report a bug,
>   to be included in @option{--help} output from programs installed with
>   @theglibc{}.  The default value refers to the main bug-reporting
>   information for @theglibc{}.
> +
> +@item --enable-fortify-source
> +@itemx --enable-fortify-source=@var{LEVEL}
> +Use -D_FORTIFY_SOURCE=@option{LEVEL} to control code hardening, if not
> +provided, @option{LEVEL} defaults to highest possible value for your system,
> +based on the supported @code{CC} features.
>   @end table
>   
>   To build the library and related programs, type @code{make}.  This will
> diff --git a/scripts/build-many-glibcs.py b/scripts/build-many-glibcs.py
> index e022abe284..e4eaec01e3 100755
> --- a/scripts/build-many-glibcs.py
> +++ b/scripts/build-many-glibcs.py
> @@ -464,7 +464,9 @@ class Context(object):
>                                         {'arch': 'i486',
>                                          'ccopts': '-m32 -march=i486'},
>                                         {'arch': 'i586',
> -                                       'ccopts': '-m32 -march=i586'}])
> +                                       'ccopts': '-m32 -march=i586'},
> +                                      {'variant': 'enable-fortify-source',
> +                                       'cfg': ['--enable-fortify-source']}])
>           self.add_config(arch='x86_64',
>                           os_name='gnu',
>                           gcc_cfg=['--disable-multilib'])