[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH for-6.2 0/7] linux-user: Clean up siginfo_t handling for arm,
From: |
Laurent Vivier |
Subject: |
Re: [PATCH for-6.2 0/7] linux-user: Clean up siginfo_t handling for arm, aarch64 |
Date: |
Thu, 23 Sep 2021 15:12:07 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 |
Le 13/08/2021 à 15:18, Peter Maydell a écrit :
> Coverity reported that we don't set the _sifields union when queuing
> the SIGTRAP for EXCP_DEBUG events on aarch64. This series fixes that
> bug and a few others, and cleans up the way we queue fault signals
> to be less error-prone.
>
> The underlying cause of the bug is that when queueing a signal
> the code must set the right field in the _sifields union depending
> on the si_type that it passes to queue_signal(), and there's nothing
> guarding against forgetting to do this. The "fill in a whole struct
> and then call queue_signal()" code is also a bit longwinded. In the
> real kernel, there is a function force_sig_fault() which does this
> for the SI_FAULT signal type. The series provides a QEMU implementation
> of this (which goes alongside the existing force_sig() that does this
> for SI_KILL signals), and uses it in the arm and aarch64 code.
> Because force_sig_fault() takes the address as an argument, it's
> not possible for the caller to forget to fill it in.
>
> Obviously we should also convert the other targets where they are
> directly calling queue_signal() (there are other places that forget
> to fill in the sifields union fields; I don't know why Coverity
> hasn't spotted those). But I thought it better to send this out
> for review of the new API and approach before spending time on
> converting other targets. (In particular fixing places where
> EXCP_DEBUG handling forgets to set the sifields address is a
> pain, because in the real kernel different architectures might
> either report the PC or else report a zero address here, so it
> requires looking into the kernel sources to check which...)
> Once all the archs are cleaned up we might be able to make
> queue_signal() static so it's local to signal.c.
>
> PS: there's probably a trivial conflict with my include-file-split
> patchseries.
>
> thanks
> -- PMM
>
> Peter Maydell (7):
> linux-user/aarch64: Set siginfo_t addr field for SIGTRAP signals
> linux-user/arm: Set siginfo_t addr field for SIGTRAP signals
> linux-user/arm: Use force_sig() to deliver fpa11 emulation SIGFPE
> linux-user: Zero out target_siginfo_t in force_sig()
> linux-user: Provide new force_sig_fault() function
> linux-user/arm: Use force_sig_fault()
> linux-user/aarch64: Use force_sig_fault()
>
> linux-user/signal-common.h | 1 +
> linux-user/aarch64/cpu_loop.c | 33 +++++-------------
> linux-user/arm/cpu_loop.c | 64 +++++++++++------------------------
> linux-user/signal.c | 19 ++++++++++-
> 4 files changed, 48 insertions(+), 69 deletions(-)
>
Applied to my linux-user-for-6.2 branch.
Thanks,
Laurent
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- Re: [PATCH for-6.2 0/7] linux-user: Clean up siginfo_t handling for arm, aarch64,
Laurent Vivier <=