Re: [Qemu-devel] [PATCH v2 00/19] linux-user: fix various signal race conditions

[Peter Maydell]

Ping?

thanks
-- PMM

On 27 May 2016 at 15:51, Peter Maydell <address@hidden> wrote:
>
> This patchset overhauls the linux-user signal handling code to
> fix a number of race conditions. It is essentially a v2 of
> Timothy Baldwin's original patchset, though I have addressed
> code review issues, refactored it a little, fixed the occasional
> minor bug and added some patches of my own for other issues I
> spotted along the way.
>
> The meat of the patchset is splitting out the guest thread's idea
> of its signal mask from the host thread's actual signal mask. This
> allows us to temporarily block all host signals as a method for
> fixing some races:
>
>  * block signals in host signal handler until we have processed
>    the signal queue to deliver the guest signal (fixes a race
>    where a second host signal could arrive and we would deliver
>    it even if the guest handler's signal mask should prevent it)
>  * block signals while we are manipulating QEMU data structures which
>    the host signal handler reads (eg in sigaction syscall emulation)
>  * block signals to fix races between signals and noninterruptible
>    syscalls like pause, which we could in theory do with safe_syscall
>    but which would be a pain to do that way because of variations
>    in whether syscalls exist on different host architectures
>  * block signals to fix races for complicated syscalls like fork
>    which would be too painful to handle by trying to roll back
>    if something was interrupted partway through
>
> We also:
>  * remove a lot of code that is made redundant by processing
>    default signal actions in one place rather than two
>  * make sure that synchronous signals correctly take priority
>    over asynchronous signals
>  * use safe_syscall for sigsuspend
>  * use safe_syscall for kill/tkill/tgkill
>  * make a better guess at which bits of the union in siginfo_t
>    need to be converted by looking at si_code as well as si_signo
>  * use __get_user and __put_user for siginfo conversion to avoid
>    potential problems with misaligned guest addresses
>
> Changes since Timothy's v1 patchset:
>  * some patches at the front to factor out handle_pending_signal()
>    to avoid using goto for flow control logic
>  * new function set_sigmask() for setting signal mask when we have
>    already blocked signals -- this allows us to define calling block_signals()
>    twice to be illegal, which then means we can have signal_pending be a
>    simple flag rather than a word with two flag bits in it
>  * use the qemu atomics.h functions rather than raw volatile variable
>    (it makes it clearer that the variable has to be handled with care IMHO)
>  * bunch of extra commentary
>  * add code to stop sigprocmask being able to mark SIGKILL, SIGSTOP as blocked
>  * fixed handling of ssetmask
>  * new patches to better handle conversion of siginfo_t structures
>    (these fix problems with LTP tests like kill10 which try to kill
>    processes by sending them an asynchronous SIGSEGV and expect to
>    see the si_pid field in the resulting siginfo in the recipient.)
>
> With all of these fixes plus the safe_syscall patches now in
> master, the following LTP test cases which used to hang now do not:
>
>  mq_timedreceive01 mq_timedsend01 kill10 kill11 msgrcv03
>  nanosleep04 splice02 waitpid02 inotify06 pselect02 pselect02_64
>
> (Not all of these were signal related issues, and a few might have
> been fixed some time back.)
>
> Next on my todo list after this is to expand the safe_syscall
> support to more host architectures. There are also a few more
> bugs lurking I suspect.
>
> thanks
> -- PMM
>
>
> Peter Maydell (11):
>   linux-user: Factor out handle_signal code from
>     process_pending_signals()
>   linux-user: Move handle_pending_signal() to avoid need for declaration
>   linux-user: Fix stray tab-indent
>   linux-user: Factor out uses of do_sigprocmask() from sigreturn code
>   linux-user: Define macro for size of host kernel sigset_t
>   linux-user: Use safe_syscall for sigsuspend syscalls
>   linux-user: Fix race between multiple signals
>   linux-user: Use safe_syscall for kill, tkill and tgkill syscalls
>   linux-user: Use both si_code and si_signo when converting siginfo_t
>   linux-user: Avoid possible misalignment in host_to_target_siginfo()
>   linux-user: Avoid possible misalignment in target_to_host_siginfo()
>
> Timothy E Baldwin (8):
>   linux-user: Remove redundant default action check in queue_signal()
>   linux-user: Remove redundant gdb_queuesig()
>   linux-user: Remove real-time signal queuing
>   linux-user: Queue synchronous signals separately
>   linux-user: Block signals during sigaction() handling
>   linux-user: pause() should not pause if signal pending
>   linux-user: Restart exit() if signal pending
>   linux-user: Restart fork() if signals pending
>
>  gdbstub.c                 |  13 --
>  include/exec/gdbstub.h    |   1 -
>  linux-user/main.c         |   7 -
>  linux-user/qemu.h         |  62 ++++-
>  linux-user/signal.c       | 572 
> ++++++++++++++++++++++++++--------------------
>  linux-user/syscall.c      | 124 ++++++----
>  linux-user/syscall_defs.h |  15 ++
>  7 files changed, 476 insertions(+), 318 deletions(-)
>
> --
> 1.9.1