From patchwork Wed Jul 2 12:18:06 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Florian Weimer X-Patchwork-Id: 366347 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id DFCB014003E for ; Wed, 2 Jul 2014 22:18:28 +1000 (EST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:message-id:date:from:mime-version:to:subject :references:in-reply-to:content-type; q=dns; s=default; b=tsxUK+ IYzCHfjsWKeY58dHFPeybDKrzuwMn+eEn4uF/S+RVIHATGJvZrKXJCkentmMMoyC nm8sK0Rvv5aBWZnUbZdLrNR+F6wQ01U9atTDBARYEmKcvC9AORp8v5qxzCJzXlPu Gjpb8GAc0e6bHQicS0MpnOXoAXebEF5eI2pq0= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:message-id:date:from:mime-version:to:subject :references:in-reply-to:content-type; s=default; bh=2iJKQ8pKsh9G XPfGEB5vEt8g6Fo=; b=GufHm8xTDFyik8aQiWDIrKQZva8RcVKg9FXLK9gnJV2L +WGu1qDSfDTIq0m1sQutddP6dsiXxI/IZWpc7U+DJhfQnERoT5p9BNaqRcWT9o3D ed0UdDrTt8IG1PDiwxN2HydMrx0hBLJPlDMlibZh2h6H+TtxyVqONi3inJaHm0E= Received: (qmail 24614 invoked by alias); 2 Jul 2014 12:18:21 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 24558 invoked by uid 89); 2 Jul 2014 12:18:16 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.4 required=5.0 tests=AWL, BAYES_00, SPF_HELO_PASS, SPF_PASS, T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Message-ID: <53B3F87E.5090507@redhat.com> Date: Wed, 02 Jul 2014 14:18:06 +0200 From: Florian Weimer User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.6.0 MIME-Version: 1.0 To: libc-alpha@sourceware.org Subject: [PATCH] [ping] nptl: Fix abort in case of set*id failure References: <20140428120545.20B0643994596@oldenburg.str.redhat.com> In-Reply-To: <20140428120545.20B0643994596@oldenburg.str.redhat.com> 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). 2014-07-02 Florian Weimer * 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/nptl/Makefile b/nptl/Makefile index cd3be12..34d14c7 100644 --- a/nptl/Makefile +++ b/nptl/Makefile @@ -269,6 +269,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..72a898a 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; @@ -1155,9 +1175,13 @@ __nptl_setxid (struct xid_command *cmdp) cmdp->id[0], cmdp->id[1], cmdp->id[2]); if (INTERNAL_SYSCALL_ERROR_P (result, err)) { - __set_errno (INTERNAL_SYSCALL_ERRNO (result, err)); + int error = INTERNAL_SYSCALL_ERRNO (result, err); + __nptl_setxid_error (cmdp, error); + __set_errno (error); result = -1; } + else + __nptl_setxid_error (cmdp, 0); 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..86d9d77 100644 --- a/nptl/nptl-init.c +++ b/nptl/nptl-init.c @@ -249,9 +249,12 @@ sighandler_setxid (int sig, siginfo_t *si, void *ctx) result = INTERNAL_SYSCALL_NCS (__xidcmd->syscall_no, err, 3, __xidcmd->id[0], __xidcmd->id[1], __xidcmd->id[2]); 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 (); + { + int error = INTERNAL_SYSCALL_ERRNO (result, err); + __nptl_setxid_error (__xidcmd, error); + } + else + __nptl_setxid_error (__xidcmd, 0); /* 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 + . */ + +#include +#include +#include +#include +#include + +/* 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"