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 b714ed0ef68853b734bdec7d52e59af5d587e5bc..dc34de234ed71a3c9fefacf8fbd5a32d6a851e4c 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 cecedd01d4eca2ca70cb4115c1e741da692fd1f3..8b90bf89b62c4a4970e54a220903d390be9fa51f 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 4666b29ac8a1eb77c5807a6fe9a83fd6046fb1b9..35aaf99bab168163aa33e030546292b4088bae69 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 c7ed302a6b59edceadb8941df006727bf6f8042f..df2ee1b8802483f2bb6b18ed708856783a5a52f2 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 d63557d3868c1bd1dfd423f2e2ee236ace816703..0e926e8968080748d08a36eab8bdf776b8906364 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 2ae201ebf90b97835305a36d94792f9101f409ec..e32ef446f45115faede66cc0d11c32d9440eb73b 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 1a1af4db5afc7f99d997a29a16462b378932f538..432688acc52340fcf62f261e16b5e659953e776f 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))) {