Re: [MIPS] Check FCSR for pending interrupts before restoring from a context.

[Atsushi Nemoto <anemo@mba.ocn.ne.jp>]

On Thu, 08 Feb 2007 12:02:19 +0900 (JST), Atsushi Nemoto <anemo@mba.ocn.ne.jp> 
wrote:
> So we can do it in C, but it will conflicts with Franck's signal
> cleanup patchset.  I hope his patchset applied first.
> 
> Also I wonder SIGSEGV is correct behavior on this case.  SIGFPE?

And I found that if signal handler set FPU_CSR_UNI_X, kernel complains
"FP exception in kernel code" ...

Here is a first cut.  Changes in r4k_fpu.S can be reverted, and after
Franck's patchset applied, this patch can be a bit smaller.  Please
review.

Note that calling __get_user(), __put_user() might lose FPU ownership.
But restore_fp_context() already has this breakage.  I already sent
two alternative patches to fix this longstanding issue ("[PATCH]
rewrite restore_fp_context/save_fp_context" and "[PATCH 2/3] Allow CpU
exception in kernel partially").


diff --git a/arch/mips/kernel/signal-common.h b/arch/mips/kernel/signal-common.h
index b1f09d5..0811298 100644
--- a/arch/mips/kernel/signal-common.h
+++ b/arch/mips/kernel/signal-common.h
@@ -66,6 +66,38 @@ out:
 }
 
 static inline int
+fpcsr_pending(unsigned int __user *fpcsr)
+{
+       int err, sig = 0;
+       unsigned int csr, enabled;
+
+       err = __get_user(csr, fpcsr);
+       enabled = FPU_CSR_UNI_X | ((csr & FPU_CSR_ALL_E) << 5);
+       /*
+        * If the signal handler set some FPU exceptions, clear it and
+        * send SIGFPE.
+        */
+       if (csr & enabled) {
+               csr &= ~enabled;
+               err |= __put_user(csr, fpcsr);
+               sig = SIGFPE;
+       }
+       return err ?: sig;
+}
+
+static inline int
+check_and_restore_fp_context(struct sigcontext __user *sc)
+{
+       int err, sig;
+
+       err = sig = fpcsr_pending(&sc->sc_fpc_csr);
+       if (err > 0)
+               err = 0;
+       err |= restore_fp_context(sc);
+       return err ?: sig;
+}
+
+static inline int
 restore_sigcontext(struct pt_regs *regs, struct sigcontext __user *sc)
 {
        unsigned int used_math;
@@ -112,7 +144,8 @@ restore_sigcontext(struct pt_regs *regs,
        if (used_math()) {
                /* restore fpu context if we have used it before */
                own_fpu();
-               err |= restore_fp_context(sc);
+               if (!err)
+                       err = check_and_restore_fp_context(sc);
        } else {
                /* signal handler may have used FPU.  Give it up. */
                lose_fpu();
diff --git a/arch/mips/kernel/signal.c b/arch/mips/kernel/signal.c
index 9a44053..31d8ec0 100644
--- a/arch/mips/kernel/signal.c
+++ b/arch/mips/kernel/signal.c
@@ -190,6 +190,7 @@ _sys_sigreturn(nabi_no_regargs struct pt
 {
        struct sigframe __user *frame;
        sigset_t blocked;
+       int sig;
 
        frame = (struct sigframe __user *) regs.regs[29];
        if (!access_ok(VERIFY_READ, frame, sizeof(*frame)))
@@ -203,8 +204,12 @@ _sys_sigreturn(nabi_no_regargs struct pt
        recalc_sigpending();
        spin_unlock_irq(&current->sighand->siglock);
 
-       if (restore_sigcontext(&regs, &frame->sf_sc))
+       sig = restore_sigcontext(&regs, &frame->sf_sc);
                goto badframe;
+       if (sig < 0)
+               goto badframe;
+       else if (sig)
+               force_sig(sig, current);
 
        /*
         * Don't let your children do this ...
@@ -228,6 +233,7 @@ _sys_rt_sigreturn(nabi_no_regargs struct
        struct rt_sigframe __user *frame;
        sigset_t set;
        stack_t st;
+       int sig;
 
        frame = (struct rt_sigframe __user *) regs.regs[29];
        if (!access_ok(VERIFY_READ, frame, sizeof(*frame)))
@@ -241,8 +247,11 @@ _sys_rt_sigreturn(nabi_no_regargs struct
        recalc_sigpending();
        spin_unlock_irq(&current->sighand->siglock);
 
-       if (restore_sigcontext(&regs, &frame->rs_uc.uc_mcontext))
+       sig = restore_sigcontext(&regs, &frame->rs_uc.uc_mcontext);
+       if (sig < 0)
                goto badframe;
+       else if (sig)
+               force_sig(sig, current);
 
        if (__copy_from_user(&st, &frame->rs_uc.uc_stack, sizeof(st)))
                goto badframe;
diff --git a/arch/mips/kernel/signal32.c b/arch/mips/kernel/signal32.c
index c86a5dd..51eb21d 100644
--- a/arch/mips/kernel/signal32.c
+++ b/arch/mips/kernel/signal32.c
@@ -326,6 +326,38 @@ asmlinkage int sys32_sigaltstack(nabi_no
        return ret;
 }
 
+static inline int
+fpcsr_pending(unsigned int __user *fpcsr)
+{
+       int err, sig = 0;
+       unsigned int csr, enabled;
+
+       err = __get_user(csr, fpcsr);
+       enabled = FPU_CSR_UNI_X | ((csr & FPU_CSR_ALL_E) << 5);
+       /*
+        * If the signal handler set some FPU exceptions, clear it and
+        * send SIGFPE.
+        */
+       if (csr & enabled) {
+               csr &= ~enabled;
+               err |= __put_user(csr, fpcsr);
+               sig = SIGFPE;
+       }
+       return err ?: sig;
+}
+
+static inline int
+check_and_restore_fp_context32(struct sigcontext32 __user *sc)
+{
+       int err, sig;
+
+       sig = fpcsr_pending(&sc->sc_fpc_csr);
+       if (sig < 0)
+               err = sig;
+       err |= restore_fp_context32(sc);
+       return err ?: sig;
+}
+
 static int restore_sigcontext32(struct pt_regs *regs, struct sigcontext32 
__user *sc)
 {
        u32 used_math;
@@ -372,7 +404,8 @@ static int restore_sigcontext32(struct p
        if (used_math()) {
                /* restore fpu context if we have used it before */
                own_fpu();
-               err |= restore_fp_context32(sc);
+               if (!err)
+                       err = check_and_restore_fp_context(sc);
        } else {
                /* signal handler may have used FPU.  Give it up. */
                lose_fpu();
@@ -469,6 +502,7 @@ _sys32_sigreturn(nabi_no_regargs struct
 {
        struct sigframe __user *frame;
        sigset_t blocked;
+       int sig;
 
        frame = (struct sigframe __user *) regs.regs[29];
        if (!access_ok(VERIFY_READ, frame, sizeof(*frame)))
@@ -482,8 +516,11 @@ _sys32_sigreturn(nabi_no_regargs struct
        recalc_sigpending();
        spin_unlock_irq(&current->sighand->siglock);
 
-       if (restore_sigcontext32(&regs, &frame->sf_sc))
+       sig = restore_sigcontext32(&regs, &frame->sf_sc);
+       if (sig < 0)
                goto badframe;
+       else if (sig)
+               force_sig(sig, current);
 
        /*
         * Don't let your children do this ...