Add _FORTIFY_SOURCE implementation documentation [BZ #28998]

  There have been multiple requests to provide more detail on how the
_FORTIFY_SOURCE macro works, so this patch adds a new node in the
Library Maintenance section that does this.  A lot of the description is
implementation detail, which is why I put this in the appendix and not
in the main documentation.

Resolves: BZ #28998.
Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
---
 manual/creature.texi |   4 +-
 manual/maint.texi    | 191 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 194 insertions(+), 1 deletion(-)
  

On Dez 15 2022, Siddhesh Poyarekar via Libc-alpha wrote:

> diff --git a/manual/creature.texi b/manual/creature.texi
> index 530a02398e..c4f9d99469 100644
> --- a/manual/creature.texi
> +++ b/manual/creature.texi
> @@ -305,7 +305,9 @@ included.
>  If this macro is defined to @math{1}, security hardening is added to
>  various library functions.  If defined to @math{2}, even stricter
>  checks are applied. If defined to @math{3}, @theglibc{} may also use
> -checks that may have an additional performance overhead.
> +checks that may have an additional performance overhead.  See
> +@xref{Source Fortification,,Fortification of function calls} for more

No See before @xref.
  

On 2022-12-15 11:35, Andreas Schwab wrote:
> On Dez 15 2022, Siddhesh Poyarekar via Libc-alpha wrote:
> 
>> diff --git a/manual/creature.texi b/manual/creature.texi
>> index 530a02398e..c4f9d99469 100644
>> --- a/manual/creature.texi
>> +++ b/manual/creature.texi
>> @@ -305,7 +305,9 @@ included.
>>   If this macro is defined to @math{1}, security hardening is added to
>>   various library functions.  If defined to @math{2}, even stricter
>>   checks are applied. If defined to @math{3}, @theglibc{} may also use
>> -checks that may have an additional performance overhead.
>> +checks that may have an additional performance overhead.  See
>> +@xref{Source Fortification,,Fortification of function calls} for more
> 
> No See before @xref.
> 

Thanks, fixed.

Sid
  

Ping!  Any further comments other than the redundant 'See' Andreas noted?

Thanks,
Sid

On 2022-12-15 11:25, Siddhesh Poyarekar via Libc-alpha wrote:
> There have been multiple requests to provide more detail on how the
> _FORTIFY_SOURCE macro works, so this patch adds a new node in the
> Library Maintenance section that does this.  A lot of the description is
> implementation detail, which is why I put this in the appendix and not
> in the main documentation.
> 
> Resolves: BZ #28998.
> Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
> ---
>   manual/creature.texi |   4 +-
>   manual/maint.texi    | 191 +++++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 194 insertions(+), 1 deletion(-)
> 
> diff --git a/manual/creature.texi b/manual/creature.texi
> index 530a02398e..c4f9d99469 100644
> --- a/manual/creature.texi
> +++ b/manual/creature.texi
> @@ -305,7 +305,9 @@ included.
>   If this macro is defined to @math{1}, security hardening is added to
>   various library functions.  If defined to @math{2}, even stricter
>   checks are applied. If defined to @math{3}, @theglibc{} may also use
> -checks that may have an additional performance overhead.
> +checks that may have an additional performance overhead.  See
> +@xref{Source Fortification,,Fortification of function calls} for more
> +information.
>   @end defvr
>   
>   @defvr Macro _DYNAMIC_STACK_SIZE_SOURCE
> diff --git a/manual/maint.texi b/manual/maint.texi
> index 49510db7bf..b99a951d3d 100644
> --- a/manual/maint.texi
> +++ b/manual/maint.texi
> @@ -5,6 +5,7 @@
>   @menu
>   * Source Layout::         How to add new functions or header files
>                                to the GNU C Library.
> +* Source Fortification::  Fortification of function calls.
>   * Symbol handling::       How to handle symbols in the GNU C Library.
>   * Porting::               How to port the GNU C Library to
>                                a new machine or operating system.
> @@ -184,6 +185,196 @@ header file in the machine-specific directory, e.g.,
>   @file{sysdeps/powerpc/sys/platform/ppc.h}.
>   
>   
> +@node Source Fortification
> +@appendixsec Fortification of function calls
> +
> +This section contains implementation details of @theglibc{} and may not
> +remain stable across releases.
> +
> +The @code{_FORTIFY_SOURCE} macro may be defined by users to control
> +hardening of calls into some functions in @theglibc{}.  This feature
> +needs a compiler that supports either the @code{__builtin_object_size}
> +or the @code{__builtin_dynamic_object_size} builtin functions.  When the
> +macro is defined, it enables code that validates access to buffers that
> +are passed to some functions in @theglibc to determine if they
> +are safe.  If the compiler is able to deduce the size of the buffer
> +passed to the function call but the call cannot be determined as safe,
> +it is replaced by a call to its hardened variant that does the access
> +validation at runtime.  At runtime, if the access validation check for
> +the buffer fails, the program will terminate with a @code{SIGABRT}
> +signal.
> +
> +@code{_FORTIFY_SOURCE} may be defined to one of the following values:
> +
> +@itemize @bullet
> +@item @math{1}: This enables buffer bounds checking using the value
> +returned by the @code{__builtin_object_size} compiler builtin function.
> +If the function returns @code{(size_t) -1}, the function call is left
> +untouched.
> +
> +@item @math{2}: This behaves like @math{1}, with the addition of some
> +checks that may trap code that is conforming but unsafe, e.g. accepting
> +@code{%n} only in read-only format strings.
> +
> +@item @math{3}: This enables buffer bounds checking using the value
> +returned by the @code{__builtin_dynamic_object_size} compiler builtin
> +function.  If the function returns @code{(size_t) -1}, the function call
> +is left untouched.  Fortification at this level may have a impact on
> +program performance if the function call that is fortified is frequently
> +encountered and the size expression returned by
> +@code{__builtin_dynamic_object_size} is complex.
> +@end itemize
> +
> +The following functions are fortified in @theglibc{}:
> +
> +@itemize @bullet
> +@item @code{asprintf}: Replaced with @code{__asprintf_chk}.
> +
> +@item @code{confstr}: Replaced with @code{__confstr_chk}.
> +
> +@item @code{dprintf}: Replaced with @code{__dprintf_chk}.
> +
> +@item @code{explicit_bzero}: Replaced with @code{__explicit_bzero_chk}.
> +
> +@item @code{fdelt}: Replaced with @code{__fdelt_chk}.
> +
> +@item @code{fgets}: Replaced with @code{__fgets_chk}.
> +
> +@item @code{fgetws}: Replaced with @code{__fgetws_chk}.
> +
> +@item @code{fprintf}: Replaced with @code{__fprintf_chk}.
> +
> +@item @code{fread}: Replaced with @code{__fread_chk}.
> +
> +@item @code{fwprintf}: Replaced with @code{__fwprintf_chk}.
> +
> +@item @code{getcwd}: Replaced with @code{__getcwd_chk}.
> +
> +@item @code{getdomainname}: Replaced with @code{__getdomainname_chk}.
> +
> +@item @code{getgroups}: Replaced with @code{__getgroups_chk}.
> +
> +@item @code{gethostname}: Replaced with @code{__gethostname_chk}.
> +
> +@item @code{gets}: Replaced with @code{__gets_chk}.
> +
> +@item @code{getwd}: Replaced with @code{__getwd_chk}.
> +
> +@item @code{longjmp}: Replaced with @code{__longjmp_chk}.
> +
> +@item @code{mbsnrtowcs}: Replaced with @code{__mbsnrtowcs_chk}.
> +
> +@item @code{mbsrtowcs}: Replaced with @code{__mbsrtowcs_chk}.
> +
> +@item @code{mbstowcs}: Replaced with @code{__mbstowcs_chk}.
> +
> +@item @code{memcpy}: Replaced with @code{__memcpy_chk}.
> +
> +@item @code{memmove}: Replaced with @code{__memmove_chk}.
> +
> +@item @code{mempcpy}: Replaced with @code{__mempcpy_chk}.
> +
> +@item @code{memset}: Replaced with @code{__memset_chk}.
> +
> +@item @code{obprintf}: Replaced with @code{__obprintf_chk}.
> +
> +@item @code{poll}: Replaced with @code{__poll_chk}.
> +
> +@item @code{ppoll}: Replaced with @code{__ppoll_chk}.
> +
> +@item @code{pread64}: Replaced with @code{__pread64_chk}.
> +
> +@item @code{pread}: Replaced with @code{__pread_chk}.
> +
> +@item @code{printf}: Replaced with @code{__printf_chk}.
> +
> +@item @code{read}: Replaced with @code{__read_chk}.
> +
> +@item @code{readlinkat}: Replaced with @code{__readlinkat_chk}.
> +
> +@item @code{readlink}: Replaced with @code{__readlink_chk}.
> +
> +@item @code{realpath}: Replaced with @code{__realpath_chk}.
> +
> +@item @code{recv}: Replaced with @code{__recv_chk}.
> +
> +@item @code{recvfrom}: Replaced with @code{__recvfrom_chk}.
> +
> +@item @code{snprintf}: Replaced with @code{__snprintf_chk}.
> +
> +@item @code{sprintf}: Replaced with @code{__sprintf_chk}.
> +
> +@item @code{stpcpy}: Replaced with @code{__stpcpy_chk}.
> +
> +@item @code{stpncpy}: Replaced with @code{__stpncpy_chk}.
> +
> +@item @code{strcat}: Replaced with @code{__strcat_chk}.
> +
> +@item @code{strcpy}: Replaced with @code{__strcpy_chk}.
> +
> +@item @code{strncat}: Replaced with @code{__strncat_chk}.
> +
> +@item @code{strncpy}: Replaced with @code{__strncpy_chk}.
> +
> +@item @code{swprintf}: Replaced with @code{__swprintf_chk}.
> +
> +@item @code{ttyname_r}: Replaced with @code{__ttyname_r_chk}.
> +
> +@item @code{vasprintf}: Replaced with @code{__vasprintf_chk}.
> +
> +@item @code{vdprintf}: Replaced with @code{__vdprintf_chk}.
> +
> +@item @code{vfprintf}: Replaced with @code{__vfprintf_chk}.
> +
> +@item @code{vfwprintf}: Replaced with @code{__vfwprintf_chk}.
> +
> +@item @code{vobprintf}: Replaced with @code{__vobprintf_chk}.
> +
> +@item @code{vprintf}: Replaced with @code{__vprintf_chk}.
> +
> +@item @code{vsnprintf}: Replaced with @code{__vsnprintf_chk}.
> +
> +@item @code{vsprintf}: Replaced with @code{__vsprintf_chk}.
> +
> +@item @code{vswprintf}: Replaced with @code{__vswprintf_chk}.
> +
> +@item @code{vwprintf}: Replaced with @code{__vwprintf_chk}.
> +
> +@item @code{wcpcpy}: Replaced with @code{__wcpcpy_chk}.
> +
> +@item @code{wcpncpy}: Replaced with @code{__wcpncpy_chk}.
> +
> +@item @code{wcrtomb}: Replaced with @code{__wcrtomb_chk}.
> +
> +@item @code{wcscat}: Replaced with @code{__wcscat_chk}.
> +
> +@item @code{wcscpy}: Replaced with @code{__wcscpy_chk}.
> +
> +@item @code{wcsncat}: Replaced with @code{__wcsncat_chk}.
> +
> +@item @code{wcsncpy}: Replaced with @code{__wcsncpy_chk}.
> +
> +@item @code{wcsnrtombs}: Replaced with @code{__wcsnrtombs_chk}.
> +
> +@item @code{wcsrtombs}: Replaced with @code{__wcsrtombs_chk}.
> +
> +@item @code{wcstombs}: Replaced with @code{__wcstombs_chk}.
> +
> +@item @code{wctomb}: Replaced with @code{__wctomb_chk}.
> +
> +@item @code{wmemcpy}: Replaced with @code{__wmemcpy_chk}.
> +
> +@item @code{wmemmove}: Replaced with @code{__wmemmove_chk}.
> +
> +@item @code{wmempcpy}: Replaced with @code{__wmempcpy_chk}.
> +
> +@item @code{wmemset}: Replaced with @code{__wmemset_chk}.
> +
> +@item @code{wprintf}: Replaced with @code{__wprintf_chk}.
> +
> +@end itemize
> +
> +
>   @node Symbol handling
>   @appendixsec Symbol handling in the GNU C Library
>
  

> On 22 Dec 2022, at 12:56, Siddhesh Poyarekar <siddhesh@gotplt.org> wrote:
> 
> Ping!  Any further comments other than the redundant 'See' Andreas noted?
> 

It looks good & thanks for doing it.

I wonder if it's worth noting (maybe it's obvious) that tools which
make use of LD_PRELOAD may need to provide their own wrappers.

(umockdev being an example I'm thinking of here.)

> Thanks,
> Sid
  

* Siddhesh Poyarekar:

> +The @code{_FORTIFY_SOURCE} macro may be defined by users to control
> +hardening of calls into some functions in @theglibc{}.  This feature
> +needs a compiler that supports either the @code{__builtin_object_size}
> +or the @code{__builtin_dynamic_object_size} builtin functions.  When the
> +macro is defined, it enables code that validates access to buffers that
> +are passed to some functions in @theglibc to determine if they
> +are safe.  If the compiler is able to deduce the size of the buffer
> +passed to the function call but the call cannot be determined as safe,
> +it is replaced by a call to its hardened variant that does the access
> +validation at runtime.  At runtime, if the access validation check for
> +the buffer fails, the program will terminate with a @code{SIGABRT}
> +signal.

This doesn't really cover %n checks and the open checks, so it's
slightly misleading.

> +The following functions are fortified in @theglibc{}:
> +
> +@itemize @bullet
> +@item @code{asprintf}: Replaced with @code{__asprintf_chk}.
> +
> +@item @code{confstr}: Replaced with @code{__confstr_chk}.

Can we auto-generate this?

It is incomplete.  __open_2, __open64_2 and the *at variants are
missing.  FD_SET, FD_CLR, FD_ISSET, too.

Thanks,
Florian
  

On 2022-12-22 08:35, Florian Weimer via Libc-alpha wrote:
> * Siddhesh Poyarekar:
> 
>> +The @code{_FORTIFY_SOURCE} macro may be defined by users to control
>> +hardening of calls into some functions in @theglibc{}.  This feature
>> +needs a compiler that supports either the @code{__builtin_object_size}
>> +or the @code{__builtin_dynamic_object_size} builtin functions.  When the
>> +macro is defined, it enables code that validates access to buffers that
>> +are passed to some functions in @theglibc to determine if they
>> +are safe.  If the compiler is able to deduce the size of the buffer
>> +passed to the function call but the call cannot be determined as safe,
>> +it is replaced by a call to its hardened variant that does the access
>> +validation at runtime.  At runtime, if the access validation check for
>> +the buffer fails, the program will terminate with a @code{SIGABRT}
>> +signal.
> 
> This doesn't really cover %n checks and the open checks, so it's
> slightly misleading.

How about the following then; I've mentioned %n in the description for 
level 2 so I'm only trying to provide a high level summary here:

"""
If the compiler is able to deduce the size of the buffer passed to the 
function call but the call cannot be determined as safe, it is replaced 
by a call to its hardened variant that performs additional safety checks 
at runtime.  At runtime, if those safety checks fail, the program will 
terminate with a @code{SIGABRT} signal.
"""

>> +The following functions are fortified in @theglibc{}:
>> +
>> +@itemize @bullet
>> +@item @code{asprintf}: Replaced with @code{__asprintf_chk}.
>> +
>> +@item @code{confstr}: Replaced with @code{__confstr_chk}.
> 
> Can we auto-generate this?
> 
> It is incomplete.  __open_2, __open64_2 and the *at variants are
> missing.  FD_SET, FD_CLR, FD_ISSET, too.

I did auto-generate this list from the entry points in debug/* so it got 
__fdelt_chk, which is the underlying function for the FD_* macros and 
not the macros themselves.  The open* calls got missed because they're 
not in debug.

We could rearrange these entry points to all be in one place in, e.g. 
fortify/ instead of debug/ and then auto-generate from there?  However 
maybe that's a good exercise for the next release and for now, hack 
together the list so that we have documentation.  I don't think I can do 
the refactoring before the freeze.

Thanks,
Sid
  

On 2022-12-22 09:19, Siddhesh Poyarekar via Libc-alpha wrote:
> On 2022-12-22 08:35, Florian Weimer via Libc-alpha wrote:
>> * Siddhesh Poyarekar:
>>
>>> +The @code{_FORTIFY_SOURCE} macro may be defined by users to control
>>> +hardening of calls into some functions in @theglibc{}.  This feature
>>> +needs a compiler that supports either the @code{__builtin_object_size}
>>> +or the @code{__builtin_dynamic_object_size} builtin functions.  When 
>>> the
>>> +macro is defined, it enables code that validates access to buffers that
>>> +are passed to some functions in @theglibc to determine if they
>>> +are safe.  If the compiler is able to deduce the size of the buffer
>>> +passed to the function call but the call cannot be determined as safe,
>>> +it is replaced by a call to its hardened variant that does the access
>>> +validation at runtime.  At runtime, if the access validation check for
>>> +the buffer fails, the program will terminate with a @code{SIGABRT}
>>> +signal.
>>
>> This doesn't really cover %n checks and the open checks, so it's
>> slightly misleading.
> 
> How about the following then; I've mentioned %n in the description for 
> level 2 so I'm only trying to provide a high level summary here:
> 
> """
> If the compiler is able to deduce the size of the buffer passed to the 
> function call but the call cannot be determined as safe, it is replaced 
> by a call to its hardened variant that performs additional safety checks 
> at runtime.  At runtime, if those safety checks fail, the program will 
> terminate with a @code{SIGABRT} signal.
> """
> 

Uhmm, I just noticed that the open* checking is enabled at 
__FORTIFY_LEVEL == 1, so I guess that description needs to change too.

Sid
  

On 2022-12-22 08:08, Sam James wrote:
> 
> 
>> On 22 Dec 2022, at 12:56, Siddhesh Poyarekar <siddhesh@gotplt.org> wrote:
>>
>> Ping!  Any further comments other than the redundant 'See' Andreas noted?
>>
> 
> It looks good & thanks for doing it.
> 
> I wonder if it's worth noting (maybe it's obvious) that tools which
> make use of LD_PRELOAD may need to provide their own wrappers.
> 
> (umockdev being an example I'm thinking of here.)

Hi Sam, sorry I missed your response (and funnily, I got 3 of them!).

I'm not sure, there are other issues with interposing, e.g. string 
function calls where the result is already dependent on how the compiler 
treats those calls, making the result of the interposition kinda 
indeterminate across builds.  So I don't know how useful it would be to 
call it out specifically in the context of _FORTIFY_SOURCE.  However, I 
don't think it's a bad idea to do so either (i.e. I don't see it 
imposing any constraints on us as implementers), so if you like it (and 
others don't express a dislike), I'll be happy to add a note in there.

If there's a positive review for the latest version of this patch, I'll 
post it as an add-on fix or else I'll include it in the next version of 
the patch.

Thanks,
Sid
  

On 1/3/23 07:43, Siddhesh Poyarekar via Libc-alpha wrote:
> On 2022-12-22 08:08, Sam James wrote:
>>
>>
>>> On 22 Dec 2022, at 12:56, Siddhesh Poyarekar <siddhesh@gotplt.org> 
>>> wrote:
>>>
>>> Ping!  Any further comments other than the redundant 'See' Andreas 
>>> noted?
>>>
>>
>> It looks good & thanks for doing it.
>>
>> I wonder if it's worth noting (maybe it's obvious) that tools which
>> make use of LD_PRELOAD may need to provide their own wrappers.
>>
>> (umockdev being an example I'm thinking of here.)
> 
> Hi Sam, sorry I missed your response (and funnily, I got 3 of them!).
> 
> I'm not sure, there are other issues with interposing, e.g. string 
> function calls where the result is already dependent on how the compiler 
> treats those calls, making the result of the interposition kinda 
> indeterminate across builds.  So I don't know how useful it would be to 
> call it out specifically in the context of _FORTIFY_SOURCE.  However, I 
> don't think it's a bad idea to do so either (i.e. I don't see it 
> imposing any constraints on us as implementers), so if you like it (and 
> others don't express a dislike), I'll be happy to add a note in there.
Right.  In general the compiler may replace a variety of library calls 
with inline versions.  The set of calls subject to this behavior isn't 
stable across compiler versions, targets or even across different call 
sites as the call site context may affect if the compiler thinks 
inlining is profitable or not.

jeff