[01/11] misc: Build getdomainname with fortification

Message ID 7710004342d09a8e62868a0bc659b5dca5b67324.1707491940.git.fweimer@redhat.com
State Accepted
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:24 p.m. UTC
  Introduce __glibc_nofortify_getdomainname to request disabling
the fortification wrapper.
---
 misc/Makefile                 | 1 -
 misc/getdomain.c              | 2 ++
 posix/bits/unistd.h           | 3 ++-
 sysdeps/mach/hurd/getdomain.c | 2 ++
 4 files changed, 6 insertions(+), 2 deletions(-)
  

Comments

Adhemerval Zanella Feb. 12, 2024, 5:14 p.m. UTC | #1
On 09/02/24 12:24, Florian Weimer wrote:
> Introduce __glibc_nofortify_getdomainname to request disabling
> the fortification wrapper.

I am not very found of __glibc_nofortify_getdomainname escaping to an
installed header, but I don't have a better solution.

LGTM, thanks.

Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>

> ---
>  misc/Makefile                 | 1 -
>  misc/getdomain.c              | 2 ++
>  posix/bits/unistd.h           | 3 ++-
>  sysdeps/mach/hurd/getdomain.c | 2 ++
>  4 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/misc/Makefile b/misc/Makefile
> index c273ec6974..44ae89670a 100644
> --- a/misc/Makefile
> +++ b/misc/Makefile
> @@ -209,7 +209,6 @@ routines := \
>  
>  # Exclude fortified routines from being built with _FORTIFY_SOURCE
>  routines_no_fortify += \
> -  getdomain \
>    gethostname \
>    syslog \
>    # routines_no_fortify
> diff --git a/misc/getdomain.c b/misc/getdomain.c
> index f6325644c9..f96636821f 100644
> --- a/misc/getdomain.c
> +++ b/misc/getdomain.c
> @@ -19,6 +19,8 @@
>     The result is null-terminated if LEN is large enough for the full
>     name and the terminator.  */
>  
> +#define __glibc_nofortify_getdomainname
> +
>  #include <errno.h>
>  #include <unistd.h>
>  #include <sys/param.h>
> diff --git a/posix/bits/unistd.h b/posix/bits/unistd.h
> index bd209ec28e..6ed2943bf1 100644
> --- a/posix/bits/unistd.h
> +++ b/posix/bits/unistd.h
> @@ -149,7 +149,8 @@ __NTH (gethostname (char *__buf, size_t __buflen))
>  #endif
>  
>  
> -#if defined __USE_MISC || (defined __USE_XOPEN && !defined __USE_UNIX98)
> +#if (defined __USE_MISC || (defined __USE_XOPEN && !defined __USE_UNIX98)) \
> +  && !defined __glibc_nofortify_getdomainname
>  __fortify_function int
>  __NTH (getdomainname (char *__buf, size_t __buflen))
>  {
> diff --git a/sysdeps/mach/hurd/getdomain.c b/sysdeps/mach/hurd/getdomain.c
> index 137ce9ad5b..e24ab2137e 100644
> --- a/sysdeps/mach/hurd/getdomain.c
> +++ b/sysdeps/mach/hurd/getdomain.c
> @@ -15,6 +15,8 @@
>     License along with the GNU C Library; if not, see
>     <https://www.gnu.org/licenses/>.  */
>  
> +#define __glibc_nofortify_getdomainname
> +
>  #include <unistd.h>
>  #include "hurdhost.h"
>
  
Andreas Schwab Feb. 12, 2024, 5:30 p.m. UTC | #2
On Feb 12 2024, Adhemerval Zanella Netto wrote:

> On 09/02/24 12:24, Florian Weimer wrote:
>> Introduce __glibc_nofortify_getdomainname to request disabling
>> the fortification wrapper.
>
> I am not very found of __glibc_nofortify_getdomainname escaping to an
> installed header, but I don't have a better solution.

Can't this be fixed with an alias?
  
Florian Weimer Feb. 12, 2024, 7:29 p.m. UTC | #3
* Andreas Schwab:

> On Feb 12 2024, Adhemerval Zanella Netto wrote:
>
>> On 09/02/24 12:24, Florian Weimer wrote:
>>> Introduce __glibc_nofortify_getdomainname to request disabling
>>> the fortification wrapper.
>>
>> I am not very found of __glibc_nofortify_getdomainname escaping to an
>> installed header, but I don't have a better solution.
>
> Can't this be fixed with an alias?

It would have to be an assembler-level alias that GCC doesn't know
about.  Would you prefer that?  It results in worse debugging
information, I think.

GCC's alias attribyte counts a definition, and we already have one due
to the inline function definition in the fortify header.
  
Andreas Schwab Feb. 13, 2024, 9:12 a.m. UTC | #4
On Feb 12 2024, Florian Weimer wrote:

> * Andreas Schwab:
>
>> On Feb 12 2024, Adhemerval Zanella Netto wrote:
>>
>>> On 09/02/24 12:24, Florian Weimer wrote:
>>>> Introduce __glibc_nofortify_getdomainname to request disabling
>>>> the fortification wrapper.
>>>
>>> I am not very found of __glibc_nofortify_getdomainname escaping to an
>>> installed header, but I don't have a better solution.
>>
>> Can't this be fixed with an alias?
>
> It would have to be an assembler-level alias that GCC doesn't know
> about.  Would you prefer that?  It results in worse debugging
> information, I think.

I didn't realize that all the fortified functions are compiled with
disabled fortification for that reason.
  
Florian Weimer Feb. 13, 2024, 9:23 a.m. UTC | #5
* Andreas Schwab:

> On Feb 12 2024, Florian Weimer wrote:
>
>> * Andreas Schwab:
>>
>>> On Feb 12 2024, Adhemerval Zanella Netto wrote:
>>>
>>>> On 09/02/24 12:24, Florian Weimer wrote:
>>>>> Introduce __glibc_nofortify_getdomainname to request disabling
>>>>> the fortification wrapper.
>>>>
>>>> I am not very found of __glibc_nofortify_getdomainname escaping to an
>>>> installed header, but I don't have a better solution.
>>>
>>> Can't this be fixed with an alias?
>>
>> It would have to be an assembler-level alias that GCC doesn't know
>> about.  Would you prefer that?  It results in worse debugging
>> information, I think.
>
> I didn't realize that all the fortified functions are compiled with
> disabled fortification for that reason.

Does this mean you are okay with the patch as posted?

Thanks,
Florian
  
Andreas Schwab Feb. 13, 2024, 9:32 a.m. UTC | #6
On Feb 13 2024, Florian Weimer wrote:

> Does this mean you are okay with the patch as posted?

Yes.
  

Patch

diff --git a/misc/Makefile b/misc/Makefile
index c273ec6974..44ae89670a 100644
--- a/misc/Makefile
+++ b/misc/Makefile
@@ -209,7 +209,6 @@  routines := \
 
 # Exclude fortified routines from being built with _FORTIFY_SOURCE
 routines_no_fortify += \
-  getdomain \
   gethostname \
   syslog \
   # routines_no_fortify
diff --git a/misc/getdomain.c b/misc/getdomain.c
index f6325644c9..f96636821f 100644
--- a/misc/getdomain.c
+++ b/misc/getdomain.c
@@ -19,6 +19,8 @@ 
    The result is null-terminated if LEN is large enough for the full
    name and the terminator.  */
 
+#define __glibc_nofortify_getdomainname
+
 #include <errno.h>
 #include <unistd.h>
 #include <sys/param.h>
diff --git a/posix/bits/unistd.h b/posix/bits/unistd.h
index bd209ec28e..6ed2943bf1 100644
--- a/posix/bits/unistd.h
+++ b/posix/bits/unistd.h
@@ -149,7 +149,8 @@  __NTH (gethostname (char *__buf, size_t __buflen))
 #endif
 
 
-#if defined __USE_MISC || (defined __USE_XOPEN && !defined __USE_UNIX98)
+#if (defined __USE_MISC || (defined __USE_XOPEN && !defined __USE_UNIX98)) \
+  && !defined __glibc_nofortify_getdomainname
 __fortify_function int
 __NTH (getdomainname (char *__buf, size_t __buflen))
 {
diff --git a/sysdeps/mach/hurd/getdomain.c b/sysdeps/mach/hurd/getdomain.c
index 137ce9ad5b..e24ab2137e 100644
--- a/sysdeps/mach/hurd/getdomain.c
+++ b/sysdeps/mach/hurd/getdomain.c
@@ -15,6 +15,8 @@ 
    License along with the GNU C Library; if not, see
    <https://www.gnu.org/licenses/>.  */
 
+#define __glibc_nofortify_getdomainname
+
 #include <unistd.h>
 #include "hurdhost.h"