Re: [Qemu-devel] [PATCH v2 00/28] linux-user: fix race between signals and syscalls

[Riku Voipio]

On Thu, May 12, 2016 at 06:47:24PM +0100, Peter Maydell wrote:
> This patch series is an updated version of the original series
> sent by Timothy Baldwin in autumn of last year
> (https://lists.gnu.org/archive/html/qemu-devel/2015-09/msg01388.html).
> I have attempted to address various code review comments made on that
> original patchset.

Thanks, applied to linux-user

> Changes v1->v2:
>  * more description in comments and commit messages of what is going on
>  * implement restarting for all guest architectures (tilegx wasn't
>in the tree when v1 was sent out)
>  * reordered the patches in the series a bit; in particular we add
>support for restarting syscalls to all targets and so we don't
>need the TARGET_USE_ERESTARTSYS define
>  * fixed a bug in the Alpha restart code where we were incorrectly
>treating env->ir[IR_PV] as the PC, not env->pc
>  * restrict scope of patchset to just providing and using the
>safe_syscall mechanism. (I plan to upstream the other race fixes
>in the second half of Timothy's patchset separately once this lot
>are in.)
>  * avoid per-architecture if-ladders in configure and C code by
>creating a linux-user/host/$(HOSTARCH) type directory structure
>so the makefiles can pull in the right files for the host
>automatically
>  * added patches to use safe_syscall for futex, select and pselect
>(in particular fixing futex() allows binaries that use the Boehm
>garbage collector, like the Mono runtime, to work)
>  * include a patch which sets r14 on exit from microblaze syscalls,
>which I think clarifies how that architecture does things
>(though it is not ABI-wise strictly required)
>  * fixed bug in wrapping of waitid syscall -- it has five args, not four
>  * switched safe_syscall() to use a "return -1 and set errno"
>convention for failure; this matches syscall(), and seemed to
>me to be more intuitive for switching code around and for when
>there's a mix of code paths using safe_syscall() and directly
>calling libc functions (though it does mean that we push error
>numbers into errno and then fish them out again later)
>  * added the magic GNU-stack rune that avoids incorrectly tainting
>our whole binary as "needs an executable stack" when an asm
>file is linked into it
> 
> Notes:
>  * safe_syscall is supported for all guest architectures, but
>only the x86_64 host architecture; adding the asm fragment
>for our other hosts should be straightforward
>  * there are a fair number of other system calls that should be
>using safe_syscall too
> In both cases I thought it was better to avoid making this patchset
> any larger or later; we can easily do these in later patches if
> the general concept and mechanism has made it into master.
> 
> 
> I've tested mostly by running the LTP test suite for 32-bit ARM
> guests; this patchset neither improves nor worsens our pass rate.
> 
> I think the fixing of select/pselect ought to deal with hangs when
> running cmake (https://bugs.launchpad.net/qemu/+bug/955379), but
> I have no reliable reproducer for that bug so can't say for sure.
> The "Mono hangs due to it using Boehm GC" bug is
> https://bugs.launchpad.net/ubuntu/+source/qemu-kvm/+bug/53
> and that is definitely fixed by this patchset.
> 
> 
> thanks
> -- PMM
> 
> Peter Maydell (5):
>   linux-user: Consistently return host errnos from do_openat()
>   linux-user: Support for restarting system calls for tilegx targets
>   linux-user: Set r14 on exit from microblaze syscall
>   linux-user: Use safe_syscall for pselect, select syscalls
>   linux-user: Use safe_syscall for futex syscall
> 
> Timothy E Baldwin (23):
>   linux-user: Check array bounds in errno conversion
>   linux-user: Reindent signal handling
>   linux-user: Define TARGET_ERESTART* errno values
>   linux-user: Renumber TARGET_QEMU_ESIGRETURN, make it not arch-specific
>   linux-user: Support for restarting system calls for x86 targets
>   linux-user: Support for restarting system calls for ARM targets
>   linux-user: Support for restarting system calls for MIPS targets
>   linux-user: Support for restarting system calls for PPC targets
>   linux-user: Support for restarting system calls for SPARC targets
>   linux-user: Support for restarting system calls for SH4 targets
>   linux-user: Support for restarting system calls for Alpha targets
>   linux-user: Support for restarting system calls for UniCore32 targets
>   linux-user: Support for restarting system calls for OpenRISC targets
>   linux-user: Support for restarting system calls for M68K targets
>   linux-user: Support for restarting system calls for S390 targets
>   linux-user: Support for restarting system calls for CRIS targets
>   linux-user: Support for restarting system calls for Microblaze targets
>   linux-user: Add debug code to exercise restarting system calls
>   linux-user: Provide safe_syscall for fixing races between signals and
> syscalls
>   

Re: [Qemu-devel] [PATCH v2 00/28] linux-user: fix race between signals and syscalls

[Peter Maydell]

Ping?

I have a second series based on top of this which is shaping up
nicely and it would be good to get this lot into master before
posting the second set...

thanks
-- PMM

On 12 May 2016 at 18:47, Peter Maydell  wrote:
> This patch series is an updated version of the original series
> sent by Timothy Baldwin in autumn of last year
> (https://lists.gnu.org/archive/html/qemu-devel/2015-09/msg01388.html).
> I have attempted to address various code review comments made on that
> original patchset.
>
> Changes v1->v2:
>  * more description in comments and commit messages of what is going on
>  * implement restarting for all guest architectures (tilegx wasn't
>in the tree when v1 was sent out)
>  * reordered the patches in the series a bit; in particular we add
>support for restarting syscalls to all targets and so we don't
>need the TARGET_USE_ERESTARTSYS define
>  * fixed a bug in the Alpha restart code where we were incorrectly
>treating env->ir[IR_PV] as the PC, not env->pc
>  * restrict scope of patchset to just providing and using the
>safe_syscall mechanism. (I plan to upstream the other race fixes
>in the second half of Timothy's patchset separately once this lot
>are in.)
>  * avoid per-architecture if-ladders in configure and C code by
>creating a linux-user/host/$(HOSTARCH) type directory structure
>so the makefiles can pull in the right files for the host
>automatically
>  * added patches to use safe_syscall for futex, select and pselect
>(in particular fixing futex() allows binaries that use the Boehm
>garbage collector, like the Mono runtime, to work)
>  * include a patch which sets r14 on exit from microblaze syscalls,
>which I think clarifies how that architecture does things
>(though it is not ABI-wise strictly required)
>  * fixed bug in wrapping of waitid syscall -- it has five args, not four
>  * switched safe_syscall() to use a "return -1 and set errno"
>convention for failure; this matches syscall(), and seemed to
>me to be more intuitive for switching code around and for when
>there's a mix of code paths using safe_syscall() and directly
>calling libc functions (though it does mean that we push error
>numbers into errno and then fish them out again later)
>  * added the magic GNU-stack rune that avoids incorrectly tainting
>our whole binary as "needs an executable stack" when an asm
>file is linked into it
>
> Notes:
>  * safe_syscall is supported for all guest architectures, but
>only the x86_64 host architecture; adding the asm fragment
>for our other hosts should be straightforward
>  * there are a fair number of other system calls that should be
>using safe_syscall too
> In both cases I thought it was better to avoid making this patchset
> any larger or later; we can easily do these in later patches if
> the general concept and mechanism has made it into master.
>
>
> I've tested mostly by running the LTP test suite for 32-bit ARM
> guests; this patchset neither improves nor worsens our pass rate.
>
> I think the fixing of select/pselect ought to deal with hangs when
> running cmake (https://bugs.launchpad.net/qemu/+bug/955379), but
> I have no reliable reproducer for that bug so can't say for sure.
> The "Mono hangs due to it using Boehm GC" bug is
> https://bugs.launchpad.net/ubuntu/+source/qemu-kvm/+bug/53
> and that is definitely fixed by this patchset.
>
>
> thanks
> -- PMM
>
> Peter Maydell (5):
>   linux-user: Consistently return host errnos from do_openat()
>   linux-user: Support for restarting system calls for tilegx targets
>   linux-user: Set r14 on exit from microblaze syscall
>   linux-user: Use safe_syscall for pselect, select syscalls
>   linux-user: Use safe_syscall for futex syscall
>
> Timothy E Baldwin (23):
>   linux-user: Check array bounds in errno conversion
>   linux-user: Reindent signal handling
>   linux-user: Define TARGET_ERESTART* errno values
>   linux-user: Renumber TARGET_QEMU_ESIGRETURN, make it not arch-specific
>   linux-user: Support for restarting system calls for x86 targets
>   linux-user: Support for restarting system calls for ARM targets
>   linux-user: Support for restarting system calls for MIPS targets
>   linux-user: Support for restarting system calls for PPC targets
>   linux-user: Support for restarting system calls for SPARC targets
>   linux-user: Support for restarting system calls for SH4 targets
>   linux-user: Support for restarting system calls for Alpha targets
>   linux-user: Support for restarting system calls for UniCore32 targets
>   linux-user: Support for restarting system calls for OpenRISC targets
>   linux-user: Support for restarting system calls for M68K targets
>   linux-user: Support for restarting system calls for S390 targets
>   linux-user: Support for restarting system calls for CRIS targets
>   linux-user: Support for restarting system calls for Microblaze targets
>   linux-user: