qemu-commits
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Qemu-commits] [qemu/qemu] 024949: linux-user: Fix locking order in fork


From: GitHub
Subject: [Qemu-commits] [qemu/qemu] 024949: linux-user: Fix locking order in fork_start()
Date: Thu, 25 Jan 2018 03:19:00 -0800

  Branch: refs/heads/master
  Home:   https://github.com/qemu/qemu
  Commit: 024949caf32805f4cc3e7d363a80084b47aac1f6
      
https://github.com/qemu/qemu/commit/024949caf32805f4cc3e7d363a80084b47aac1f6
  Author: Peter Maydell <address@hidden>
  Date:   2018-01-23 (Tue, 23 Jan 2018)

  Changed paths:
    M linux-user/main.c

  Log Message:
  -----------
  linux-user: Fix locking order in fork_start()

Our locking order is that the tb lock should be taken
inside the mmap_lock, but fork_start() grabs locks the
other way around. This means that if a heavily multithreaded
guest process (such as Java) calls fork() it can deadlock,
with the thread that called fork() stuck in fork_start()
with the tb lock and waiting for the mmap lock, but some
other thread in tb_find() with the mmap lock and waiting
for the tb lock. The cpu_list_lock() should also always be
taken last, not first.

Fix this by making fork_start() grab the locks in the
right order. The order in which we drop locks doesn't
matter, so we leave fork_end() the way it is.

Signed-off-by: Peter Maydell <address@hidden>
Cc: address@hidden
Reviewed-by: Paolo Bonzini <address@hidden>
Reviewed-by: Alex Bennée <address@hidden>
Message-Id: <address@hidden>
Signed-off-by: Laurent Vivier <address@hidden>


  Commit: 06065c451f10c7ef62cfb575a87f323a70ae1c9e
      
https://github.com/qemu/qemu/commit/06065c451f10c7ef62cfb575a87f323a70ae1c9e
  Author: Peter Maydell <address@hidden>
  Date:   2018-01-23 (Tue, 23 Jan 2018)

  Changed paths:
    M linux-user/main.c

  Log Message:
  -----------
  linux-user: wrap fork() in a start/end exclusive section

When we do a fork() in usermode emulation, we need to be in
a start/end exclusive section, so that we can ensure that no
other thread is in an RCU section. Otherwise you can get this
deadlock:

- fork thread: has mmap_lock, waits for rcu_sync_lock
  (because rcu_init_lock() is registered as a pthread_atfork() hook)
- RCU thread: has rcu_sync_lock, waits for rcu_read_(un)lock
- another CPU thread: in RCU critical section, waits for mmap_lock

This can show up if you have a heavily multithreaded guest program
that does a fork().

Signed-off-by: Peter Maydell <address@hidden>
Reported-by: Stuart Monteith <address@hidden>
Message-Id: <address@hidden>
Signed-off-by: Laurent Vivier <address@hidden>


  Commit: 7174970a94df10ee84143edc7c94a2472d654604
      
https://github.com/qemu/qemu/commit/7174970a94df10ee84143edc7c94a2472d654604
  Author: Peter Maydell <address@hidden>
  Date:   2018-01-23 (Tue, 23 Jan 2018)

  Changed paths:
    M linux-user/syscall.c

  Log Message:
  -----------
  linux-user: Fix length calculations in host_to_target_cmsg()

The handling of length calculations in host_to_target_cmsg()
was rather confused:
 * when checking for whether the target cmsg header fit in
   the remaining buffer, we were using the host struct size,
   not the target size
 * we were setting tgt_len to "target payload + header length"
   but then using it as if it were the target payload length alone
 * in various message type cases we weren't handling the possibility
   that host or target buffers were truncated

Fix these problems. The second one in particular is liable
to result in us overrunning the guest provided buffer,
since we will try to convert more data than is actually
present.

Fixes: https://bugs.launchpad.net/qemu/+bug/1701808
Reported-by: Bruno Haible  <address@hidden>
Signed-off-by: Peter Maydell <address@hidden>
Message-Id: <address@hidden>
Signed-off-by: Laurent Vivier <address@hidden>


  Commit: ad762b990fa9da53e203b934583838d4dd371e20
      
https://github.com/qemu/qemu/commit/ad762b990fa9da53e203b934583838d4dd371e20
  Author: Peter Maydell <address@hidden>
  Date:   2018-01-23 (Tue, 23 Jan 2018)

  Changed paths:
    M linux-user/syscall.c
    M linux-user/syscall_defs.h

  Log Message:
  -----------
  linux-user: Don't use CMSG_ALIGN(sizeof struct cmsghdr)

The Linux struct cmsghdr is already guaranteed to be sufficiently
aligned that CMSG_ALIGN(sizeof struct cmsghdr) is always equal
to sizeof struct cmsghdr. Stop doing the unnecessary alignment
arithmetic for host and target cmsghdr.

This follows kernel commit 1ff8cebf49ed9e9ca2 and brings our
TARGET_CMSG_* macros back into line with the kernel ones,
as well as making them easier to understand.

Signed-off-by: Peter Maydell <address@hidden>
Reviewed-by: Laurent Vivier <address@hidden>
Message-Id: <address@hidden>
Signed-off-by: Laurent Vivier <address@hidden>


  Commit: 10fa993aae539fa8d0da1da17169e05f1dfb4f95
      
https://github.com/qemu/qemu/commit/10fa993aae539fa8d0da1da17169e05f1dfb4f95
  Author: Peter Maydell <address@hidden>
  Date:   2018-01-23 (Tue, 23 Jan 2018)

  Changed paths:
    M linux-user/syscall.c

  Log Message:
  -----------
  linux-user: Translate flags argument to dup3 syscall

The third argument to dup3() is a flags word which may be
O_CLOEXEC. We weren't translating this flag from target to
host value, which meant that if the target used a different
value from the host (eg sparc guest and x86 host) the dup3()
call would fail EINVAL. Do the correct translation.

Fixes: https://bugs.launchpad.net/qemu/+bug/1704658
Reported-by: Bruno Haible  <address@hidden>
Signed-off-by: Peter Maydell <address@hidden>
Reviewed-by: Laurent Vivier <address@hidden>
Message-Id: <address@hidden>
Signed-off-by: Laurent Vivier <address@hidden>


  Commit: 95e6d4305affbbf5258c6a533be9bfd24599da90
      
https://github.com/qemu/qemu/commit/95e6d4305affbbf5258c6a533be9bfd24599da90
  Author: Maximilian Riemensberger <address@hidden>
  Date:   2018-01-23 (Tue, 23 Jan 2018)

  Changed paths:
    M linux-user/mmap.c

  Log Message:
  -----------
  linux-user/mmap.c: Avoid choosing NULL as start address

mmap() is required by the linux kernel ABI and POSIX to return a
non-NULL address when the implementation chooses a start address for the
mapping.

The current implementation of mmap_find_vma_reserved() can return NULL
as start address of a mapping which leads to subsequent crashes inside
the guests glibc, e.g. output of qemu-arm-static --strace executing a
test binary stx_test:

    1879 
mmap2(NULL,8388608,PROT_READ|PROT_WRITE,MAP_PRIVATE|MAP_ANONYMOUS|0x20000,-1,0) 
= 0x00000000
    1879 write(2,0xf6fd39d0,79) stx_test: allocatestack.c:514: allocate_stack: 
Assertion `mem != NULL' failed.

This patch fixes mmap_find_vma_reserved() by skipping NULL as start
address while searching for a suitable mapping start address.

CC: Riku Voipio <address@hidden>
CC: Laurent Vivier <address@hidden>
CC: Peter Maydell <address@hidden>
Signed-off-by: Maximilian Riemensberger <address@hidden>
Reviewed-by: Laurent Vivier <address@hidden>
Reviewed-by: Richard Henderson <address@hidden>
Message-Id: <address@hidden>
Signed-off-by: Laurent Vivier <address@hidden>


  Commit: 2e0a8713bd6804ce49044915ba9cd5bd7148de77
      
https://github.com/qemu/qemu/commit/2e0a8713bd6804ce49044915ba9cd5bd7148de77
  Author: Samuel Thibault <address@hidden>
  Date:   2018-01-23 (Tue, 23 Jan 2018)

  Changed paths:
    M linux-user/syscall.c

  Log Message:
  -----------
  linux-user: Fix sched_get/setaffinity conversion

sched_get/setaffinity linux-user syscalls were missing conversions for
little/big endian, which is hairy since longs may not be the same size
either.

For simplicity, this just introduces loops to convert bit by bit like is
done for select.

Signed-off-by: Samuel Thibault <address@hidden>
Reviewed-by: Laurent Vivier <address@hidden>
Message-Id: <address@hidden>
Signed-off-by: Laurent Vivier <address@hidden>


  Commit: 444cd5c3aeb2acffce302e354ad22d1bb37bbfae
      
https://github.com/qemu/qemu/commit/444cd5c3aeb2acffce302e354ad22d1bb37bbfae
  Author: Marco A L Barbosa <address@hidden>
  Date:   2018-01-23 (Tue, 23 Jan 2018)

  Changed paths:
    M linux-user/elfload.c

  Log Message:
  -----------
  linux-user: Add AT_SECURE auxval

Signed-off-by: Marco A L Barbosa <address@hidden>
Reviewed-by: Peter Maydell <address@hidden>
Reviewed-by: Laurent Vivier <address@hidden>
Message-Id: <address@hidden>
Signed-off-by: Laurent Vivier <address@hidden>


  Commit: b827c3ed30e4777ab9bd91cb240252a4815b3322
      
https://github.com/qemu/qemu/commit/b827c3ed30e4777ab9bd91cb240252a4815b3322
  Author: Samuel Thibault <address@hidden>
  Date:   2018-01-23 (Tue, 23 Jan 2018)

  Changed paths:
    M linux-user/syscall.c

  Log Message:
  -----------
  linux-user: Add getcpu() support

Signed-off-by: Samuel Thibault <address@hidden>
Reviewed-by: Laurent Vivier <address@hidden>
Message-Id: <address@hidden>
Signed-off-by: Laurent Vivier <address@hidden>


  Commit: bfdec7f80e639cb858ed7df77da763cca85e1e61
      
https://github.com/qemu/qemu/commit/bfdec7f80e639cb858ed7df77da763cca85e1e61
  Author: Laurent Vivier <address@hidden>
  Date:   2018-01-23 (Tue, 23 Jan 2018)

  Changed paths:
    M hw/core/Makefile.objs
    A hw/core/qdev-fw.c
    M hw/core/qdev.c

  Log Message:
  -----------
  linux-user: remove nmi.c and fw-path-provider.c

linux-user binaries don't need firmware and NMI,
so don't add them in this case, move QDEV
firmware functions to qdev-fw.c

Signed-off-by: Laurent Vivier <address@hidden>
Acked-by: Paolo Bonzini <address@hidden>
Message-Id: <address@hidden>


  Commit: a78b1299f1bbb9608e3e3a36a7f16cf700a2789d
      
https://github.com/qemu/qemu/commit/a78b1299f1bbb9608e3e3a36a7f16cf700a2789d
  Author: Peter Maydell <address@hidden>
  Date:   2018-01-23 (Tue, 23 Jan 2018)

  Changed paths:
    M accel/tcg/user-exec.c

  Log Message:
  -----------
  linux-user: Propagate siginfo_t through to handle_cpu_signal()

Currently all the architecture/OS specific cpu_signal_handler()
functions call handle_cpu_signal() without passing it the
siginfo_t. We're going to want that so we can look at the si_code
to determine whether this is a SEGV_ACCERR access violation or
some other kind of fault, so change the functions to pass through
the pointer to the siginfo_t rather than just the si_addr value.

Signed-off-by: Peter Maydell <address@hidden>
Reviewed-by: Richard Henderson <address@hidden>
Message-Id: <address@hidden>
Signed-off-by: Laurent Vivier <address@hidden>


  Commit: 9c4bbee9e3b83544257e82566342c29e15a88637
      
https://github.com/qemu/qemu/commit/9c4bbee9e3b83544257e82566342c29e15a88637
  Author: Peter Maydell <address@hidden>
  Date:   2018-01-23 (Tue, 23 Jan 2018)

  Changed paths:
    M accel/tcg/translate-all.c
    M accel/tcg/user-exec.c

  Log Message:
  -----------
  page_unprotect(): handle calls to pages that are PAGE_WRITE

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.)

Signed-off-by: Peter Maydell <address@hidden>
Message-Id: <address@hidden>
Signed-off-by: Laurent Vivier <address@hidden>


  Commit: 95d0307cc10ca3df879c1be519f1ad650efb20a8
      
https://github.com/qemu/qemu/commit/95d0307cc10ca3df879c1be519f1ad650efb20a8
  Author: Andreas Schwab <address@hidden>
  Date:   2018-01-23 (Tue, 23 Jan 2018)

  Changed paths:
    M linux-user/syscall.c

  Log Message:
  -----------
  linux-user: implement renameat2

This is needed for new architectures like RISC-V which do not provide any
other rename-like syscall.

Signed-off-by: Andreas Schwab <address@hidden>
Reviewed-by: Laurent Vivier <address@hidden>
Message-Id: <address@hidden>
Signed-off-by: Laurent Vivier <address@hidden>


  Commit: 0f79bfe38a2cf0f43c7ea4959da7f8ebd7858f3d
      
https://github.com/qemu/qemu/commit/0f79bfe38a2cf0f43c7ea4959da7f8ebd7858f3d
  Author: Peter Maydell <address@hidden>
  Date:   2018-01-25 (Thu, 25 Jan 2018)

  Changed paths:
    M accel/tcg/translate-all.c
    M accel/tcg/user-exec.c
    M hw/core/Makefile.objs
    A hw/core/qdev-fw.c
    M hw/core/qdev.c
    M linux-user/elfload.c
    M linux-user/main.c
    M linux-user/mmap.c
    M linux-user/syscall.c
    M linux-user/syscall_defs.h

  Log Message:
  -----------
  Merge remote-tracking branch 
'remotes/vivier2/tags/linux-user-for-2.12-pull-request' into staging

# gpg: Signature made Tue 23 Jan 2018 14:47:41 GMT
# gpg:                using RSA key 0xF30C38BD3F2FBE3C
# gpg: Good signature from "Laurent Vivier <address@hidden>"
# gpg:                 aka "Laurent Vivier <address@hidden>"
# gpg:                 aka "Laurent Vivier (Red Hat) <address@hidden>"
# Primary key fingerprint: CD2F 75DD C8E3 A4DC 2E4F  5173 F30C 38BD 3F2F BE3C

* remotes/vivier2/tags/linux-user-for-2.12-pull-request:
  linux-user: implement renameat2
  page_unprotect(): handle calls to pages that are PAGE_WRITE
  linux-user: Propagate siginfo_t through to handle_cpu_signal()
  linux-user: remove nmi.c and fw-path-provider.c
  linux-user: Add getcpu() support
  linux-user: Add AT_SECURE auxval
  linux-user: Fix sched_get/setaffinity conversion
  linux-user/mmap.c: Avoid choosing NULL as start address
  linux-user: Translate flags argument to dup3 syscall
  linux-user: Don't use CMSG_ALIGN(sizeof struct cmsghdr)
  linux-user: Fix length calculations in host_to_target_cmsg()
  linux-user: wrap fork() in a start/end exclusive section
  linux-user: Fix locking order in fork_start()

Signed-off-by: Peter Maydell <address@hidden>


Compare: https://github.com/qemu/qemu/compare/f78b6f9b1161...0f79bfe38a2c

reply via email to

[Prev in Thread] Current Thread [Next in Thread]