Skip to content

Commit

Permalink
seccomp: Adjust selftests to avoid double-join
Browse files Browse the repository at this point in the history
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.

Cc: Paul Lawrence <paullawrence@google.com>
Signed-off-by: Kees Cook <keescook@chromium.org>
  • Loading branch information
Kees Cook committed Jun 26, 2017
1 parent 131b635 commit 93bd70e
Showing 1 changed file with 34 additions and 17 deletions.
51 changes: 34 additions & 17 deletions tools/testing/selftests/seccomp/seccomp_bpf.c
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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);
}

Expand All @@ -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);
}

Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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))
Expand All @@ -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))
Expand Down

0 comments on commit 93bd70e

Please sign in to comment.