From 1a07681a27cef6cc7f02e58cb677bb9f1ea20ec1 Mon Sep 17 00:00:00 2001 From: thorpej Date: Thu, 30 Apr 2020 04:18:07 +0000 Subject: [PATCH] - In uvm_voaddr_acquire(), take an extra hold on the anon lock obj. - In uvm_voaddr_release(), if the anon ref count drops to 0, call uvm_anfree() rather than uvm_anon_release(). Unconditionally drop the anon lock, and release the extra hold on the anon lock obj. Fixes a panic that occurs if the backing store for a futex backed by an anon memory location is unmapped while a thread is waiting in the futex. Add a test case that reproduced the panic to verify that it's fixed. --- sys/uvm/uvm_map.c | 17 +++---- tests/lib/libc/sys/t_futex_ops.c | 78 +++++++++++++++++++++++++++++++- 2 files changed, 85 insertions(+), 10 deletions(-) diff --git a/sys/uvm/uvm_map.c b/sys/uvm/uvm_map.c index 5adbc5f64dae..a8e5882b4b1d 100644 --- a/sys/uvm/uvm_map.c +++ b/sys/uvm/uvm_map.c @@ -1,4 +1,4 @@ -/* $NetBSD: uvm_map.c,v 1.381 2020/04/19 08:59:53 skrll Exp $ */ +/* $NetBSD: uvm_map.c,v 1.382 2020/04/30 04:18:07 thorpej Exp $ */ /* * Copyright (c) 1997 Charles D. Cranor and Washington University. @@ -66,7 +66,7 @@ */ #include -__KERNEL_RCSID(0, "$NetBSD: uvm_map.c,v 1.381 2020/04/19 08:59:53 skrll Exp $"); +__KERNEL_RCSID(0, "$NetBSD: uvm_map.c,v 1.382 2020/04/30 04:18:07 thorpej Exp $"); #include "opt_ddb.h" #include "opt_pax.h" @@ -4934,6 +4934,7 @@ uvm_voaddr_acquire(struct vm_map * const map, vaddr_t const va, if (anon) { found_anon: KASSERT(anon->an_lock == entry->aref.ar_amap->am_lock); anon->an_ref++; + rw_obj_hold(anon->an_lock); KASSERT(anon->an_ref != 0); voaddr->type = UVM_VOADDR_TYPE_ANON; voaddr->anon = anon; @@ -4987,16 +4988,16 @@ uvm_voaddr_release(struct uvm_voaddr * const voaddr) } case UVM_VOADDR_TYPE_ANON: { struct vm_anon * const anon = voaddr->anon; + krwlock_t *lock; KASSERT(anon != NULL); - rw_enter(anon->an_lock, RW_WRITER); + rw_enter((lock = anon->an_lock), RW_WRITER); KASSERT(anon->an_ref > 0); - anon->an_ref--; - if (anon->an_ref == 0) { - uvm_anon_release(anon); - } else { - rw_exit(anon->an_lock); + if (--anon->an_ref == 0) { + uvm_anfree(anon); } + rw_exit(lock); + rw_obj_free(lock); break; } default: diff --git a/tests/lib/libc/sys/t_futex_ops.c b/tests/lib/libc/sys/t_futex_ops.c index 420350e0b792..ebc97c544822 100644 --- a/tests/lib/libc/sys/t_futex_ops.c +++ b/tests/lib/libc/sys/t_futex_ops.c @@ -1,4 +1,4 @@ -/* $NetBSD: t_futex_ops.c,v 1.2 2020/04/28 17:27:03 riastradh Exp $ */ +/* $NetBSD: t_futex_ops.c,v 1.3 2020/04/30 04:18:07 thorpej Exp $ */ /*- * Copyright (c) 2019, 2020 The NetBSD Foundation, Inc. @@ -29,7 +29,7 @@ #include __COPYRIGHT("@(#) Copyright (c) 2019, 2020\ The NetBSD Foundation, inc. All rights reserved."); -__RCSID("$NetBSD: t_futex_ops.c,v 1.2 2020/04/28 17:27:03 riastradh Exp $"); +__RCSID("$NetBSD: t_futex_ops.c,v 1.3 2020/04/30 04:18:07 thorpej Exp $"); #include #include @@ -39,6 +39,7 @@ __RCSID("$NetBSD: t_futex_ops.c,v 1.2 2020/04/28 17:27:03 riastradh Exp $"); #include #include #include +#include #include #include #include @@ -166,12 +167,15 @@ simple_test_waiter_lwp(void *arg) d->threadid = _lwp_self(); + membar_producer(); atomic_inc_uint(&nlwps_running); membar_sync(); if (__futex(d->futex_ptr, d->wait_op | d->op_flags, d->block_val, NULL, NULL, 0, d->bitset) == -1) { d->futex_error = errno; + membar_sync(); + atomic_dec_uint(&nlwps_running); _lwp_exit(); } else { d->futex_error = 0; @@ -1262,6 +1266,74 @@ ATF_TC_BODY(futex_wait_timeout_deadline_rt, tc) /*****************************************************************************/ +static void +sig_noop(int sig __unused) +{ +} + +static void (*old_act)(int) = SIG_DFL; + +static void +do_futex_wait_evil_unmapped(int map_flags) +{ + int i; + + create_bs(map_flags); + + old_act = signal(SIGUSR1, sig_noop); + ATF_REQUIRE(old_act != SIG_ERR); + + setup_lwp_context(&lwp_data[0], simple_test_waiter_lwp); + lwp_data[0].op_flags = 0; + lwp_data[0].futex_error = -1; + lwp_data[0].futex_ptr = &bs_addr[0]; + lwp_data[0].block_val = 0; + lwp_data[0].bitset = 0; + lwp_data[0].wait_op = FUTEX_WAIT; + ATF_REQUIRE(_lwp_create(&lwp_data[0].context, 0, + &lwp_data[0].lwpid) == 0); + + for (i = 0; i < 5; i++) { + membar_sync(); + if (nlwps_running == 1) + break; + sleep(1); + } + membar_sync(); + ATF_REQUIRE_EQ_MSG(nlwps_running, 1, "waiters failed to start"); + + /* Ensure it's blocked. */ + ATF_REQUIRE(lwp_data[0].futex_error == -1); + + /* Rudely unmap the backing store. */ + cleanup_bs(); + + /* Signal the waiter so that it leaves the futex. */ + ATF_REQUIRE(_lwp_kill(lwp_data[0].threadid, SIGUSR1) == 0); + + /* Yay! No panic! */ + + reap_lwp_waiter(&lwp_data[0]); +} + +ATF_TC_WITH_CLEANUP(futex_wait_evil_unmapped_anon); +ATF_TC_HEAD(futex_wait_evil_unmapped_anon, tc) +{ + atf_tc_set_md_var(tc, "descr", + "tests futex WAIT while futex is unmapped - anon memory"); +} +ATF_TC_BODY(futex_wait_evil_unmapped_anon, tc) +{ + do_futex_wait_evil_unmapped(MAP_ANON); +} +ATF_TC_CLEANUP(futex_wait_evil_unmapped_anon, tc) +{ + signal(SIGUSR1, old_act); + do_cleanup(); +} + +/*****************************************************************************/ + ATF_TP_ADD_TCS(tp) { ATF_TP_ADD_TC(tp, futex_basic_wait_wake_private); @@ -1284,6 +1356,8 @@ ATF_TP_ADD_TCS(tp) ATF_TP_ADD_TC(tp, futex_wait_timeout_deadline); ATF_TP_ADD_TC(tp, futex_wait_timeout_deadline_rt); + ATF_TP_ADD_TC(tp, futex_wait_evil_unmapped_anon); + ATF_TP_ADD_TC(tp, futex_requeue); ATF_TP_ADD_TC(tp, futex_cmp_requeue);