powerpc/ptrace: Split gpr32_set_common
authorChristophe Leroy <christophe.leroy@csgroup.eu>
Thu, 22 Jun 2023 10:01:23 +0000 (12:01 +0200)
committerMichael Ellerman <mpe@ellerman.id.au>
Wed, 16 Aug 2023 13:54:50 +0000 (23:54 +1000)
objtool reports the following warning:

  arch/powerpc/kernel/ptrace/ptrace-view.o: warning: objtool:
    gpr32_set_common+0x23c (.text+0x860): redundant UACCESS disable

gpr32_set_common() conditionally opens and closes UACCESS based on
whether kbuf pointer is NULL or not. This is wackelig.

Split gpr32_set_common() in two fonctions, one for user one for
kernel.

Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
[mpe: Fix oops in gpr32_set_common_user() due to NULL kbuf]
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Link: https://msgid.link/b8d6ae4483fcfd17524e79d803c969694a85cc02.1687428075.git.christophe.leroy@csgroup.eu
arch/powerpc/kernel/ptrace/ptrace-view.c

index 3910cd7..584cf5c 100644 (file)
@@ -716,69 +716,86 @@ int gpr32_get_common(struct task_struct *target,
        return membuf_zero(&to, (ELF_NGREG - PT_REGS_COUNT) * sizeof(u32));
 }
 
-int gpr32_set_common(struct task_struct *target,
-                    const struct user_regset *regset,
-                    unsigned int pos, unsigned int count,
-                    const void *kbuf, const void __user *ubuf,
-                    unsigned long *regs)
+static int gpr32_set_common_kernel(struct task_struct *target,
+                                  const struct user_regset *regset,
+                                  unsigned int pos, unsigned int count,
+                                  const void *kbuf, unsigned long *regs)
 {
        const compat_ulong_t *k = kbuf;
+
+       pos /= sizeof(compat_ulong_t);
+       count /= sizeof(compat_ulong_t);
+
+       for (; count > 0 && pos < PT_MSR; --count)
+               regs[pos++] = *k++;
+
+       if (count > 0 && pos == PT_MSR) {
+               set_user_msr(target, *k++);
+               ++pos;
+               --count;
+       }
+
+       for (; count > 0 && pos <= PT_MAX_PUT_REG; --count)
+               regs[pos++] = *k++;
+       for (; count > 0 && pos < PT_TRAP; --count, ++pos)
+               ++k;
+
+       if (count > 0 && pos == PT_TRAP) {
+               set_user_trap(target, *k++);
+               ++pos;
+               --count;
+       }
+
+       kbuf = k;
+       pos *= sizeof(compat_ulong_t);
+       count *= sizeof(compat_ulong_t);
+       user_regset_copyin_ignore(&pos, &count, &kbuf, NULL,
+                                 (PT_TRAP + 1) * sizeof(compat_ulong_t), -1);
+       return 0;
+}
+
+static int gpr32_set_common_user(struct task_struct *target,
+                                const struct user_regset *regset,
+                                unsigned int pos, unsigned int count,
+                                const void __user *ubuf, unsigned long *regs)
+{
        const compat_ulong_t __user *u = ubuf;
+       const void *kbuf = NULL;
        compat_ulong_t reg;
 
-       if (!kbuf && !user_read_access_begin(u, count))
+       if (!user_read_access_begin(u, count))
                return -EFAULT;
 
        pos /= sizeof(reg);
        count /= sizeof(reg);
 
-       if (kbuf)
-               for (; count > 0 && pos < PT_MSR; --count)
-                       regs[pos++] = *k++;
-       else
-               for (; count > 0 && pos < PT_MSR; --count) {
-                       unsafe_get_user(reg, u++, Efault);
-                       regs[pos++] = reg;
-               }
-
+       for (; count > 0 && pos < PT_MSR; --count) {
+               unsafe_get_user(reg, u++, Efault);
+               regs[pos++] = reg;
+       }
 
        if (count > 0 && pos == PT_MSR) {
-               if (kbuf)
-                       reg = *k++;
-               else
-                       unsafe_get_user(reg, u++, Efault);
+               unsafe_get_user(reg, u++, Efault);
                set_user_msr(target, reg);
                ++pos;
                --count;
        }
 
-       if (kbuf) {
-               for (; count > 0 && pos <= PT_MAX_PUT_REG; --count)
-                       regs[pos++] = *k++;
-               for (; count > 0 && pos < PT_TRAP; --count, ++pos)
-                       ++k;
-       } else {
-               for (; count > 0 && pos <= PT_MAX_PUT_REG; --count) {
-                       unsafe_get_user(reg, u++, Efault);
-                       regs[pos++] = reg;
-               }
-               for (; count > 0 && pos < PT_TRAP; --count, ++pos)
-                       unsafe_get_user(reg, u++, Efault);
+       for (; count > 0 && pos <= PT_MAX_PUT_REG; --count) {
+               unsafe_get_user(reg, u++, Efault);
+               regs[pos++] = reg;
        }
+       for (; count > 0 && pos < PT_TRAP; --count, ++pos)
+               unsafe_get_user(reg, u++, Efault);
 
        if (count > 0 && pos == PT_TRAP) {
-               if (kbuf)
-                       reg = *k++;
-               else
-                       unsafe_get_user(reg, u++, Efault);
+               unsafe_get_user(reg, u++, Efault);
                set_user_trap(target, reg);
                ++pos;
                --count;
        }
-       if (!kbuf)
-               user_read_access_end();
+       user_read_access_end();
 
-       kbuf = k;
        ubuf = u;
        pos *= sizeof(reg);
        count *= sizeof(reg);
@@ -791,6 +808,18 @@ Efault:
        return -EFAULT;
 }
 
+int gpr32_set_common(struct task_struct *target,
+                    const struct user_regset *regset,
+                    unsigned int pos, unsigned int count,
+                    const void *kbuf, const void __user *ubuf,
+                    unsigned long *regs)
+{
+       if (kbuf)
+               return gpr32_set_common_kernel(target, regset, pos, count, kbuf, regs);
+       else
+               return gpr32_set_common_user(target, regset, pos, count, ubuf, regs);
+}
+
 static int gpr32_get(struct task_struct *target,
                     const struct user_regset *regset,
                     struct membuf to)