nptl: Fix abort in case of set*id failure [BZ #17135]

If a call to the set*id functions fails in a multi-threaded program,
the abort introduced in commit 13f7fe35ae2b0ea55dc4b9628763aafdc8bdc30c
was triggered.

We address by checking that all calls to set*id on all threads give
the same result, and only abort if we see success followed by failure
(or vice versa).

diff --git a/ChangeLog b/ChangeLog
index 99b87329d5bf84745a20f738ecb150dae9f224d8..7e847697b01ff0ae6bd20c1185141406278fb00b 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,15 @@
+2014-07-10  Florian Weimer  <fweimer@redhat.com>
+
+       [BZ #17135]
+       * nptl/pthreadP.h (__nptl_setxid_error): Declare function.
+       * nptl/allocatestack.c (__nptl_setxid_error): New function.
+       (__nptl_setxid): Initialize error member.  Call
+       __nptl_setxid_error.
+       * nptl/nptl-init.c (sighandler_setxid): Call __nptl_setxid_error.
+       * nptl/descr.h (struct xid_command): Add error member.
+       * nptl/tst-setuid3.c: New file.
+       * nptl/Makefile (tests): Add it.
+
 2014-07-10  Adhemerval Zanella  <azanella@linux.vnet.ibm.com>
 
        * sysdeps/unix/sysv/linux/powerpc/lowlevellock.h (__lll_base_trylock):

diff --git a/NEWS b/NEWS
index a6617a145e86144279709746c6d3635f3e2d5045..697b730ee0421385d741754a9e885fc7351280cf 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -22,7 +22,7 @@ Version 2.20
   16927, 16928, 16932, 16943, 16958, 16965, 16966, 16967, 16977, 16978,
   16984, 16990, 16996, 17009, 17022, 17031, 17042, 17048, 17050, 17058,
   17061, 17062, 17069, 17075, 17079, 17084, 17086, 17092, 17097, 17125,
-  17137.
+  17135, 17137.
 
 * Optimized strchr implementation for AArch64.  Contributed by ARM Ltd.
 

diff --git a/nptl/Makefile b/nptl/Makefile
index 67f7d5bfa90f9925f37393adff7857717389f094..ab3080e5bb3d55aba794d41a34fb0b42a6f02f66 100644 (file)
--- a/nptl/Makefile
+++ b/nptl/Makefile
@@ -271,6 +271,7 @@ tests = tst-typesizes \
        tst-abstime \
        tst-vfork1 tst-vfork2 tst-vfork1x tst-vfork2x \
        tst-getpid1 tst-getpid2 tst-getpid3 \
+       tst-setuid3 \
        tst-initializers1 $(patsubst %,tst-initializers1-%,c89 gnu89 c99 gnu99)
 xtests = tst-setuid1 tst-setuid1-static tst-setuid2 \
        tst-mutexpp1 tst-mutexpp6 tst-mutexpp10

diff --git a/nptl/allocatestack.c b/nptl/allocatestack.c
index 9095ef430d97474115fa25a3427ddd2e62616a0f..d95ffe9d3621a621ba685e91096d4163818319e4 100644 (file)
--- a/nptl/allocatestack.c
+++ b/nptl/allocatestack.c
@@ -1059,6 +1059,25 @@ setxid_signal_thread (struct xid_command *cmdp, struct pthread *t)
     return 0;
 }
 
+/* Check for consistency across set*id system call results.  The abort
+   should not happen as long as all privileges changes happen through
+   the glibc wrappers.  ERROR must be 0 (no error) or an errno
+   code.  */
+void
+attribute_hidden
+__nptl_setxid_error (struct xid_command *cmdp, int error)
+{
+  do
+    {
+      int olderror = cmdp->error;
+      if (olderror == error)
+       break;
+      if (olderror != -1)
+       /* Mismatch between current and previous results.  */
+       abort ();
+    }
+  while (atomic_compare_and_exchange_bool_acq (&cmdp->error, error, -1));
+}
 
 int
 attribute_hidden
@@ -1070,6 +1089,7 @@ __nptl_setxid (struct xid_command *cmdp)
 
   __xidcmd = cmdp;
   cmdp->cntr = 0;
+  cmdp->error = -1;
 
   struct pthread *self = THREAD_SELF;
 
@@ -1153,11 +1173,14 @@ __nptl_setxid (struct xid_command *cmdp)
   INTERNAL_SYSCALL_DECL (err);
   result = INTERNAL_SYSCALL_NCS (cmdp->syscall_no, err, 3,
                                 cmdp->id[0], cmdp->id[1], cmdp->id[2]);
-  if (INTERNAL_SYSCALL_ERROR_P (result, err))
+  int error = 0;
+  if (__glibc_unlikely (INTERNAL_SYSCALL_ERROR_P (result, err)))
     {
-      __set_errno (INTERNAL_SYSCALL_ERRNO (result, err));
+      error = INTERNAL_SYSCALL_ERRNO (result, err);
+      __set_errno (error);
       result = -1;
     }
+  __nptl_setxid_error (cmdp, error);
 
   lll_unlock (stack_cache_lock, LLL_PRIVATE);
   return result;

diff --git a/nptl/descr.h b/nptl/descr.h
index 61d57d5732f6d2f1cd1bbc5ceb7147b592d9b10b..6738591585d5d6c72de6677ab49ce597aaf26c72 100644 (file)
--- a/nptl/descr.h
+++ b/nptl/descr.h
@@ -100,6 +100,7 @@ struct xid_command
   int syscall_no;
   long int id[3];
   volatile int cntr;
+  volatile int error; /* -1: no call yet, 0: success seen, >0: error seen.  */
 };
 
 

diff --git a/nptl/nptl-init.c b/nptl/nptl-init.c
index 2796dc5182f1ed5780a6e386ab31b05a71d70052..9f7b20a90e8e05c6d9f86ff26d63ad49daa06343 100644 (file)
--- a/nptl/nptl-init.c
+++ b/nptl/nptl-init.c
@@ -248,10 +248,10 @@ sighandler_setxid (int sig, siginfo_t *si, void *ctx)
   INTERNAL_SYSCALL_DECL (err);
   result = INTERNAL_SYSCALL_NCS (__xidcmd->syscall_no, err, 3, __xidcmd->id[0],
                                 __xidcmd->id[1], __xidcmd->id[2]);
+  int error = 0;
   if (__glibc_unlikely (INTERNAL_SYSCALL_ERROR_P (result, err)))
-    /* Safety check.  This should never happen if the setxid system
-       calls are only ever called through their glibc wrappers.  */
-    abort ();
+    error = INTERNAL_SYSCALL_ERRNO (result, err);
+  __nptl_setxid_error (__xidcmd, error);
 
   /* Reset the SETXID flag.  */
   struct pthread *self = THREAD_SELF;

diff --git a/nptl/pthreadP.h b/nptl/pthreadP.h
index 197401af890a1d936f0d0333efc55b10a0105974..94e7890fa50a05dfb524d29aff6fb2257906c9bd 100644 (file)
--- a/nptl/pthreadP.h
+++ b/nptl/pthreadP.h
@@ -578,6 +578,8 @@ extern void _pthread_cleanup_pop_restore (struct _pthread_cleanup_buffer *buffer
 
 extern void __nptl_deallocate_tsd (void) attribute_hidden;
 
+extern void __nptl_setxid_error (struct xid_command *cmdp, int error)
+  attribute_hidden;
 extern int __nptl_setxid (struct xid_command *cmdp) attribute_hidden;
 #ifndef SHARED
 extern void __nptl_set_robust (struct pthread *self);

diff --git a/nptl/tst-setuid3.c b/nptl/tst-setuid3.c
new file mode 100644 (file)
index 0000000..f78f485
--- /dev/null
+++ b/nptl/tst-setuid3.c
@@ -0,0 +1,104 @@
+/* Copyright (C) 2014 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
+   <http://www.gnu.org/licenses/>.  */
+
+#include <err.h>
+#include <errno.h>
+#include <pthread.h>
+#include <stdbool.h>
+#include <unistd.h>
+
+/* The test must run under a non-privileged user ID.  */
+static const uid_t test_uid = 1;
+
+static pthread_barrier_t barrier1;
+static pthread_barrier_t barrier2;
+
+static void *
+thread_func (void *ctx __attribute__ ((unused)))
+{
+  int ret = pthread_barrier_wait (&barrier1);
+  if (ret != PTHREAD_BARRIER_SERIAL_THREAD && ret != 0)
+    errx (1, "pthread_barrier_wait (barrier1) (on thread): %d", ret);
+  ret = pthread_barrier_wait (&barrier2);
+  if (ret != PTHREAD_BARRIER_SERIAL_THREAD && ret != 0)
+    errx (1, "pthread_barrier_wait (barrier2) (on thread): %d", ret);
+  return NULL;
+}
+
+static void
+setuid_failure (int phase)
+{
+  int ret = setuid (0);
+  switch (ret)
+    {
+    case 0:
+      errx (1, "setuid succeeded unexpectedly in phase %d", phase);
+    case -1:
+      if (errno != EPERM)
+       err (1, "setuid phase %d", phase);
+      break;
+    default:
+      errx (1, "invalid setuid return value in phase %d: %d", phase, ret);
+    }
+}
+
+static int
+do_test (void)
+{
+  if (getuid () == 0)
+    if (setuid (test_uid) != 0)
+      err (1, "setuid (%u)", (unsigned) test_uid);
+  if (setuid (getuid ()))
+    err (1, "setuid (getuid ())");
+  setuid_failure (1);
+
+  int ret = pthread_barrier_init (&barrier1, NULL, 2);
+  if (ret != 0)
+    errx (1, "pthread_barrier_init (barrier1): %d", ret);
+  ret = pthread_barrier_init (&barrier2, NULL, 2);
+  if (ret != 0)
+    errx (1, "pthread_barrier_init (barrier2): %d", ret);
+
+  pthread_t thread;
+  ret = pthread_create (&thread, NULL, thread_func, NULL);
+  if (ret != 0)
+    errx (1, "pthread_create: %d", ret);
+
+  /* Ensure that the thread is running properly.  */
+  ret = pthread_barrier_wait (&barrier1);
+  if (ret != 0)
+    errx (1, "pthread_barrier_wait (barrier1): %d", ret);
+
+  setuid_failure (2);
+
+  /* Check success case. */
+  if (setuid (getuid ()) != 0)
+    err (1, "setuid (getuid ())");
+
+  /* Shutdown.  */
+  ret = pthread_barrier_wait (&barrier2);
+  if (ret != PTHREAD_BARRIER_SERIAL_THREAD && ret != 0)
+    errx (1, "pthread_barrier_wait (barrier2): %d", ret);
+
+  if (ret != PTHREAD_BARRIER_SERIAL_THREAD && ret != 0)
+    errx (1, "pthread_join: %d", ret);
+
+  return 0;
+}
+
+#define TEST_FUNCTION do_test ()
+#include "../test-skeleton.c"