[03/11] libio: Add fortify wrapper for internal __snprintf

Message ID 45d399f3e70726984705466772370ba33ce3aa8a.1707491940.git.fweimer@redhat.com
State Changes Requested
Delegated to: Adhemerval Zanella Netto
Headers
Series Build getdomainname, gethostname, syslog with fortification |

Checks

Context Check Description
redhat-pt-bot/TryBot-apply_patch success Patch applied to master at the time it was sent
linaro-tcwg-bot/tcwg_glibc_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_glibc_check--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_glibc_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_glibc_check--master-arm success Testing passed

Commit Message

Florian Weimer Feb. 9, 2024, 3:25 p.m. UTC
  ---
 debug/snprintf_chk.c  | 1 +
 include/bits/stdio2.h | 9 +++++++++
 include/stdio.h       | 1 +
 3 files changed, 11 insertions(+)
  

Comments

Adhemerval Zanella Netto Feb. 12, 2024, 5:34 p.m. UTC | #1
On 09/02/24 12:25, Florian Weimer wrote:
> ---
>  debug/snprintf_chk.c  | 1 +
>  include/bits/stdio2.h | 9 +++++++++
>  include/stdio.h       | 1 +
>  3 files changed, 11 insertions(+)
> 
> diff --git a/debug/snprintf_chk.c b/debug/snprintf_chk.c
> index 995d805f5d..ea02efec3e 100644
> --- a/debug/snprintf_chk.c
> +++ b/debug/snprintf_chk.c
> @@ -40,4 +40,5 @@ ___snprintf_chk (char *s, size_t maxlen, int flag, size_t slen,
>  
>    return ret;
>  }
> +ldbl_hidden_def (___snprintf_chk, __snprintf_chk)
>  ldbl_strong_alias (___snprintf_chk, __snprintf_chk)

I am seeing a build failure on powerpc64le with config options 
--enable-stack-protector=all --enable-tunables=yes --enable-bind-now=yes 
--enable-profile=yes --enable-fortify-source=2 --enable-hardcoded-path-in-tests:

powerpc64le-glibc-linux-gnu-gcc gconv_conf.c -c -std=gnu11 -fgnu89-inline  -g -O2 -Wall -Wwrite-strings -Wundef -Werror -fmerge-all-constants -frounding-math -fstack-protector-all -fno-common -Wp,-U_FORTIFY_SOURCE,-D_FORTIFY_SOURCE=2 -Wstrict-prototypes -Wold-style-definition -fmath-errno  -mabi=ieeelongdouble -Wno-psabi -mno-gnu-attribute -mlong-double-128  -fPIE [....]
In file included from gconv_conf.c:26:
../include/stdio.h:65:1: error: ‘asm’ declaration ignored due to conflict with previous rename [-Werror=pragmas]
   65 | stdio_hidden_ldbl_proto (__, snprintf_chk)
      | ^~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors

All other ABIs build fine.

> diff --git a/include/bits/stdio2.h b/include/bits/stdio2.h
> index 6d7cf4306f..fb909c21ee 100644
> --- a/include/bits/stdio2.h
> +++ b/include/bits/stdio2.h
> @@ -1 +1,10 @@
>  #include <libio/bits/stdio2.h>
> +
> +__fortify_function int
> +__NTH (__snprintf (char *__restrict __s, size_t __n,
> +                   const char *__restrict __fmt, ...))
> +{
> +  return __builtin___snprintf_chk (__s, __n, __USE_FORTIFY_LEVEL - 1,
> +				   __glibc_objsize (__s), __fmt,
> +				   __va_arg_pack ());
> +}
> diff --git a/include/stdio.h b/include/stdio.h
> index 24f1652f19..364f4d22a1 100644
> --- a/include/stdio.h
> +++ b/include/stdio.h
> @@ -62,6 +62,7 @@ extern int __vsscanf (const char *__restrict __s,
>  extern int __sprintf_chk (char *, int, size_t, const char *, ...) __THROW;
>  extern int __snprintf_chk (char *, size_t, int, size_t, const char *, ...)
>       __THROW;
> +stdio_hidden_ldbl_proto (__, snprintf_chk)
>  extern int __vsprintf_chk (char *, int, size_t, const char *,
>  			   __gnuc_va_list) __THROW;
>  extern int __vsnprintf_chk (char *, size_t, int, size_t, const char *,
  
Florian Weimer Feb. 13, 2024, 12:13 p.m. UTC | #2
* Adhemerval Zanella Netto:

> On 09/02/24 12:25, Florian Weimer wrote:
>> ---
>>  debug/snprintf_chk.c  | 1 +
>>  include/bits/stdio2.h | 9 +++++++++
>>  include/stdio.h       | 1 +
>>  3 files changed, 11 insertions(+)
>> 
>> diff --git a/debug/snprintf_chk.c b/debug/snprintf_chk.c
>> index 995d805f5d..ea02efec3e 100644
>> --- a/debug/snprintf_chk.c
>> +++ b/debug/snprintf_chk.c
>> @@ -40,4 +40,5 @@ ___snprintf_chk (char *s, size_t maxlen, int flag, size_t slen,
>>  
>>    return ret;
>>  }
>> +ldbl_hidden_def (___snprintf_chk, __snprintf_chk)
>>  ldbl_strong_alias (___snprintf_chk, __snprintf_chk)
>
> I am seeing a build failure on powerpc64le with config options 
> --enable-stack-protector=all --enable-tunables=yes --enable-bind-now=yes 
> --enable-profile=yes --enable-fortify-source=2 --enable-hardcoded-path-in-tests:
>
> powerpc64le-glibc-linux-gnu-gcc gconv_conf.c -c -std=gnu11 -fgnu89-inline  -g -O2 -Wall -Wwrite-strings -Wundef -Werror -fmerge-all-constants -frounding-math -fstack-protector-all -fno-common -Wp,-U_FORTIFY_SOURCE,-D_FORTIFY_SOURCE=2 -Wstrict-prototypes -Wold-style-definition -fmath-errno  -mabi=ieeelongdouble -Wno-psabi -mno-gnu-attribute -mlong-double-128  -fPIE [....]
> In file included from gconv_conf.c:26:
> ../include/stdio.h:65:1: error: ‘asm’ declaration ignored due to conflict with previous rename [-Werror=pragmas]
>    65 | stdio_hidden_ldbl_proto (__, snprintf_chk)
>       | ^~~~~~~~~~~~~~~~~~~~~~~
> cc1: all warnings being treated as errors
>
> All other ABIs build fine.

The issue seems to be that libio/bits/stdio-ldbl.h wants to alias
__snprintf_chk to __snprintf_chkieee128, and stdio_hidden_ldbl_proto
wants to create an alias to ___ieee128_snprintf_chk.

I don't know how to solve this properly.  It seems that so far, few of
the fortified variants in the printf function family have hidden aliases
for PLT avoidance.

Thanks,
Florian
  
Adhemerval Zanella Netto Feb. 13, 2024, 1:16 p.m. UTC | #3
On 13/02/24 09:13, Florian Weimer wrote:
> * Adhemerval Zanella Netto:
> 
>> On 09/02/24 12:25, Florian Weimer wrote:
>>> ---
>>>  debug/snprintf_chk.c  | 1 +
>>>  include/bits/stdio2.h | 9 +++++++++
>>>  include/stdio.h       | 1 +
>>>  3 files changed, 11 insertions(+)
>>>
>>> diff --git a/debug/snprintf_chk.c b/debug/snprintf_chk.c
>>> index 995d805f5d..ea02efec3e 100644
>>> --- a/debug/snprintf_chk.c
>>> +++ b/debug/snprintf_chk.c
>>> @@ -40,4 +40,5 @@ ___snprintf_chk (char *s, size_t maxlen, int flag, size_t slen,
>>>  
>>>    return ret;
>>>  }
>>> +ldbl_hidden_def (___snprintf_chk, __snprintf_chk)
>>>  ldbl_strong_alias (___snprintf_chk, __snprintf_chk)
>>
>> I am seeing a build failure on powerpc64le with config options 
>> --enable-stack-protector=all --enable-tunables=yes --enable-bind-now=yes 
>> --enable-profile=yes --enable-fortify-source=2 --enable-hardcoded-path-in-tests:
>>
>> powerpc64le-glibc-linux-gnu-gcc gconv_conf.c -c -std=gnu11 -fgnu89-inline  -g -O2 -Wall -Wwrite-strings -Wundef -Werror -fmerge-all-constants -frounding-math -fstack-protector-all -fno-common -Wp,-U_FORTIFY_SOURCE,-D_FORTIFY_SOURCE=2 -Wstrict-prototypes -Wold-style-definition -fmath-errno  -mabi=ieeelongdouble -Wno-psabi -mno-gnu-attribute -mlong-double-128  -fPIE [....]
>> In file included from gconv_conf.c:26:
>> ../include/stdio.h:65:1: error: ‘asm’ declaration ignored due to conflict with previous rename [-Werror=pragmas]
>>    65 | stdio_hidden_ldbl_proto (__, snprintf_chk)
>>       | ^~~~~~~~~~~~~~~~~~~~~~~
>> cc1: all warnings being treated as errors
>>
>> All other ABIs build fine.
> 
> The issue seems to be that libio/bits/stdio-ldbl.h wants to alias
> __snprintf_chk to __snprintf_chkieee128, and stdio_hidden_ldbl_proto
> wants to create an alias to ___ieee128_snprintf_chk.
> 
> I don't know how to solve this properly.  It seems that so far, few of
> the fortified variants in the printf function family have hidden aliases
> for PLT avoidance.

Maybe something like:

diff --git a/include/stdio.h b/include/stdio.h
index 364f4d22a1..0fda47cf82 100644
--- a/include/stdio.h
+++ b/include/stdio.h
@@ -24,7 +24,7 @@
     directly alias them to their internal name.  */
 # if __LDOUBLE_REDIRECTS_TO_FLOAT128_ABI == 1 && IS_IN (libc)
 #  define stdio_hidden_ldbl_proto(p, f) \
-  extern __typeof (p ## f) p ## f __asm (__ASMNAME ("___ieee128_" #f));
+  libc_hidden_proto_alias (p ## f, ___ieee128_ ## p ## f)
 # elif __LDOUBLE_REDIRECTS_TO_FLOAT128_ABI == 1
 #  define stdio_hidden_ldbl_proto(p,f) __LDBL_REDIR1_DECL (p ## f, p ## f ## ieee128)
 # else
diff --git a/sysdeps/ieee754/ldbl-128ibm-compat/ieee128-snprintf_chk.c b/sysdeps/ieee754/ldbl-128ibm-compat/ieee128-snprintf_chk.c
index d5bd6aa25f..566f525a9c 100644
--- a/sysdeps/ieee754/ldbl-128ibm-compat/ieee128-snprintf_chk.c
+++ b/sysdeps/ieee754/ldbl-128ibm-compat/ieee128-snprintf_chk.c
@@ -40,3 +40,4 @@ ___ieee128___snprintf_chk (char *s, size_t maxlen, int flag, size_t slen,
   return done;
 }
 strong_alias (___ieee128___snprintf_chk, __snprintf_chkieee128)
+libc_hidden_ver (__snprintf_chkieee128, ___ieee128___snprintf_chk)
  

Patch

diff --git a/debug/snprintf_chk.c b/debug/snprintf_chk.c
index 995d805f5d..ea02efec3e 100644
--- a/debug/snprintf_chk.c
+++ b/debug/snprintf_chk.c
@@ -40,4 +40,5 @@  ___snprintf_chk (char *s, size_t maxlen, int flag, size_t slen,
 
   return ret;
 }
+ldbl_hidden_def (___snprintf_chk, __snprintf_chk)
 ldbl_strong_alias (___snprintf_chk, __snprintf_chk)
diff --git a/include/bits/stdio2.h b/include/bits/stdio2.h
index 6d7cf4306f..fb909c21ee 100644
--- a/include/bits/stdio2.h
+++ b/include/bits/stdio2.h
@@ -1 +1,10 @@ 
 #include <libio/bits/stdio2.h>
+
+__fortify_function int
+__NTH (__snprintf (char *__restrict __s, size_t __n,
+                   const char *__restrict __fmt, ...))
+{
+  return __builtin___snprintf_chk (__s, __n, __USE_FORTIFY_LEVEL - 1,
+				   __glibc_objsize (__s), __fmt,
+				   __va_arg_pack ());
+}
diff --git a/include/stdio.h b/include/stdio.h
index 24f1652f19..364f4d22a1 100644
--- a/include/stdio.h
+++ b/include/stdio.h
@@ -62,6 +62,7 @@  extern int __vsscanf (const char *__restrict __s,
 extern int __sprintf_chk (char *, int, size_t, const char *, ...) __THROW;
 extern int __snprintf_chk (char *, size_t, int, size_t, const char *, ...)
      __THROW;
+stdio_hidden_ldbl_proto (__, snprintf_chk)
 extern int __vsprintf_chk (char *, int, size_t, const char *,
 			   __gnuc_va_list) __THROW;
 extern int __vsnprintf_chk (char *, size_t, int, size_t, const char *,