diff mbox

[ping] nptl: Fix abort in case of set*id failure

Message ID 53BED0F5.2080806@redhat.com
State New
Headers show

Commit Message

Florian Weimer July 10, 2014, 5:44 p.m. UTC
On 07/09/2014 12:42 PM, Florian Weimer wrote:

> Thanks for this suggestion, applied (twice).

Here's the updated patch, tested on x86_64-redhat-linux-gnu.  Okay to 
commit?

Comments

Siddhesh Poyarekar July 11, 2014, 5:03 a.m. UTC | #1
On Thu, Jul 10, 2014 at 07:44:21PM +0200, Florian Weimer wrote:
> On 07/09/2014 12:42 PM, Florian Weimer wrote:
> 
> >Thanks for this suggestion, applied (twice).
> 
> Here's the updated patch, tested on x86_64-redhat-linux-gnu.  Okay to
> commit?

Looks good to me.

Siddhesh

> 
> -- 
> Florian Weimer / Red Hat Product Security

> 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.
> 
> diff --git a/NEWS b/NEWS
> index a6617a1..697b730 100644
> --- 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 67f7d5b..ab3080e 100644
> --- 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 9095ef4..d95ffe9 100644
> --- 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 61d57d5..6738591 100644
> --- 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 2796dc5..9f7b20a 100644
> --- 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 197401a..94e7890 100644
> --- 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
> 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"
> -- 
> 1.9.3
>
diff mbox

Patch

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.

diff --git a/NEWS b/NEWS
index a6617a1..697b730 100644
--- 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 67f7d5b..ab3080e 100644
--- 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 9095ef4..d95ffe9 100644
--- 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 61d57d5..6738591 100644
--- 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 2796dc5..9f7b20a 100644
--- 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 197401a..94e7890 100644
--- 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
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"
-- 
1.9.3