Re: [PATCH] nptl: Retry SIGSETXID delivery on EAGAIN in setxid (bug 21108)

Ping (x2) with Andreas typos fixes.

On 01/07/26 14:33, Adhemerval Zanella Netto wrote:
> Ping.
> 
> On 09/06/26 17:14, Adhemerval Zanella wrote:
>> setxid_signal_thread assumed tgkill could only fail if the target
>> thread had not started yet or had already exited, and treated any error
>> as "thread is gone".  But SIGSETXID is a realtime signal, and for those
>> the tgkill may fail with with EAGAIN once the per-real-user
>> RLIMIT_SIGPENDING limit is reached (process-directed kill uses the
>> guaranteed delivery override, but a thread-directed tgkill cannot.)
>>
>> When that happened, the thread never ran the syscall, yet __nptl_setxid
>> returned success.  A multithreaded process dropping privileges with
>> 'setuid' could therefore silently leave one or more threads with the old
>> credentials (a potential security issue).
>>
>> Distinguish EAGAIN from a vanished thread and retry delivery, with a
>> bounded exponential backoff, until tgkill succeeds.  If the queue never
>> drains the call blocks indefinitely, which is the intended behaviour:
>> hanging is safer than continuing with threads that wrongly retain
>> privileges.
>>
>> The retry uses a sleeping backoff rather than a busy tgkill loop.  Each
>> successfully-sent SIGSETXID holds an RLIMIT_SIGPENDING slot until its
>> target thread runs the handler, so forward progress requires letting
>> those threads run: sleeping yields the CPU, whereas a busy loop merely
>> relies on the scheduler preempting the spinner.
>>
>> Under thread priorities (SCHED_FIFO) or CPU pressure that might becomes a
>> deadlock -- if the setxid caller outranks the thread whose handler must
>> free a slot, a busy loop spins forever.  A sleep also avoids pinning a
>> core at 100% in the intentional block-forever case when the queue is
>> exhausted by a source that never drains.
>>
>> The new test reproduces the bug without any privileges by checking
>> signal delivery rather than the credential outcome: with the queue full,
>> a no-op setresuid still broadcasts SIGSETXID and, once fixed, must block
>> until the queue drains instead of returning immediately.
>>
>> Tested on aarch64-linux-gnu, x86_64-linux-gnu, and i686-linux-gnu.
>> I also manually tests the xfail tests on nptl.
>> ---
>>  nptl/Makefile            |   1 +
>>  nptl/nptl_setxid.c       | 100 +++++++++++++++++++++++----
>>  nptl/tst-setuid-eagain.c | 144 +++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 233 insertions(+), 12 deletions(-)
>>  create mode 100644 nptl/tst-setuid-eagain.c
>>
>> diff --git a/nptl/Makefile b/nptl/Makefile
>> index 02862d1c04b..29db8dae505 100644
>> --- a/nptl/Makefile
>> +++ b/nptl/Makefile
>> @@ -350,6 +350,7 @@ tests = \
>>    tst-rwlock22 \
>>    tst-sched1 \
>>    tst-sem17 \
>> +  tst-setuid-eagain \
>>    tst-signal3 \
>>    tst-stack2 \
>>    tst-stack3 \
>> diff --git a/nptl/nptl_setxid.c b/nptl/nptl_setxid.c
>> index 2214b325ddf..2ee83e80631 100644
>> --- a/nptl/nptl_setxid.c
>> +++ b/nptl/nptl_setxid.c
>> @@ -150,31 +150,86 @@ setxid_unmark_thread (struct xid_command *cmdp, struct pthread *t)
>>  }
>>  
>>  
>> -static int
>> +enum setxid_signal_state
>> +{
>> +  setxid_signal_done,  /* Thread  has not finished starting or has already
>> +			  exited; ignore it.  */
>> +  setxid_signal_sent,  /* The signal was delivered and the thread signal
>> +			  handler will run (the xid_command::crnt was already
>> +			  incremented.  */
>> +  setxid_signal_retry, /* tgkill returned EAGAIN (the signal queue
>> +			  RLIMIT_SIGPENDING is full), the thread is still
>> +			  running and must be signal.  The caller has to
>> +			  retry.  */
>> +};
>> +
>> +static enum setxid_signal_state
>>  setxid_signal_thread (struct xid_command *cmdp, struct pthread *t)
>>  {
>>    if ((t->cancelhandling & SETXID_BITMASK) == 0)
>> -    return 0;
>> +    return setxid_signal_done;
>>  
>>    int val;
>>    pid_t pid = __getpid ();
>>    val = INTERNAL_SYSCALL_CALL (tgkill, pid, t->tid, SIGSETXID);
>>  
>> -  /* If this failed, it must have had not started yet or else exited.  */
>>    if (!INTERNAL_SYSCALL_ERROR_P (val))
>>      {
>>        atomic_fetch_add_relaxed (&cmdp->cntr, 1);
>> -      return 1;
>> +      return setxid_signal_sent;
>>      }
>> -  else
>> -    return 0;
>> +
>> +  /* The kernel might return EAGAIN for realtime signals if the signal queue
>> +     reaches its limits (RLIMIT_SIGPENDING) because tgkill does not use the
>> +     guaranteed-delivery override path as the kill syscall.  The threads is
>> +     still running and *must* process the credential change, so instruc the
>> +     caller to retry.  Any other error means the thread has not started yet
>> +     or has already exited (and can be ignored).  */
>> +  if (INTERNAL_SYSCALL_ERRNO (val) == EAGAIN)
>> +    return setxid_signal_retry;
>> +
>> +  return setxid_signal_done;
>> +}
>> +
>> +enum
>> +{
>> +  setxid_backoff_min_ns = 1000L,     /* 1 us first retry.  */
>> +  setxid_backoff_max_ns = 65536000L  /* ~65 ms ceiling.  */
>> +};
>> +/* Make sure the maximum backoff sleep is within a resonable maximum value
>> +   of 100 ms.  It should always fit a 32-bit timespec.  */
>> +verify (setxid_backoff_min_ns > 0
>> +	&& setxid_backoff_min_ns < setxid_backoff_max_ns
>> +	&& setxid_backoff_max_ns < 100000000L);
>> +
>> +/* Sleep for *NS nanoseconds, then grow the delay towards the ceiling.  */
>> +static void
>> +setxid_signal_backoff (long int *ns)
>> +{
>> +  /* We can not use __clock_nanosleep because it is a cancellable entrypoint,
>> +     and we want to not touch errno.   CLOCK_MONOTONIC with flags == 0
>> +     requests a *relative* sleep against a clock that never steps backwards,
>> +     which is what a retry delay wants.  */
>> +#ifdef __ASSUME_TIME64_SYSCALLS
>> +# ifndef __NR_clock_nanosleep_time64
>> +#  define __NR_clock_nanosleep_time64 __NR_clock_nanosleep
>> +# endif
>> +  struct __timespec64 ts = { .tv_sec = 0, .tv_nsec = *ns };
>> +  INTERNAL_SYSCALL_CALL (clock_nanosleep_time64, CLOCK_MONOTONIC, 0, &ts,
>> +			 NULL);
>> +#else
>> +  /* The backoff timer should always fit in 32 bit timespec.  */
>> +  struct timespec ts = { .tv_sec = 0, .tv_nsec = *ns };
>> +  INTERNAL_SYSCALL_CALL (clock_nanosleep, CLOCK_MONOTONIC, 0, &ts, NULL);
>> +#endif
>> +
>> +  *ns = *ns >= setxid_backoff_max_ns / 2 ? setxid_backoff_max_ns : *ns * 2;
>>  }
>>  
>>  int
>> -attribute_hidden
>>  __nptl_setxid (struct xid_command *cmdp)
>>  {
>> -  int signalled;
>> +  bool signalled;
>>    int result;
>>    lll_lock (GL (dl_stack_cache_lock), LLL_PRIVATE);
>>  
>> @@ -208,9 +263,11 @@ __nptl_setxid (struct xid_command *cmdp)
>>    /* Iterate until we don't succeed in signalling anyone.  That means
>>       we have gotten all running threads, and their children will be
>>       automatically correct once started.  */
>> +  long int backoff_ns = setxid_backoff_min_ns;
>>    do
>>      {
>> -      signalled = 0;
>> +      signalled = false;
>> +      bool retry = false;
>>  
>>        list_for_each (runp, &GL (dl_stack_used))
>>          {
>> @@ -218,7 +275,12 @@ __nptl_setxid (struct xid_command *cmdp)
>>            if (t == self)
>>              continue;
>>  
>> -          signalled += setxid_signal_thread (cmdp, t);
>> +          switch (setxid_signal_thread (cmdp, t))
>> +            {
>> +            case setxid_signal_sent:  signalled = true; break;
>> +            case setxid_signal_retry: retry = true;     break;
>> +            case setxid_signal_done:  break;
>> +            }
>>          }
>>  
>>        list_for_each (runp, &GL (dl_stack_user))
>> @@ -227,7 +289,12 @@ __nptl_setxid (struct xid_command *cmdp)
>>            if (t == self)
>>              continue;
>>  
>> -          signalled += setxid_signal_thread (cmdp, t);
>> +          switch (setxid_signal_thread (cmdp, t))
>> +            {
>> +            case setxid_signal_sent:  signalled = true; break;
>> +            case setxid_signal_retry: retry = true;     break;
>> +            case setxid_signal_done:  break;
>> +            }
>>          }
>>  
>>        int cur = cmdp->cntr;
>> @@ -237,8 +304,17 @@ __nptl_setxid (struct xid_command *cmdp)
>>                               FUTEX_PRIVATE);
>>            cur = cmdp->cntr;
>>          }
>> +
>> +      if (retry)
>> +        {
>> +	  /* Blocking here until delivery eventually succeeds is intentional
>> +	     and safer than returning while a thread still holds the old
>> +	     credentials.  */
>> +          setxid_signal_backoff (&backoff_ns);
>> +          signalled = true;
>> +        }
>>      }
>> -  while (signalled != 0);
>> +  while (signalled);
>>  
>>    /* Clean up flags, so that no thread blocks during exit waiting
>>       for a signal which will never come.  */
>> diff --git a/nptl/tst-setuid-eagain.c b/nptl/tst-setuid-eagain.c
>> new file mode 100644
>> index 00000000000..942091bbbb8
>> --- /dev/null
>> +++ b/nptl/tst-setuid-eagain.c
>> @@ -0,0 +1,144 @@
>> +/* Test that a multithreaded setxid does not silently give up when the
>> +   realtime signal queue is exhausted (bug 21108).
>> +   Copyright (C) 2026 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/>.  */
>> +
>> +/* The sexid is implemented on Linux with a SIGSETXID broadcast, and tgkill
>> +   might fail with EAGAIN if the signal queue is full (RLIMIT_SIGPENDING)
>> +   because SIGSETXID is a realtime signal.
>> +
>> +   The test first fills the queue, run the setresuid in a helper thread, and
>> +   verifiers that it does *not* complete while the queue is still full.
>> +
>> +   The test checks whether the signal delivery works as intended, so there's
>> +   no need to change privileges.  A no-op setresuid still broadcasts SIGSETXID
>> +   to the other threads, and with a full queue, the call may only return once
>> +   the queue drains.  */
>> +
>> +#include <errno.h>
>> +#include <pthread.h>
>> +#include <semaphore.h>
>> +#include <signal.h>
>> +#include <stdlib.h>
>> +#include <sys/resource.h>
>> +#include <time.h>
>> +#include <unistd.h>
>> +
>> +#include <support/check.h>
>> +#include <support/xthread.h>
>> +
>> +/* How long to wait for the (no-op) setxid to complete before concluding it
>> +   is correctly blocking on the full queue.  It should not return until the
>> +   queue is drained.  */
>> +#define BLOCK_WAIT_SECONDS 2
>> +
>> +static pthread_barrier_t start_barrier;
>> +static sem_t setxid_done;
>> +static sem_t worker_exit;
>> +
>> +/* A plain thread that must be signalled by the setxid broadcast.  It simply
>> +   parks, remaining in the thread list so it is a delivery target.  */
>> +static void *
>> +worker_thread (void *closure)
>> +{
>> +  xpthread_barrier_wait (&start_barrier);
>> +  while (sem_wait (&worker_exit) != 0)
>> +    /* Restart if interrupted by the SIGSETXID handler.  */;
>> +  return NULL;
>> +}
>> +
>> +/* setresuid to the current ids is a no-op but still triggers the full
>> +   SIGSETXID broadcast to the other threads.  */
>> +static void *
>> +setxid_thread (void *closure)
>> +{
>> +  xpthread_barrier_wait (&start_barrier);
>> +  uid_t uid = getuid ();
>> +  TEST_VERIFY_EXIT (setresuid (uid, uid, uid) == 0);
>> +  TEST_VERIFY_EXIT (sem_post (&setxid_done) == 0);
>> +  return NULL;
>> +}
>> +
>> +static int
>> +do_test (void)
>> +{
>> +  TEST_COMPARE (sem_init (&setxid_done, 0, 0), 0);
>> +  TEST_COMPARE (sem_init (&worker_exit, 0, 0), 0);
>> +  xpthread_barrier_init (&start_barrier, NULL, 3);
>> +
>> +  sigset_t set;
>> +  sigemptyset (&set);
>> +  sigaddset (&set, SIGRTMIN);
>> +  TEST_COMPARE (pthread_sigmask (SIG_BLOCK, &set, NULL), 0);
>> +
>> +  pthread_t worker = xpthread_create (NULL, worker_thread, NULL);
>> +  pthread_t setxid = xpthread_create (NULL, setxid_thread, NULL);
>> +
>> +  /* Lower the pending-signal limit, and fill the realtime signal queue until
>> +     it reaches RLIMIT_SIGPENDING.  */
>> +  {
>> +    struct rlimit rl;
>> +    TEST_COMPARE (getrlimit (RLIMIT_SIGPENDING, &rl), 0);
>> +    rl.rlim_cur = 64;
>> +    TEST_COMPARE (setrlimit (RLIMIT_SIGPENDING, &rl), 0);
>> +  }
>> +
>> +  {
>> +    const union sigval val = { .sival_int = 0 };
>> +    while (sigqueue (getpid (), SIGRTMIN, val) == 0)
>> +      continue;
>> +    TEST_COMPARE (errno, EAGAIN);
>> +  }
>> +
>> +  xpthread_barrier_wait (&start_barrier);
>> +
>> +  struct timespec ts;
>> +  TEST_COMPARE (clock_gettime (CLOCK_REALTIME, &ts), 0);
>> +  ts.tv_sec += BLOCK_WAIT_SECONDS;
>> +  int r = sem_timedwait (&setxid_done, &ts);
>> +
>> +  if (r == 0)
>> +    /* The setxid completed while the queue was still full: the broadcast
>> +       silently gave up instead of delivering SIGSETXID (bug 21108).  */
>> +    FAIL_EXIT1 ("setxid returned while the signal queue was full");
>> +  TEST_COMPARE (errno, ETIMEDOUT);
>> +
>> +  /* Now drain the queue so the (correctly blocking) setxid can finish.  */
>> +  while (1)
>> +    {
>> +      struct timespec zero = { 0, 0 };
>> +      if (sigtimedwait (&set, NULL, &zero) < 0)
>> +	{
>> +	  if (errno == EINTR)
>> +	    continue;
>> +	  break;
>> +	}
>> +    }
>> +
>> +  /* With the queue drained, the setxid must complete.  */
>> +  while (sem_wait (&setxid_done) != 0)
>> +    /* Restart if interrupted.  */;
>> +
>> +  /* Release the worker and clean up.  */
>> +  TEST_COMPARE (sem_post (&worker_exit), 0);
>> +  xpthread_join (worker);
>> +  xpthread_join (setxid);
>> +  return 0;
>> +}
>> +
>> +#define TIMEOUT 10
>> +#include <support/test-driver.c>
>