[v3,16/16] Add --enable-fortify-source option

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.
---
 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(-)

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'])

On Jun 30 2023, Siddhesh Poyarekar wrote:

> 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.

I prefer a configure option, mirroring --enable-stack-protector.  Since
glibc has very strict requirements wrt compiler flags it needs to handle
it specially anyway, and making it explicit is cleaner.

On 03/07/23 05:50, Andreas Schwab wrote:
> On Jun 30 2023, Siddhesh Poyarekar wrote:
> 
>> 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.
> 
> I prefer a configure option, mirroring --enable-stack-protector.  Since
> glibc has very strict requirements wrt compiler flags it needs to handle
> it specially anyway, and making it explicit is cleaner.
> 

Fair enough, I am aiming to simplify the configure options and thus the
build permutation that arise for multiple option; but I see that following
current practice should be ok.

On Mon, Jul 3, 2023 at 2:51 PM Adhemerval Zanella Netto <
adhemerval.zanella@linaro.org> wrote:

>
>
> On 03/07/23 05:50, Andreas Schwab wrote:
> > On Jun 30 2023, Siddhesh Poyarekar wrote:
> >
> >> 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.
> >
> > I prefer a configure option, mirroring --enable-stack-protector.  Since
> > glibc has very strict requirements wrt compiler flags it needs to handle
> > it specially anyway, and making it explicit is cleaner.
> >
>
> Fair enough, I am aiming to simplify the configure options and thus the
> build permutation that arise for multiple option; but I see that following
> current practice should be ok.
>
>
That would mean for me to do the following on this patch:
- if "--enable-fortify-source" is set, set -D_FORTIFY_SOURCE accordingly
(already done).
- if "--enable-fortify-source" is NOT set (i.e. assume
"--disable-fortify-source"), forcibly undefine _FORTIFY_SOURCE (currently
not done).

Do you all agree with that ?

Fred.

On 2023-07-04 08:40, Frederic Berat wrote:
> That would mean for me to do the following on this patch:
> - if "--enable-fortify-source" is set, set -D_FORTIFY_SOURCE accordingly 
> (already done).
> - if "--enable-fortify-source" is NOT set (i.e. assume 
> "--disable-fortify-source"), forcibly undefine _FORTIFY_SOURCE 
> (currently not done).

Correct.

Sid