[v7,1/4] elf: Cleanup and improve tst-execstack

Use libsupport and handle new SELinux versions.
---
 elf/tst-execstack.c | 200 ++++++++++++++++++++------------------------
 1 file changed, 91 insertions(+), 109 deletions(-)

* Adhemerval Zanella:

> +static bool
> +check_selinux (void)
> +{
> +  /* Old Red Hat like systems.  */
> +  enum selinux_status r =
> +    check_selinux_generic ("/selinux/enforce",
> +			   "/selinux/booleans/allow_execstack");
> +  if (r == SELINUX_NOT_PRESENT)
> +    /* New Red Hard like systems.  */
> +    r = check_selinux_generic ("/sys/fs/selinux/enforce",
> +			       "/sys/fs/selinux/booleans/selinuxuser_execstack");
> +  return r == SELINUX_NOT_PRESENT || r == SELINUX_EXECSTACK_ENABLE;
> +}

Typo: Red Ha[t]

I think this isn't quite right.  The path prefix (selinuxfs mount point)
is independent of the policy style.  The reference policy
<https://github.com/SELinuxProject/refpolicy> still uses allow_execstack
in its most recent versions.  Only the Fedora policy
<https://github.com/fedora-selinux/selinux-policy> uses
selinuxuser_execstack.

Rest of the changes look okay.

Thanks,
Florian

On 30/12/24 14:08, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>> +static bool
>> +check_selinux (void)
>> +{
>> +  /* Old Red Hat like systems.  */
>> +  enum selinux_status r =
>> +    check_selinux_generic ("/selinux/enforce",
>> +			   "/selinux/booleans/allow_execstack");
>> +  if (r == SELINUX_NOT_PRESENT)
>> +    /* New Red Hard like systems.  */
>> +    r = check_selinux_generic ("/sys/fs/selinux/enforce",
>> +			       "/sys/fs/selinux/booleans/selinuxuser_execstack");
>> +  return r == SELINUX_NOT_PRESENT || r == SELINUX_EXECSTACK_ENABLE;
>> +}
> 
> Typo: Red Ha[t]
> 
> I think this isn't quite right.  The path prefix (selinuxfs mount point)
> is independent of the policy style.  The reference policy
> <https://github.com/SELinuxProject/refpolicy> still uses allow_execstack
> in its most recent versions.  Only the Fedora policy
> <https://github.com/fedora-selinux/selinux-policy> uses
> selinuxuser_execstack.

Right, do you prefer if I can move this to a different patch? My first
approach was to use a list of mount points and check for both policy
styles.

I don't have access to a Red Hat release, so I used as a proxy the CentOS 
and Alma Linux we have on gcc compile farm.  It seems that such system
might not be a 100% proxy for Red Hat systems.

> 
> Rest of the changes look okay.
> 
> Thanks,
> Florian
>

* Adhemerval Zanella Netto:

> On 30/12/24 14:08, Florian Weimer wrote:
>> * Adhemerval Zanella:
>> 
>>> +static bool
>>> +check_selinux (void)
>>> +{
>>> +  /* Old Red Hat like systems.  */
>>> +  enum selinux_status r =
>>> +    check_selinux_generic ("/selinux/enforce",
>>> +			   "/selinux/booleans/allow_execstack");
>>> +  if (r == SELINUX_NOT_PRESENT)
>>> +    /* New Red Hard like systems.  */
>>> +    r = check_selinux_generic ("/sys/fs/selinux/enforce",
>>> +			       "/sys/fs/selinux/booleans/selinuxuser_execstack");
>>> +  return r == SELINUX_NOT_PRESENT || r == SELINUX_EXECSTACK_ENABLE;
>>> +}
>> 
>> Typo: Red Ha[t]
>> 
>> I think this isn't quite right.  The path prefix (selinuxfs mount point)
>> is independent of the policy style.  The reference policy
>> <https://github.com/SELinuxProject/refpolicy> still uses allow_execstack
>> in its most recent versions.  Only the Fedora policy
>> <https://github.com/fedora-selinux/selinux-policy> uses
>> selinuxuser_execstack.
>
> Right, do you prefer if I can move this to a different patch? My first
> approach was to use a list of mount points and check for both policy
> styles.

Let's keep it separate I would say.  It seems uncommon to test glibc
with allow_execstack or equivalent disabled.

> I don't have access to a Red Hat release, so I used as a proxy the CentOS 
> and Alma Linux we have on gcc compile farm.  It seems that such system
> might not be a 100% proxy for Red Hat systems.

I don't think downstreams from RHEL deviate here.  The last RHEL release
that used /selinux as the mount was RHEL 6.  It's got a heavily patched
2.6.32 kernel (with lots of *at system call backports).  It's probably
good enough to build and run glibc now that we have removed the kernel
version check, but I don't expect anyone doing that.

I don't know which mountpoints other distributions use (that aren't
downstream from Fedora).  Fedora formally forked from refpolicy here:

commit 51dc83b2d4cd22cff6fbf6116b954b4f672b90c4
Author: Lukas Vrabec <lvrabec@redhat.com>
Date:   Sun Dec 24 14:31:11 2017 +0100

    Commit removes big SELinux policy patches against tresys refpolicy.
    
    We're quite diverted from upstream policy. This change will use tarballs
    from github projects:
    https://github.com/fedora-selinux/selinux-policy
    https://github.com/fedora-selinux/selinux-policy-contrib

But the introduction of selinuxuser_execstack is even older, it seems to
date back to:

commit c3956376c7bcb2927655da7737fe6c064d746d72
Author: Dan Walsh <dwalsh@redhat.com>
Date:   Fri Jun 8 10:09:54 2012 -0400

    Add booleans.subs_dist to selinux-policy package

And that didn't make it into the RHEL 6 release, only RHEL 7 and later.

I expect other distributions based on refpolicy to still use
allow_execstack, though.

Thanks,
Florian

On 30/12/24 16:47, Florian Weimer wrote:
> * Adhemerval Zanella Netto:
> 
>> On 30/12/24 14:08, Florian Weimer wrote:
>>> * Adhemerval Zanella:
>>>
>>>> +static bool
>>>> +check_selinux (void)
>>>> +{
>>>> +  /* Old Red Hat like systems.  */
>>>> +  enum selinux_status r =
>>>> +    check_selinux_generic ("/selinux/enforce",
>>>> +			   "/selinux/booleans/allow_execstack");
>>>> +  if (r == SELINUX_NOT_PRESENT)
>>>> +    /* New Red Hard like systems.  */
>>>> +    r = check_selinux_generic ("/sys/fs/selinux/enforce",
>>>> +			       "/sys/fs/selinux/booleans/selinuxuser_execstack");
>>>> +  return r == SELINUX_NOT_PRESENT || r == SELINUX_EXECSTACK_ENABLE;
>>>> +}
>>>
>>> Typo: Red Ha[t]
>>>
>>> I think this isn't quite right.  The path prefix (selinuxfs mount point)
>>> is independent of the policy style.  The reference policy
>>> <https://github.com/SELinuxProject/refpolicy> still uses allow_execstack
>>> in its most recent versions.  Only the Fedora policy
>>> <https://github.com/fedora-selinux/selinux-policy> uses
>>> selinuxuser_execstack.
>>
>> Right, do you prefer if I can move this to a different patch? My first
>> approach was to use a list of mount points and check for both policy
>> styles.
> 
> Let's keep it separate I would say.  It seems uncommon to test glibc
> with allow_execstack or equivalent disabled.

Ack, updated patch in below.

> 
>> I don't have access to a Red Hat release, so I used as a proxy the CentOS 
>> and Alma Linux we have on gcc compile farm.  It seems that such system
>> might not be a 100% proxy for Red Hat systems.
> 
> I don't think downstreams from RHEL deviate here.  The last RHEL release
> that used /selinux as the mount was RHEL 6.  It's got a heavily patched
> 2.6.32 kernel (with lots of *at system call backports).  It's probably
> good enough to build and run glibc now that we have removed the kernel
> version check, but I don't expect anyone doing that.
> 
> I don't know which mountpoints other distributions use (that aren't
> downstream from Fedora).  Fedora formally forked from refpolicy here:
> 
> commit 51dc83b2d4cd22cff6fbf6116b954b4f672b90c4
> Author: Lukas Vrabec <lvrabec@redhat.com>
> Date:   Sun Dec 24 14:31:11 2017 +0100
> 
>     Commit removes big SELinux policy patches against tresys refpolicy.
>     
>     We're quite diverted from upstream policy. This change will use tarballs
>     from github projects:
>     https://github.com/fedora-selinux/selinux-policy
>     https://github.com/fedora-selinux/selinux-policy-contrib
> 
> But the introduction of selinuxuser_execstack is even older, it seems to
> date back to:
> 
> commit c3956376c7bcb2927655da7737fe6c064d746d72
> Author: Dan Walsh <dwalsh@redhat.com>
> Date:   Fri Jun 8 10:09:54 2012 -0400
> 
>     Add booleans.subs_dist to selinux-policy package
> 
> And that didn't make it into the RHEL 6 release, only RHEL 7 and later.
> 
> I expect other distributions based on refpolicy to still use
> allow_execstack, though.

Thanks for digging this up.  I added this snippet just because I noticed
the tests was being ineffective on CentOS and other Fedora like system.

---

From d66cce76422f1c53e7f4e9dced3112e5f90c1e28 Mon Sep 17 00:00:00 2001
From: Adhemerval Zanella <adhemerval.zanella@linaro.org>
Date: Thu, 26 Dec 2024 11:25:28 -0300
Subject: [PATCH 1/4] elf: Cleanup and improve tst-execstack

Use libsupport and handle new SELinux versions.
---
 elf/tst-execstack.c | 126 ++++++++++++++------------------------------
 1 file changed, 41 insertions(+), 85 deletions(-)

diff --git a/elf/tst-execstack.c b/elf/tst-execstack.c
index 560b353918..fae2930f70 100644
--- a/elf/tst-execstack.c
+++ b/elf/tst-execstack.c
@@ -1,24 +1,32 @@
 /* Test program for making nonexecutable stacks executable
-   on load of a DSO that requires executable stacks.  */
+   on load of a DSO that requires executable stacks.
 
-#include <dlfcn.h>
-#include <stdbool.h>
-#include <stdio.h>
-#include <string.h>
-#include <unistd.h>
+   Copyright (C) 2003-2024 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#include <array_length.h>
 #include <error.h>
 #include <stackinfo.h>
-
-static void
-print_maps (void)
-{
-#if 0
-  char *cmd = NULL;
-  asprintf (&cmd, "cat /proc/%d/maps", getpid ());
-  system (cmd);
-  free (cmd);
-#endif
-}
+#include <stdbool.h>
+#include <string.h>
+#include <support/xdlfcn.h>
+#include <support/xthread.h>
+#include <support/check.h>
+#include <support/xstdio.h>
 
 static void deeper (void (*f) (void));
 
@@ -38,8 +46,8 @@ static void *
 waiter_thread (void *arg)
 {
   void **f = arg;
-  pthread_barrier_wait (&startup_barrier);
-  pthread_barrier_wait (&go_barrier);
+  xpthread_barrier_wait (&startup_barrier);
+  xpthread_barrier_wait (&go_barrier);
 
   (*((void (*) (void)) *f)) ();
 
@@ -88,47 +96,26 @@ do_test (void)
 #if USE_PTHREADS
   /* Create some threads while stacks are nonexecutable.  */
   #define N 5
-  pthread_t thr[N];
 
-  pthread_barrier_init (&startup_barrier, NULL, N + 1);
-  pthread_barrier_init (&go_barrier, NULL, N + 1);
+  xpthread_barrier_init (&startup_barrier, NULL, N + 1);
+  xpthread_barrier_init (&go_barrier, NULL, N + 1);
 
   for (int i = 0; i < N; ++i)
-    {
-      int rc = pthread_create (&thr[i], NULL, &waiter_thread, &f);
-      if (rc)
-	error (1, rc, "pthread_create");
-    }
+    xpthread_create (NULL, &waiter_thread, &f);
 
   /* Make sure they are all there using their stacks.  */
-  pthread_barrier_wait (&startup_barrier);
+  xpthread_barrier_wait (&startup_barrier);
   puts ("threads waiting");
 #endif
 
-  print_maps ();
-
 #if USE_PTHREADS
   void *old_stack_addr, *new_stack_addr;
   size_t stack_size;
   pthread_t me = pthread_self ();
   pthread_attr_t attr;
-  int ret = 0;
-
-  ret = pthread_getattr_np (me, &attr);
-  if (ret)
-    {
-      printf ("before execstack: pthread_getattr_np returned error: %s\n",
-	      strerror (ret));
-      return 1;
-    }
-
-  ret = pthread_attr_getstack (&attr, &old_stack_addr, &stack_size);
-  if (ret)
-    {
-      printf ("before execstack: pthread_attr_getstack returned error: %s\n",
-	      strerror (ret));
-      return 1;
-    }
+  TEST_VERIFY_EXIT (pthread_getattr_np (me, &attr) == 0);
+  TEST_VERIFY_EXIT (pthread_attr_getstack (&attr, &old_stack_addr,
+					    &stack_size) == 0);
 # if _STACK_GROWS_DOWN
     old_stack_addr += stack_size;
 # else
@@ -149,36 +136,17 @@ do_test (void)
       return allow_execstack;
     }
 
-  f = dlsym (h, "tryme");
-  if (f == NULL)
-    {
-      printf ("symbol not found: %s\n", dlerror ());
-      return 1;
-    }
+  f = xdlsym (h, "tryme");
 
   /* Test if that really made our stack executable.
      The `tryme' function should crash if not.  */
 
   (*((void (*) (void)) f)) ();
 
-  print_maps ();
-
 #if USE_PTHREADS
-  ret = pthread_getattr_np (me, &attr);
-  if (ret)
-    {
-      printf ("after execstack: pthread_getattr_np returned error: %s\n",
-	      strerror (ret));
-      return 1;
-    }
-
-  ret = pthread_attr_getstack (&attr, &new_stack_addr, &stack_size);
-  if (ret)
-    {
-      printf ("after execstack: pthread_attr_getstack returned error: %s\n",
-	      strerror (ret));
-      return 1;
-    }
+  TEST_VERIFY_EXIT (pthread_getattr_np (me, &attr) == 0);
+  TEST_VERIFY_EXIT (pthread_attr_getstack (&attr, &new_stack_addr,
+					    &stack_size) == 0);
 
 # if _STACK_GROWS_DOWN
     new_stack_addr += stack_size;
@@ -194,33 +162,21 @@ do_test (void)
      stacksize and stackaddr respectively.  If the size changes due to the
      above, then both stacksize and stackaddr can change, but the stack bottom
      should remain the same, which is computed as stackaddr + stacksize.  */
-  if (old_stack_addr != new_stack_addr)
-    {
-      printf ("Stack end changed, old: %p, new: %p\n",
-	      old_stack_addr, new_stack_addr);
-      return 1;
-    }
+  TEST_VERIFY_EXIT (old_stack_addr == new_stack_addr);
   printf ("Stack address remains the same: %p\n", old_stack_addr);
 #endif
 
   /* Test that growing the stack region gets new executable pages too.  */
   deeper ((void (*) (void)) f);
 
-  print_maps ();
-
 #if USE_PTHREADS
   /* Test that a fresh thread now gets an executable stack.  */
-  {
-    pthread_t th;
-    int rc = pthread_create (&th, NULL, &tryme_thread, f);
-    if (rc)
-      error (1, rc, "pthread_create");
-  }
+  xpthread_create (NULL, &tryme_thread, f);
 
   puts ("threads go");
   /* The existing threads' stacks should have been changed.
      Let them run to test it.  */
-  pthread_barrier_wait (&go_barrier);
+  xpthread_barrier_wait (&go_barrier);
 
   pthread_exit ((void *) (long int) (! allow_execstack));
 #endif

* Adhemerval Zanella Netto:

> From d66cce76422f1c53e7f4e9dced3112e5f90c1e28 Mon Sep 17 00:00:00 2001
> From: Adhemerval Zanella <adhemerval.zanella@linaro.org>
> Date: Thu, 26 Dec 2024 11:25:28 -0300
> Subject: [PATCH 1/4] elf: Cleanup and improve tst-execstack
>
> Use libsupport and handle new SELinux versions.

Commit message is now outdated.

For the rest:

Reviewed-by: Florian Weimer <fweimer@redhat.com>

Thanks,
Florian