[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-Id: <20220916135953.1320601-1-keescook@chromium.org>
Date: Fri, 16 Sep 2022 06:59:51 -0700
From: Kees Cook <keescook@...omium.org>
To: Matthew Wilcox <willy@...radead.org>
Cc: Kees Cook <keescook@...omium.org>,
Uladzislau Rezki <urezki@...il.com>,
Andrew Morton <akpm@...ux-foundation.org>,
Yu Zhao <yuzhao@...gle.com>, dev@...-flo.net,
Peter Zijlstra <peterz@...radead.org>,
Ingo Molnar <mingo@...hat.com>, linux-kernel@...r.kernel.org,
x86@...nel.org, linux-perf-users@...r.kernel.org,
linux-mm@...ck.org, linux-hardening@...r.kernel.org,
linux-arch@...r.kernel.org
Subject: [PATCH 0/3] x86/dumpstack: Inline copy_from_user_nmi()
Hi,
This fixes a find_vmap_area() deadlock. The main fix is patch 2, repeated here:
The check_object_size() helper under CONFIG_HARDENED_USERCOPY is
designed to skip any checks where the length is known at compile time as
a reasonable heuristic to avoid "likely known-good" cases. However, it can
only do this when the copy_*_user() helpers are, themselves, inline too.
Using find_vmap_area() requires taking a spinlock. The check_object_size()
helper can call find_vmap_area() when the destination is in vmap memory.
If show_regs() is called in interrupt context, it will attempt a call to
copy_from_user_nmi(), which may call check_object_size() and then
find_vmap_area(). If something in normal context happens to be in the
middle of calling find_vmap_area() (with the spinlock held), the interrupt
handler will hang forever.
The copy_from_user_nmi() call is actually being called with a fixed-size
length, so check_object_size() should never have been called in the
first place. In order for check_object_size() to see that the length is
a fixed size, inline copy_from_user_nmi(), as already done with all the
other uaccess helpers.
Reported-by: Yu Zhao <yuzhao@...gle.com>
Link: https://lore.kernel.org/all/CAOUHufaPshtKrTWOz7T7QFYUNVGFm0JBjvM700Nhf9qEL9b3EQ@mail.gmail.com
Reported-by: dev@...-flo.net
Patch 1 is a refactor for patch 2, and patch 3 should make sure we avoid
future deadlocks.
Thanks,
-Kees
Kees Cook (3):
x86/uaccess: Move nmi_uaccess_okay() into uaccess.h
x86/dumpstack: Inline copy_from_user_nmi()
usercopy: Add find_vmap_area_try() to avoid deadlocks
arch/x86/events/core.c | 1 -
arch/x86/include/asm/tlbflush.h | 3 --
arch/x86/include/asm/uaccess.h | 5 ++--
arch/x86/kernel/dumpstack.c | 4 +--
arch/x86/lib/Makefile | 2 +-
arch/x86/lib/usercopy.c | 52 ---------------------------------
include/asm-generic/tlb.h | 9 ------
include/linux/uaccess.h | 50 +++++++++++++++++++++++++++++++
include/linux/vmalloc.h | 1 +
kernel/trace/bpf_trace.c | 2 --
mm/usercopy.c | 11 ++++++-
mm/vmalloc.c | 11 +++++++
12 files changed, 78 insertions(+), 73 deletions(-)
delete mode 100644 arch/x86/lib/usercopy.c
--
2.34.1
Powered by blists - more mailing lists