s390/fpu: get rid of test_fp_ctl()

It is quite subtle to use test_fp_ctl() correctly. Therefore remove it -
instead copy whatever new floating point control (fpc) register values are
supposed to be used into its save area.

Test the validity of the new value when loading it. If the new value is
invalid, load the fpc register with zero.

This seems to be a the best way to approach this problem. Even though this
changes behavior:

- sigreturn with an invalid fpc value on the stack will succeed, and
  continue with zero value, instead of returning with SIGSEGV

- ptraced processes will also use a zero value instead of letting the
  request fail with -EINVAL

However all of this seems to acceptable. After all testing of the value was
only implemented to avoid that user space can crash the kernel. It is not
there to test values for validity; and the assumption is that there is no
existing user space which is doing this.

Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
Signed-off-by: Alexander Gordeev <agordeev@linux.ibm.com>

diff --git a/arch/s390/include/asm/fpu/api.h b/arch/s390/include/asm/fpu/api.h
index b714ed0..dc34de2 100644 (file)
--- a/arch/s390/include/asm/fpu/api.h
+++ b/arch/s390/include/asm/fpu/api.h
@@ -51,21 +51,27 @@ void save_fpu_regs(void);
 void load_fpu_regs(void);
 void __load_fpu_regs(void);
 
-static inline int test_fp_ctl(u32 fpc)
+/**
+ * sfpc_safe - Set floating point control register safely.
+ * @fpc: new value for floating point control register
+ *
+ * Set floating point control register. This may lead to an exception,
+ * since a saved value may have been modified by user space (ptrace,
+ * signal return, kvm registers) to an invalid value. In such a case
+ * set the floating point control register to zero.
+ */
+static inline void sfpc_safe(u32 fpc)
 {
-       u32 orig_fpc;
-       int rc;
-
-       asm volatile(
-               "       efpc    %1\n"
-               "       sfpc    %2\n"
-               "0:     sfpc    %1\n"
-               "       la      %0,0\n"
-               "1:\n"
-               EX_TABLE(0b,1b)
-               : "=d" (rc), "=&d" (orig_fpc)
-               : "d" (fpc), "0" (-EINVAL));
-       return rc;
+       asm volatile("\n"
+               "0:     sfpc    %[fpc]\n"
+               "1:     nopr    %%r7\n"
+               ".pushsection .fixup, \"ax\"\n"
+               "2:     lghi    %[fpc],0\n"
+               "       jg      0b\n"
+               ".popsection\n"
+               EX_TABLE(1b, 2b)
+               : [fpc] "+d" (fpc)
+               : : "memory");
 }
 
 #define KERNEL_FPC             1

diff --git a/arch/s390/kernel/compat_signal.c b/arch/s390/kernel/compat_signal.c
index cecedd0..8b90bf8 100644 (file)
--- a/arch/s390/kernel/compat_signal.c
+++ b/arch/s390/kernel/compat_signal.c
@@ -98,10 +98,6 @@ static int restore_sigregs32(struct pt_regs *regs,_sigregs32 __user *sregs)
        if (!is_ri_task(current) && (user_sregs.regs.psw.mask & PSW32_MASK_RI))
                return -EINVAL;
 
-       /* Test the floating-point-control word. */
-       if (test_fp_ctl(user_sregs.fpregs.fpc))
-               return -EINVAL;
-
        /* Use regs->psw.mask instead of PSW_USER_BITS to preserve PER bit. */
        regs->psw.mask = (regs->psw.mask & ~(PSW_MASK_USER | PSW_MASK_RI)) |
                (__u64)(user_sregs.regs.psw.mask & PSW32_MASK_USER) << 32 |

diff --git a/arch/s390/kernel/fpu.c b/arch/s390/kernel/fpu.c
index 4666b29..35aaf99 100644 (file)
--- a/arch/s390/kernel/fpu.c
+++ b/arch/s390/kernel/fpu.c
@@ -177,10 +177,10 @@ EXPORT_SYMBOL(__kernel_fpu_end);
 
 void __load_fpu_regs(void)
 {
-       struct fpu *state = &current->thread.fpu;
        unsigned long *regs = current->thread.fpu.regs;
+       struct fpu *state = &current->thread.fpu;
 
-       asm volatile("lfpc %0" : : "Q" (state->fpc));
+       sfpc_safe(state->fpc);
        if (likely(MACHINE_HAS_VX)) {
                asm volatile("lgr       1,%0\n"
                             "VLM       0,15,0,1\n"

diff --git a/arch/s390/kernel/ptrace.c b/arch/s390/kernel/ptrace.c
index c7ed302..df2ee1b 100644 (file)
--- a/arch/s390/kernel/ptrace.c
+++ b/arch/s390/kernel/ptrace.c
@@ -392,9 +392,7 @@ static int __poke_user(struct task_struct *child, addr_t addr, addr_t data)
                /*
                 * floating point control reg. is in the thread structure
                 */
-               save_fpu_regs();
-               if ((unsigned int) data != 0 ||
-                   test_fp_ctl(data >> (BITS_PER_LONG - 32)))
+               if ((unsigned int)data != 0)
                        return -EINVAL;
                child->thread.fpu.fpc = data >> (BITS_PER_LONG - 32);
 
@@ -749,9 +747,6 @@ static int __poke_user_compat(struct task_struct *child,
                /*
                 * floating point control reg. is in the thread structure
                 */
-               save_fpu_regs();
-               if (test_fp_ctl(tmp))
-                       return -EINVAL;
                child->thread.fpu.fpc = data;
 
        } else if (addr < offsetof(struct compat_user, regs.fp_regs) + sizeof(s390_fp_regs)) {
@@ -913,7 +908,9 @@ static int s390_fpregs_set(struct task_struct *target,
        int rc = 0;
        freg_t fprs[__NUM_FPRS];
 
-       save_fpu_regs();
+       if (target == current)
+               save_fpu_regs();
+
        if (MACHINE_HAS_VX)
                convert_vx_to_fp(fprs, target->thread.fpu.vxrs);
        else
@@ -926,7 +923,7 @@ static int s390_fpregs_set(struct task_struct *target,
                                        0, offsetof(s390_fp_regs, fprs));
                if (rc)
                        return rc;
-               if (ufpc[1] != 0 || test_fp_ctl(ufpc[0]))
+               if (ufpc[1] != 0)
                        return -EINVAL;
                target->thread.fpu.fpc = ufpc[0];
        }

diff --git a/arch/s390/kernel/signal.c b/arch/s390/kernel/signal.c
index d63557d..0e926e8 100644 (file)
--- a/arch/s390/kernel/signal.c
+++ b/arch/s390/kernel/signal.c
@@ -149,10 +149,6 @@ static int restore_sigregs(struct pt_regs *regs, _sigregs __user *sregs)
        if (!is_ri_task(current) && (user_sregs.regs.psw.mask & PSW_MASK_RI))
                return -EINVAL;
 
-       /* Test the floating-point-control word. */
-       if (test_fp_ctl(user_sregs.fpregs.fpc))
-               return -EINVAL;
-
        /* Use regs->psw.mask instead of PSW_USER_BITS to preserve PER bit. */
        regs->psw.mask = (regs->psw.mask & ~(PSW_MASK_USER | PSW_MASK_RI)) |
                (user_sregs.regs.psw.mask & (PSW_MASK_USER | PSW_MASK_RI));

diff --git a/arch/s390/kernel/vmlinux.lds.S b/arch/s390/kernel/vmlinux.lds.S
index 2ae201e..e32ef44 100644 (file)
--- a/arch/s390/kernel/vmlinux.lds.S
+++ b/arch/s390/kernel/vmlinux.lds.S
@@ -52,6 +52,7 @@ SECTIONS
                SOFTIRQENTRY_TEXT
                FTRACE_HOTPATCH_TRAMPOLINES_TEXT
                *(.text.*_indirect_*)
+               *(.fixup)
                *(.gnu.warning)
                . = ALIGN(PAGE_SIZE);
                _etext = .;             /* End of text section */

diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 1a1af4d..432688a 100644 (file)
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -4962,10 +4962,7 @@ static void sync_regs(struct kvm_vcpu *vcpu)
                current->thread.fpu.regs = vcpu->run->s.regs.vrs;
        else
                current->thread.fpu.regs = vcpu->run->s.regs.fprs;
-       current->thread.fpu.fpc = READ_ONCE(vcpu->run->s.regs.fpc);
-       if (test_fp_ctl(current->thread.fpu.fpc))
-               /* User space provided an invalid FPC, let's clear it */
-               current->thread.fpu.fpc = 0;
+       current->thread.fpu.fpc = vcpu->run->s.regs.fpc;
 
        /* Sync fmt2 only data */
        if (likely(!kvm_s390_pv_cpu_is_protected(vcpu))) {