Skip to content

Commit

Permalink
lazy stack: prevent deadlock when taking vma_list_mutex for write
Browse files Browse the repository at this point in the history
This patch makes all functions in core/mmu.cc that take vma_list_mutex
for write to pre-fault the stack two pages deep before using the mutex.
This is necessary to prevent any follow up stack faults down the call stack
after the vma_list_mutex is take for write as this would lead to a deadlock
experienced when testing one of the apps when the page fault handler and
mmu::vm_fault() function would try to take the same vma_list_mutex for read.

Signed-off-by: Waldemar Kozaczuk <[email protected]>
  • Loading branch information
wkozaczuk committed Oct 16, 2022
1 parent 5250512 commit 9fb4574
Showing 1 changed file with 17 additions and 0 deletions.
17 changes: 17 additions & 0 deletions core/mmu.cc
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,17 @@ extern const char text_start[], text_end[];

namespace mmu {

#if CONF_lazy_stack
// We need to ensure that lazy stack is populated deeply enough (2 pages)
// for all the cases when the vma_list_mutex is taken for write to prevent
// page faults triggered on stack. The page-fault handling logic would
// attempt to take same vma_list_mutex fo read and end up with a deadlock.
#define PREVENT_STACK_PAGE_FAULT \
arch::ensure_next_two_stack_pages();
#else
#define PREVENT_STACK_PAGE_FAULT
#endif

struct vma_range_compare {
bool operator()(const vma_range& a, const vma_range& b) {
return a.start() < b.start();
Expand Down Expand Up @@ -1271,6 +1282,7 @@ static void nohugepage(void* addr, size_t length)

error advise(void* addr, size_t size, int advice)
{
PREVENT_STACK_PAGE_FAULT
WITH_LOCK(vma_list_mutex.for_write()) {
if (!ismapped(addr, size)) {
return make_error(ENOMEM);
Expand Down Expand Up @@ -1310,6 +1322,7 @@ void* map_anon(const void* addr, size_t size, unsigned flags, unsigned perm)
size = align_up(size, mmu::page_size);
auto start = reinterpret_cast<uintptr_t>(addr);
auto* vma = new mmu::anon_vma(addr_range(start, start + size), perm, flags);
PREVENT_STACK_PAGE_FAULT
SCOPE_LOCK(vma_list_mutex.for_write());
auto v = (void*) allocate(vma, start, size, search);
if (flags & mmap_populate) {
Expand All @@ -1336,6 +1349,7 @@ void* map_file(const void* addr, size_t size, unsigned flags, unsigned perm,
auto start = reinterpret_cast<uintptr_t>(addr);
auto *vma = f->mmap(addr_range(start, start + size), flags | mmap_file, perm, offset).release();
void *v;
PREVENT_STACK_PAGE_FAULT
WITH_LOCK(vma_list_mutex.for_write()) {
v = (void*) allocate(vma, start, size, search);
if (flags & mmap_populate) {
Expand Down Expand Up @@ -1708,6 +1722,7 @@ ulong map_jvm(unsigned char* jvm_addr, size_t size, size_t align, balloon_ptr b)

auto* vma = new mmu::jvm_balloon_vma(jvm_addr, start, start + size, b, v->perm(), v->flags());

PREVENT_STACK_PAGE_FAULT
WITH_LOCK(vma_list_mutex.for_write()) {
// This means that the mapping that we had before was a balloon mapping
// that was laying around and wasn't updated to an anon mapping. If we
Expand Down Expand Up @@ -2014,6 +2029,7 @@ void free_initial_memory_range(uintptr_t addr, size_t size)

error mprotect(const void *addr, size_t len, unsigned perm)
{
PREVENT_STACK_PAGE_FAULT
SCOPE_LOCK(vma_list_mutex.for_write());

if (!ismapped(addr, len)) {
Expand All @@ -2025,6 +2041,7 @@ error mprotect(const void *addr, size_t len, unsigned perm)

error munmap(const void *addr, size_t length)
{
PREVENT_STACK_PAGE_FAULT
SCOPE_LOCK(vma_list_mutex.for_write());

length = align_up(length, mmu::page_size);
Expand Down

0 comments on commit 9fb4574

Please sign in to comment.