[src/trunk]: src - In uvm_voaddr_acquire(), take an extra hold on the anon lo...

[thorpej <thorpej%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/f0de42ae79de
branches:  trunk
changeset: 931755:f0de42ae79de
user:      thorpej <thorpej%NetBSD.org@localhost>
date:      Thu Apr 30 04:18:07 2020 +0000

description:
- 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.

diffstat:

 sys/uvm/uvm_map.c                |  17 ++++----
 tests/lib/libc/sys/t_futex_ops.c |  78 ++++++++++++++++++++++++++++++++++++++-
 2 files changed, 85 insertions(+), 10 deletions(-)

diffs (175 lines):

diff -r fc43a5c76c57 -r f0de42ae79de sys/uvm/uvm_map.c
--- a/sys/uvm/uvm_map.c Thu Apr 30 03:42:23 2020 +0000
+++ b/sys/uvm/uvm_map.c Thu Apr 30 04:18:07 2020 +0000
@@ -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 <sys/cdefs.h>
-__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 @@
                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 @@
            }
        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 -r fc43a5c76c57 -r f0de42ae79de tests/lib/libc/sys/t_futex_ops.c
--- a/tests/lib/libc/sys/t_futex_ops.c  Thu Apr 30 03:42:23 2020 +0000
+++ b/tests/lib/libc/sys/t_futex_ops.c  Thu Apr 30 04:18:07 2020 +0000
@@ -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 <sys/cdefs.h>
 __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 <sys/fcntl.h>
 #include <sys/mman.h>
@@ -39,6 +39,7 @@
 #include <lwp.h>
 #include <stdlib.h>
 #include <stdio.h>
+#include <signal.h>
 #include <time.h>
 #include <limits.h>
 #include <unistd.h>
@@ -166,12 +167,15 @@
 
        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 @@
 
 /*****************************************************************************/
 
+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_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);