From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932123AbdCTXqL (ORCPT ); Mon, 20 Mar 2017 19:46:11 -0400 Received: from mail-pg0-f49.google.com ([74.125.83.49]:36657 "EHLO mail-pg0-f49.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753377AbdCTXqI (ORCPT ); Mon, 20 Mar 2017 19:46:08 -0400 Date: Mon, 20 Mar 2017 16:46:05 -0700 From: Kees Cook To: Shuah Khan Cc: Paul Lawrence , Nick Kralevich , Andy Lutomirski , Will Drewry , linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH] seccomp: Adjust selftests to avoid double-join Message-ID: <20170320234605.GA140256@beast> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org While glibc's pthread implementation is rather forgiving about repeat thread joining, Bionic has recently become much more strict. To deal with this, actually track which threads have been successfully joined and kill the rest at teardown. Based on a patch from Paul Lawrence. Signed-off-by: Kees Cook Cc: Paul Lawrence --- tools/testing/selftests/seccomp/seccomp_bpf.c | 51 ++++++++++++++++++--------- 1 file changed, 34 insertions(+), 17 deletions(-) diff --git a/tools/testing/selftests/seccomp/seccomp_bpf.c b/tools/testing/selftests/seccomp/seccomp_bpf.c index 03f1fa495d74..00a928b833d0 100644 --- a/tools/testing/selftests/seccomp/seccomp_bpf.c +++ b/tools/testing/selftests/seccomp/seccomp_bpf.c @@ -1822,6 +1822,23 @@ struct tsync_sibling { struct __test_metadata *metadata; }; +/* + * To avoid joining joined threads (which is not allowed by Bionic), + * make sure we both successfully join and clear the tid to skip a + * later join attempt during fixture teardown. Any remaining threads + * will be directly killed during teardown. + */ +#define PTHREAD_JOIN(tid, status) \ + do { \ + int _rc = pthread_join(tid, status); \ + if (_rc) { \ + TH_LOG("pthread_join of tid %u failed: %d\n", \ + (unsigned int)tid, _rc); \ + } else { \ + tid = 0; \ + } \ + } while (0) + FIXTURE_DATA(TSYNC) { struct sock_fprog root_prog, apply_prog; struct tsync_sibling sibling[TSYNC_SIBLINGS]; @@ -1890,14 +1907,14 @@ FIXTURE_TEARDOWN(TSYNC) for ( ; sib < self->sibling_count; ++sib) { struct tsync_sibling *s = &self->sibling[sib]; - void *status; if (!s->tid) continue; - if (pthread_kill(s->tid, 0)) { - pthread_cancel(s->tid); - pthread_join(s->tid, &status); - } + /* + * If a thread is still running, it may be stuck, so hit + * it over the head really hard. + */ + pthread_kill(s->tid, 9); } pthread_mutex_destroy(&self->mutex); pthread_cond_destroy(&self->cond); @@ -1987,9 +2004,9 @@ TEST_F(TSYNC, siblings_fail_prctl) pthread_mutex_unlock(&self->mutex); /* Ensure diverging sibling failed to call prctl. */ - pthread_join(self->sibling[0].tid, &status); + PTHREAD_JOIN(self->sibling[0].tid, &status); EXPECT_EQ(SIBLING_EXIT_FAILURE, (long)status); - pthread_join(self->sibling[1].tid, &status); + PTHREAD_JOIN(self->sibling[1].tid, &status); EXPECT_EQ(SIBLING_EXIT_UNKILLED, (long)status); } @@ -2029,9 +2046,9 @@ TEST_F(TSYNC, two_siblings_with_ancestor) } pthread_mutex_unlock(&self->mutex); /* Ensure they are both killed and don't exit cleanly. */ - pthread_join(self->sibling[0].tid, &status); + PTHREAD_JOIN(self->sibling[0].tid, &status); EXPECT_EQ(0x0, (long)status); - pthread_join(self->sibling[1].tid, &status); + PTHREAD_JOIN(self->sibling[1].tid, &status); EXPECT_EQ(0x0, (long)status); } @@ -2055,9 +2072,9 @@ TEST_F(TSYNC, two_sibling_want_nnp) pthread_mutex_unlock(&self->mutex); /* Ensure they are both upset about lacking nnp. */ - pthread_join(self->sibling[0].tid, &status); + PTHREAD_JOIN(self->sibling[0].tid, &status); EXPECT_EQ(SIBLING_EXIT_NEWPRIVS, (long)status); - pthread_join(self->sibling[1].tid, &status); + PTHREAD_JOIN(self->sibling[1].tid, &status); EXPECT_EQ(SIBLING_EXIT_NEWPRIVS, (long)status); } @@ -2095,9 +2112,9 @@ TEST_F(TSYNC, two_siblings_with_no_filter) pthread_mutex_unlock(&self->mutex); /* Ensure they are both killed and don't exit cleanly. */ - pthread_join(self->sibling[0].tid, &status); + PTHREAD_JOIN(self->sibling[0].tid, &status); EXPECT_EQ(0x0, (long)status); - pthread_join(self->sibling[1].tid, &status); + PTHREAD_JOIN(self->sibling[1].tid, &status); EXPECT_EQ(0x0, (long)status); } @@ -2140,9 +2157,9 @@ TEST_F(TSYNC, two_siblings_with_one_divergence) pthread_mutex_unlock(&self->mutex); /* Ensure they are both unkilled. */ - pthread_join(self->sibling[0].tid, &status); + PTHREAD_JOIN(self->sibling[0].tid, &status); EXPECT_EQ(SIBLING_EXIT_UNKILLED, (long)status); - pthread_join(self->sibling[1].tid, &status); + PTHREAD_JOIN(self->sibling[1].tid, &status); EXPECT_EQ(SIBLING_EXIT_UNKILLED, (long)status); } @@ -2199,7 +2216,7 @@ TEST_F(TSYNC, two_siblings_not_under_filter) TH_LOG("cond broadcast non-zero"); } pthread_mutex_unlock(&self->mutex); - pthread_join(self->sibling[sib].tid, &status); + PTHREAD_JOIN(self->sibling[sib].tid, &status); EXPECT_EQ(SIBLING_EXIT_UNKILLED, (long)status); /* Poll for actual task death. pthread_join doesn't guarantee it. */ while (!kill(self->sibling[sib].system_tid, 0)) @@ -2224,7 +2241,7 @@ TEST_F(TSYNC, two_siblings_not_under_filter) TH_LOG("cond broadcast non-zero"); } pthread_mutex_unlock(&self->mutex); - pthread_join(self->sibling[sib].tid, &status); + PTHREAD_JOIN(self->sibling[sib].tid, &status); EXPECT_EQ(0, (long)status); /* Poll for actual task death. pthread_join doesn't guarantee it. */ while (!kill(self->sibling[sib].system_tid, 0)) -- 2.7.4 -- Kees Cook Pixel Security