Skip to content

Route mmap_lock through acquire and release - #397

Open
Max042004 wants to merge 1 commit into
sysprog21:mainfrom
Max042004:mmap-lock-helpers
Open

Max042004 wants to merge 1 commit into
sysprog21:mainfrom
Max042004:mmap-lock-helpers

Conversation

@Max042004

@Max042004 Max042004 commented Sep 29, 2026 •

Copy link
Copy Markdown
Collaborator

This opens a series of PRs that split #372 into smaller parts so the mmap changes are easier to review.
This PR routes all mmap_lock through mmap_lock_acquire() and mmap_lock_release(), so later PRs hook one place instead of every call site, and misuse trips an assertion.

mmap_lock has the default mutex type, so an owner that takes it a second time deadlocks with no trace, and an unlock from a thread that does not hold it is undefined. Nothing at the raw pthread call sites could catch either mistake.

mmap_lock_acquire and mmap_lock_release record the owner in a thread-local flag and assert on a recursive acquire or an unowned release, so the mistake aborts at the offending call instead of hanging the VM.

The lock order and every critical section are unchanged.

mmap_lock has the default mutex type, so an owner that takes it a
second time deadlocks with no trace, and an unlock from a thread that
does not hold it is undefined. Nothing at the raw pthread call sites
could catch either mistake.

mmap_lock_acquire and mmap_lock_release record the owner in a
thread-local flag and assert on a recursive acquire or an unowned
release, so the mistake aborts at the offending call instead of
hanging the VM.

The lock order and every critical section are unchanged.
Comment thread src/syscall/mem.c
static pthread_mutex_t mmap_lock =
PTHREAD_MUTEX_INITIALIZER; /* Lock order: 1 */

static _Thread_local bool mmap_lock_owned;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The flag catches misuse at these two wrappers, but the preconditions it could enforce live elsewhere: gva_resolve_perm, guest_split_block, guest_update_perms, and guest_materialize_lazy all carry a "callers MUST hold mmap_lock" comment in src/core/guest.h, and mem.c repeats the same prose above the region-splitting and overlay helpers. Exporting a mmap_lock_held() reader over mmap_lock_owned would let a follow-up turn those into asserts at the callee, which is where a missing acquire actually corrupts the region table.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants