[Qemu-devel] [PATCH 0/2] linux-user: Fix race between threads in page_unprotect()

[Peter Maydell]

If multiple guest threads in user-mode emulation write to a
page which QEMU has marked read-only because of cached TCG
translations, the threads can race in page_unprotect:

 * threads A & B both try to do a write to a page with code in it at
   the same time (ie which we've made non-writeable, so SEGV)
 * they race into the signal handler with this faulting address
 * thread A happens to get to page_unprotect() first and takes the
   mmap lock, so thread B sits waiting for it to be done
 * A then finds the page, marks it PAGE_WRITE and mprotect()s it writable
 * A can then continue OK (returns from signal handler to retry the
   memory access)
 * ...but when B gets the mmap lock it finds that the page is already
   PAGE_WRITE, and so it exits page_unprotect() via the "not due to
   protected translation" code path, and wrongly delivers the signal
   to the guest rather than just retrying the access

In particular, this meant that trying to run 'javac' in user-mode
emulation would fail with a spurious guest SIGSEGV.

Handle this by making page_unprotect() assume that a call for a page
which is already PAGE_WRITE is due to a race of this sort and return
a "fault handled" indication.

Since this would cause an infinite loop if we ever called
page_unprotect() for some other kind of fault than "write failed due
to bad access permissions", tighten the condition in
handle_cpu_signal() to check the signal number and si_code, and add a
comment so that if somebody does ever find themselves debugging an
infinite loop of faults they have some clue about why.

(The trick for identifying the correct setting for
current_tb_invalidated for thread B (needed to handle the precise-SMC
case) is due to Richard Henderson.  Paolo Bonzini suggested just
relying on si_code rather than trying anything more complicated.)

I'm slightly nervous about assuming that we get SEGV with SEGV_ACCERR
on a write if and only if the page permissions are set to non-writeable
(for instance https://www.mail-archive.com/address@hidden/msg40358.html
is a surprisingly recent patch to fix this for OpenBSD/x86, and I
would be unsurprised if QEMU usermode itself got the si_code wrong,
and the Boehm gc code at 
https://github.com/ivmai/bdwgc/blob/master/os_dep.c#L3182
is quite convoluted), but on the other hand we really ought in theory
to be able to rely on si_code, and trying to handle this some other way
gets pretty convoluted...

thanks
-- PMM

Peter Maydell (2):
  linux-user: Propagate siginfo_t through to handle_cpu_signal()
  page_unprotect(): handle calls to pages that are PAGE_WRITE

 accel/tcg/translate-all.c | 50 ++++++++++++++++++++++++++++-----------------
 accel/tcg/user-exec.c     | 52 +++++++++++++++++++++++------------------------
 2 files changed, 57 insertions(+), 45 deletions(-)

-- 
2.7.4