All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Maydell <peter.maydell@linaro.org>
To: qemu-devel@nongnu.org
Subject: [PULL 10/29] linux-user/arm: Fix identification of syscall numbers
Date: Thu, 21 May 2020 20:15:51 +0100	[thread overview]
Message-ID: <20200521191610.10941-11-peter.maydell@linaro.org> (raw)
In-Reply-To: <20200521191610.10941-1-peter.maydell@linaro.org>

Our code to identify syscall numbers has some issues:
 * for Thumb mode, we never need the immediate value from the insn,
   but we always read it anyway
 * bad immediate values in the svc insn should cause a SIGILL, but we
   were abort()ing instead (via "goto error")

We can fix both these things by refactoring the code that identifies
the syscall number to more closely follow the kernel COMPAT_OABI code:
 * for Thumb it is always r7
 * for Arm, if the immediate value is 0, then this is an EABI call
   with the syscall number in r7
 * otherwise, we XOR the immediate value with 0x900000
   (ARM_SYSCALL_BASE for QEMU; __NR_OABI_SYSCALL_BASE in the kernel),
   which converts valid syscall immediates into the desired value,
   and puts all invalid immediates in the range 0x100000 or above
 * then we can just let the existing "value too large, deliver
   SIGILL" case handle invalid numbers, and drop the 'goto error'

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
Message-id: 20200420212206.12776-5-peter.maydell@linaro.org
---
 linux-user/arm/cpu_loop.c | 143 ++++++++++++++++++++------------------
 1 file changed, 77 insertions(+), 66 deletions(-)

diff --git a/linux-user/arm/cpu_loop.c b/linux-user/arm/cpu_loop.c
index df8b7b3fa96..13629ee1f6a 100644
--- a/linux-user/arm/cpu_loop.c
+++ b/linux-user/arm/cpu_loop.c
@@ -299,85 +299,96 @@ void cpu_loop(CPUARMState *env)
                 env->eabi = 1;
                 /* system call */
                 if (env->thumb) {
-                    /* FIXME - what to do if get_user() fails? */
-                    get_user_code_u16(insn, env->regs[15] - 2, env);
-                    n = insn & 0xff;
+                    /* Thumb is always EABI style with syscall number in r7 */
+                    n = env->regs[7];
                 } else {
+                    /*
+                     * Equivalent of kernel CONFIG_OABI_COMPAT: read the
+                     * Arm SVC insn to extract the immediate, which is the
+                     * syscall number in OABI.
+                     */
                     /* FIXME - what to do if get_user() fails? */
                     get_user_code_u32(insn, env->regs[15] - 4, env);
                     n = insn & 0xffffff;
-                }
-
-                if (n == 0 || n >= ARM_SYSCALL_BASE || env->thumb) {
-                    /* linux syscall */
-                    if (env->thumb || n == 0) {
+                    if (n == 0) {
+                        /* zero immediate: EABI, syscall number in r7 */
                         n = env->regs[7];
                     } else {
-                        n -= ARM_SYSCALL_BASE;
+                        /*
+                         * This XOR matches the kernel code: an immediate
+                         * in the valid range (0x900000 .. 0x9fffff) is
+                         * converted into the correct EABI-style syscall
+                         * number; invalid immediates end up as values
+                         * > 0xfffff and are handled below as out-of-range.
+                         */
+                        n ^= ARM_SYSCALL_BASE;
                         env->eabi = 0;
                     }
-                    if ( n > ARM_NR_BASE) {
-                        switch (n) {
-                        case ARM_NR_cacheflush:
-                            /* nop */
-                            break;
-                        case ARM_NR_set_tls:
-                            cpu_set_tls(env, env->regs[0]);
-                            env->regs[0] = 0;
-                            break;
-                        case ARM_NR_breakpoint:
-                            env->regs[15] -= env->thumb ? 2 : 4;
-                            goto excp_debug;
-                        case ARM_NR_get_tls:
-                            env->regs[0] = cpu_get_tls(env);
-                            break;
-                        default:
-                            if (n < 0xf0800) {
-                                /*
-                                 * Syscalls 0xf0000..0xf07ff (or 0x9f0000..
-                                 * 0x9f07ff in OABI numbering) are defined
-                                 * to return -ENOSYS rather than raising
-                                 * SIGILL. Note that we have already
-                                 * removed the 0x900000 prefix.
-                                 */
-                                qemu_log_mask(LOG_UNIMP,
-                                    "qemu: Unsupported ARM syscall: 0x%x\n",
-                                              n);
-                                env->regs[0] = -TARGET_ENOSYS;
+                }
+
+                if (n > ARM_NR_BASE) {
+                    switch (n) {
+                    case ARM_NR_cacheflush:
+                        /* nop */
+                        break;
+                    case ARM_NR_set_tls:
+                        cpu_set_tls(env, env->regs[0]);
+                        env->regs[0] = 0;
+                        break;
+                    case ARM_NR_breakpoint:
+                        env->regs[15] -= env->thumb ? 2 : 4;
+                        goto excp_debug;
+                    case ARM_NR_get_tls:
+                        env->regs[0] = cpu_get_tls(env);
+                        break;
+                    default:
+                        if (n < 0xf0800) {
+                            /*
+                             * Syscalls 0xf0000..0xf07ff (or 0x9f0000..
+                             * 0x9f07ff in OABI numbering) are defined
+                             * to return -ENOSYS rather than raising
+                             * SIGILL. Note that we have already
+                             * removed the 0x900000 prefix.
+                             */
+                            qemu_log_mask(LOG_UNIMP,
+                                "qemu: Unsupported ARM syscall: 0x%x\n",
+                                          n);
+                            env->regs[0] = -TARGET_ENOSYS;
+                        } else {
+                            /*
+                             * Otherwise SIGILL. This includes any SWI with
+                             * immediate not originally 0x9fxxxx, because
+                             * of the earlier XOR.
+                             */
+                            info.si_signo = TARGET_SIGILL;
+                            info.si_errno = 0;
+                            info.si_code = TARGET_ILL_ILLTRP;
+                            info._sifields._sigfault._addr = env->regs[15];
+                            if (env->thumb) {
+                                info._sifields._sigfault._addr -= 2;
                             } else {
-                                /* Otherwise SIGILL */
-                                info.si_signo = TARGET_SIGILL;
-                                info.si_errno = 0;
-                                info.si_code = TARGET_ILL_ILLTRP;
-                                info._sifields._sigfault._addr = env->regs[15];
-                                if (env->thumb) {
-                                    info._sifields._sigfault._addr -= 2;
-                                } else {
-                                    info._sifields._sigfault._addr -= 4;
-                                }
-                                queue_signal(env, info.si_signo,
-                                             QEMU_SI_FAULT, &info);
+                                info._sifields._sigfault._addr -= 4;
                             }
-                            break;
-                        }
-                    } else {
-                        ret = do_syscall(env,
-                                         n,
-                                         env->regs[0],
-                                         env->regs[1],
-                                         env->regs[2],
-                                         env->regs[3],
-                                         env->regs[4],
-                                         env->regs[5],
-                                         0, 0);
-                        if (ret == -TARGET_ERESTARTSYS) {
-                            env->regs[15] -= env->thumb ? 2 : 4;
-                        } else if (ret != -TARGET_QEMU_ESIGRETURN) {
-                            env->regs[0] = ret;
+                            queue_signal(env, info.si_signo,
+                                         QEMU_SI_FAULT, &info);
                         }
+                        break;
                     }
                 } else {
-                    goto error;
+                    ret = do_syscall(env,
+                                     n,
+                                     env->regs[0],
+                                     env->regs[1],
+                                     env->regs[2],
+                                     env->regs[3],
+                                     env->regs[4],
+                                     env->regs[5],
+                                     0, 0);
+                    if (ret == -TARGET_ERESTARTSYS) {
+                        env->regs[15] -= env->thumb ? 2 : 4;
+                    } else if (ret != -TARGET_QEMU_ESIGRETURN) {
+                        env->regs[0] = ret;
+                    }
                 }
             }
             break;
-- 
2.20.1



  parent reply	other threads:[~2020-05-21 19:19 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-21 19:15 [PULL 00/29] target-arm queue Peter Maydell
2020-05-21 19:15 ` [PULL 01/29] tests/acceptance: Add a test for the canon-a1100 machine Peter Maydell
2020-05-21 19:15 ` [PULL 02/29] docs/system: Add 'Arm' to the Integrator/CP document title Peter Maydell
2020-05-21 19:15 ` [PULL 03/29] docs/system: Sort Arm board index into alphabetical order Peter Maydell
2020-05-21 19:15 ` [PULL 04/29] docs/system: Document Arm Versatile Express boards Peter Maydell
2020-05-21 19:15 ` [PULL 05/29] docs/system: Document the various MPS2 models Peter Maydell
2020-05-21 19:15 ` [PULL 06/29] docs/system: Document Musca boards Peter Maydell
2020-05-21 19:15 ` [PULL 07/29] linux-user/arm: BKPT should cause SIGTRAP, not be a syscall Peter Maydell
2020-05-21 19:15 ` [PULL 08/29] linux-user/arm: Remove bogus SVC 0xf0002 handling Peter Maydell
2020-05-21 19:15 ` [PULL 09/29] linux-user/arm: Handle invalid arm-specific syscalls correctly Peter Maydell
2020-05-21 19:15 ` Peter Maydell [this message]
2020-05-21 19:15 ` [PULL 11/29] target/arm: Remove unused GEN_NEON_INTEGER_OP macro Peter Maydell
2020-05-21 19:15 ` [PULL 12/29] hw: Move i.MX watchdog driver to hw/watchdog Peter Maydell
2020-05-21 19:15 ` [PULL 13/29] hw/watchdog: Implement full i.MX watchdog support Peter Maydell
2020-05-21 19:15 ` [PULL 14/29] hw/arm/fsl-imx25: Wire up watchdog Peter Maydell
2020-05-21 19:15 ` [PULL 15/29] hw/arm/fsl-imx31: " Peter Maydell
2020-05-21 19:15 ` [PULL 16/29] hw/arm/fsl-imx6: Connect watchdog interrupts Peter Maydell
2020-05-21 19:15 ` [PULL 17/29] hw/arm/fsl-imx6ul: " Peter Maydell
2020-05-21 19:15 ` [PULL 18/29] hw/arm/fsl-imx7: Instantiate various unimplemented devices Peter Maydell
2020-05-21 19:16 ` [PULL 19/29] hw/arm/fsl-imx7: Connect watchdog interrupts Peter Maydell
2020-05-21 19:16 ` [PULL 20/29] hw/arm/integratorcp: Replace hw_error() by qemu_log_mask() Peter Maydell
2020-05-21 19:16 ` [PULL 21/29] hw/arm/pxa2xx: " Peter Maydell
2020-05-21 19:16 ` [PULL 22/29] hw/char/xilinx_uartlite: " Peter Maydell
2020-05-21 19:16 ` [PULL 23/29] hw/timer/exynos4210_mct: " Peter Maydell
2020-05-21 19:16 ` [PULL 24/29] ARM: PL061: Introduce N_GPIOS Peter Maydell
2020-05-21 19:16 ` [PULL 25/29] target/arm: Use tcg_gen_gvec_mov for clear_vec_high Peter Maydell
2020-05-21 19:16 ` [PULL 26/29] target/arm: Use clear_vec_high more effectively Peter Maydell
2020-05-21 19:16 ` [PULL 27/29] target/arm: Allow user-mode code to write CPSR.E via MSR Peter Maydell
2020-05-21 19:16 ` [PULL 28/29] linux-user/arm: Reset CPSR_E when entering a signal handler Peter Maydell
2020-05-21 19:16 ` [PULL 29/29] linux-user/arm/signal.c: Drop TARGET_CONFIG_CPU_32 Peter Maydell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200521191610.10941-11-peter.maydell@linaro.org \
    --to=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.