Re: [PATCH v4 2/7] linux-user: make syscall emulation interruptible

["Alex Bennée" <alex.bennee@linaro.org>]

Florian Hofhammer <florian.hofhammer@epfl.ch> writes:

> On 25/02/2026 10:25, Alex Bennée wrote:
>> Pierrick Bouvier <pierrick.bouvier@linaro.org> writes:
>> 
>>> On 2/24/26 7:50 AM, Florian Hofhammer wrote:
>>>> The syscall emulation code previously wasn't interruptible via
>>>> cpu_loop_exit(), as this construct relies on a longjmp target that is not
>>>> live anymore in the syscall handling code. Consequently, longjmp() would
>>>> operate on a (potentially overwritten) stale jump buffer. This patch adds an additional
>>>> setjmp and the necessary handling around it to make longjmp() (and by
>>>> proxy cpu_loop_exit() safe to call even within a syscall context.
>>>> Signed-off-by: Florian Hofhammer <florian.hofhammer@epfl.ch>
>>>> ---
>>>>   linux-user/aarch64/cpu_loop.c      |  2 +-
>>>>   linux-user/alpha/cpu_loop.c        |  2 +-
>>>>   linux-user/arm/cpu_loop.c          |  2 +-
>>>>   linux-user/hexagon/cpu_loop.c      |  2 +-
>>>>   linux-user/hppa/cpu_loop.c         |  4 ++++
>>>>   linux-user/i386/cpu_loop.c         |  8 +++++---
>>>>   linux-user/include/special-errno.h |  8 ++++++++
>>>>   linux-user/loongarch64/cpu_loop.c  |  5 +++--
>>>>   linux-user/m68k/cpu_loop.c         |  2 +-
>>>>   linux-user/microblaze/cpu_loop.c   |  2 +-
>>>>   linux-user/mips/cpu_loop.c         |  5 +++--
>>>>   linux-user/or1k/cpu_loop.c         |  2 +-
>>>>   linux-user/ppc/cpu_loop.c          |  6 ++++--
>>>>   linux-user/riscv/cpu_loop.c        |  2 +-
>>>>   linux-user/s390x/cpu_loop.c        |  2 +-
>>>>   linux-user/sh4/cpu_loop.c          |  2 +-
>>>>   linux-user/sparc/cpu_loop.c        |  4 +++-
>>>>   linux-user/syscall.c               | 16 ++++++++++++++++
>>>>   linux-user/xtensa/cpu_loop.c       |  3 +++
>>>>   19 files changed, 59 insertions(+), 20 deletions(-)
>>>> diff --git a/linux-user/sparc/cpu_loop.c
>>>> b/linux-user/sparc/cpu_loop.c
>>>> index 7391e2add8..f054316dce 100644
>>>> --- a/linux-user/sparc/cpu_loop.c
>>>> +++ b/linux-user/sparc/cpu_loop.c
>>>> @@ -229,7 +229,9 @@ void cpu_loop (CPUSPARCState *env)
>>>>                                 env->regwptr[2], env->regwptr[3],
>>>>                                 env->regwptr[4], env->regwptr[5],
>>>>                                 0, 0);
>>>> -            if (ret == -QEMU_ERESTARTSYS || ret == -QEMU_ESIGRETURN) {
>>>> +            if (ret == -QEMU_ERESTARTSYS
>>>> +                    || ret == -QEMU_ESIGRETURN
>>>> +                    || ret == -QEMU_ESETPC) {
>>>>                   break;
>>>>               }
>>>
>>> Just a style nit:
>>> if (ret == -QEMU_ERESTARTSYS ||
>>>     ret == -QEMU_ESIGRETURN ||
>>>     ret == -QEMU_ESETPC) {
>> 
>> I was hopping the ret test could be wrapped up into a helper but it
>> seems sparc and x86 have enough variation in handled ret codes to make
>> that difficult.
>
> It's not just x86 and sparc, there generally seems to not be consensus
> on whether return codes are checked via a single conditional check (as
> is the case with sparc), dedicated conditional checks for each possible
> error value, or via switch statements. I think moving that into a helper
> would require a bit of refactoring across architectures.
> I could check that out for a follow-up patch set if you think this makes
> sense.

I think we can live with it for now:

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>


>
> Best regards,
> Florian

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro