On 7/13/26 07:14, Peter Maydell wrote:
Every callsite of block_signals() checks its return value, except the one in sparc64_set_context(). Generally you need to check, because the standard pattern is:if (block_signals()) { return -QEMU_ERESTARTSYS; } /* do some blocking syscall */ and we need to take any pending signal before we do the blocking operation, not afterwards. The use in sparc64_set_context() doesn't do this. It doesn't have to because the operations it is doing aren't blocking, so it won't get into "we didn't take the signal that we should have" races that blocking syscalls do. But it does make this way of updating the signal mask inconsistent with how we do it in do_sigprocmask(). do_sigprocmask() does the usual "return -QEMU_ERESTARTSYS", so a pending signal that was not blocked by the old signal mask and which will be blocked by the new mask we're about to install will be taken before we change the mask. sparc64_set_context() doesn't check the return value, so we won't take that pending signal. That's not wrong, because it just means the signal lost the race with the executing code. But it seems clearer to behave the same way as do_sigprocmask(), not differently. Make sparc64_set_context() check the return value of block_signals() and return early if there's a pending signal to take. We don't need to return a separate return code to indicate this because the main loop handles it the same either way. Coverity CID: 1660058 Fixes: e0f0ce88eb9 ("linux-user/sparc: call block_signals() before set_sigmask() in setcontext") Signed-off-by: Peter Maydell<[email protected]> --- I thought about just adding a comment about why we're OK to ignore the return value, but eventually decided that consistency with how other parts of the linux-user code handle setting the signal mask was better. This was originally flagged up by Coverity, not by any actual observed problem. Disclaimer: checked the original repro test case that prompted e0f0ce88eb9, and ran 'make check-tcg', but no other testing. --- linux-user/sparc/cpu_loop.c | 10 ++++++++++ linux-user/sparc/signal.c | 30 +++++++++++++++++++++--------- 2 files changed, 31 insertions(+), 9 deletions(-)
Reviewed-by: Richard Henderson <[email protected]> r~
