Patches 1-3 are cleaning parts of uaccess.h not related
to put_user/get_user
Patch 4 removes some usage of consecutives __get_user
Patches 5 rewrite __patch_instruction to not use uaccess.h internals.
Patches 6-8 switch some parts of code to user_access_begin/end blocks
All patches are independant.
Christophe Leroy (8):
powerpc/uaccess: Also perform 64 bits copies in unsafe_copy_to_user()
on ppc32
powerpc/uaccess: Swap clear_user() and __clear_user()
powerpc/uaccess: Move copy_mc_xxx() functions down
powerpc/syscalls: Use sys_old_select() in ppc_select()
powerpc/lib: Don't use __put_user_asm_goto() outside of uaccess.h
powerpc/net: Switch csum_and_copy_{to/from}_user to user_access block
powerpc/futex: Switch to user_access block
powerpc/ptrace: Convert gpr32_set_common() to user access block
arch/powerpc/include/asm/futex.h | 12 ++--
arch/powerpc/include/asm/ptrace.h | 2 +-
arch/powerpc/include/asm/uaccess.h | 75 ++++++++++++------------
arch/powerpc/include/asm/unistd.h | 1 +
arch/powerpc/kernel/ptrace/ptrace-view.c | 30 ++++++----
arch/powerpc/kernel/syscalls.c | 12 +---
arch/powerpc/lib/checksum_wrappers.c | 15 ++---
arch/powerpc/lib/code-patching.c | 13 ++--
8 files changed, 77 insertions(+), 83 deletions(-)
--
2.25.0
Instead of opencodying the copy of parameters, use
the generic sys_old_select().
Signed-off-by: Christophe Leroy <[email protected]>
---
arch/powerpc/include/asm/unistd.h | 1 +
arch/powerpc/kernel/syscalls.c | 12 ++----------
2 files changed, 3 insertions(+), 10 deletions(-)
diff --git a/arch/powerpc/include/asm/unistd.h b/arch/powerpc/include/asm/unistd.h
index 700fcdac2e3c..b541c690a31c 100644
--- a/arch/powerpc/include/asm/unistd.h
+++ b/arch/powerpc/include/asm/unistd.h
@@ -40,6 +40,7 @@
#define __ARCH_WANT_SYS_SIGPROCMASK
#ifdef CONFIG_PPC32
#define __ARCH_WANT_OLD_STAT
+#define __ARCH_WANT_SYS_OLD_SELECT
#endif
#ifdef CONFIG_PPC64
#define __ARCH_WANT_SYS_TIME
diff --git a/arch/powerpc/kernel/syscalls.c b/arch/powerpc/kernel/syscalls.c
index 078608ec2e92..a552c9e68d7e 100644
--- a/arch/powerpc/kernel/syscalls.c
+++ b/arch/powerpc/kernel/syscalls.c
@@ -82,16 +82,8 @@ int
ppc_select(int n, fd_set __user *inp, fd_set __user *outp, fd_set __user *exp, struct __kernel_old_timeval __user *tvp)
{
if ( (unsigned long)n >= 4096 )
- {
- unsigned long __user *buffer = (unsigned long __user *)n;
- if (!access_ok(buffer, 5*sizeof(unsigned long))
- || __get_user(n, buffer)
- || __get_user(inp, ((fd_set __user * __user *)(buffer+1)))
- || __get_user(outp, ((fd_set __user * __user *)(buffer+2)))
- || __get_user(exp, ((fd_set __user * __user *)(buffer+3)))
- || __get_user(tvp, ((struct __kernel_old_timeval __user * __user *)(buffer+4))))
- return -EFAULT;
- }
+ return sys_old_select((void __user *)n);
+
return sys_select(n, inp, outp, exp, tvp);
}
#endif
--
2.25.0
It is clear_user() which is expected to call __clear_user(),
not the reverse.
Signed-off-by: Christophe Leroy <[email protected]>
---
arch/powerpc/include/asm/uaccess.h | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
index 2c09cff205ef..1c1d404514b1 100644
--- a/arch/powerpc/include/asm/uaccess.h
+++ b/arch/powerpc/include/asm/uaccess.h
@@ -414,21 +414,20 @@ raw_copy_to_user(void __user *to, const void *from, unsigned long n)
unsigned long __arch_clear_user(void __user *addr, unsigned long size);
-static inline unsigned long clear_user(void __user *addr, unsigned long size)
+static inline unsigned long __clear_user(void __user *addr, unsigned long size)
{
- unsigned long ret = size;
+ unsigned long ret;
+
might_fault();
- if (likely(access_ok(addr, size))) {
- allow_write_to_user(addr, size);
- ret = __arch_clear_user(addr, size);
- prevent_write_to_user(addr, size);
- }
+ allow_write_to_user(addr, size);
+ ret = __arch_clear_user(addr, size);
+ prevent_write_to_user(addr, size);
return ret;
}
-static inline unsigned long __clear_user(void __user *addr, unsigned long size)
+static inline unsigned long clear_user(void __user *addr, unsigned long size)
{
- return clear_user(addr, size);
+ return likely(access_ok(addr, size)) ? __clear_user(addr, size) : size;
}
extern long strncpy_from_user(char *dst, const char __user *src, long count);
--
2.25.0
__put_user_asm_goto() is internal to uaccess.h
Use __put_kernel_nofault() instead. The generated code is identical.
Signed-off-by: Christophe Leroy <[email protected]>
---
arch/powerpc/lib/code-patching.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
index 2333625b5e31..65aec4d6d9ba 100644
--- a/arch/powerpc/lib/code-patching.c
+++ b/arch/powerpc/lib/code-patching.c
@@ -21,10 +21,15 @@
static int __patch_instruction(struct ppc_inst *exec_addr, struct ppc_inst instr,
struct ppc_inst *patch_addr)
{
- if (!ppc_inst_prefixed(instr))
- __put_user_asm_goto(ppc_inst_val(instr), patch_addr, failed, "stw");
- else
- __put_user_asm_goto(ppc_inst_as_u64(instr), patch_addr, failed, "std");
+ if (!ppc_inst_prefixed(instr)) {
+ u32 val = ppc_inst_val(instr);
+
+ __put_kernel_nofault(patch_addr, &val, u32, failed);
+ } else {
+ u64 val = ppc_inst_as_u64(instr);
+
+ __put_kernel_nofault(patch_addr, &val, u64, failed);
+ }
asm ("dcbst 0, %0; sync; icbi 0,%1; sync; isync" :: "r" (patch_addr),
"r" (exec_addr));
--
2.25.0
Use user access block in gpr32_set_common() instead of
repetitive __get_user() which imply repetitive KUAP open/close.
To get it clean, force inlining of the small set of tiny functions
called inside the block.
Signed-off-by: Christophe Leroy <[email protected]>
---
arch/powerpc/include/asm/ptrace.h | 2 +-
arch/powerpc/kernel/ptrace/ptrace-view.c | 30 ++++++++++++++----------
2 files changed, 19 insertions(+), 13 deletions(-)
diff --git a/arch/powerpc/include/asm/ptrace.h b/arch/powerpc/include/asm/ptrace.h
index 975ba260006a..cb154fb7b605 100644
--- a/arch/powerpc/include/asm/ptrace.h
+++ b/arch/powerpc/include/asm/ptrace.h
@@ -222,7 +222,7 @@ do { \
} while (0)
#endif /* __powerpc64__ */
-static inline void set_trap(struct pt_regs *regs, unsigned long val)
+static __always_inline void set_trap(struct pt_regs *regs, unsigned long val)
{
regs->trap = (regs->trap & TRAP_FLAGS_MASK) | (val & ~TRAP_FLAGS_MASK);
}
diff --git a/arch/powerpc/kernel/ptrace/ptrace-view.c b/arch/powerpc/kernel/ptrace/ptrace-view.c
index 2bad8068f598..0923c94f684e 100644
--- a/arch/powerpc/kernel/ptrace/ptrace-view.c
+++ b/arch/powerpc/kernel/ptrace/ptrace-view.c
@@ -111,7 +111,7 @@ static unsigned long get_user_msr(struct task_struct *task)
return task->thread.regs->msr | task->thread.fpexc_mode;
}
-static int set_user_msr(struct task_struct *task, unsigned long msr)
+static __always_inline int set_user_msr(struct task_struct *task, unsigned long msr)
{
task->thread.regs->msr &= ~MSR_DEBUGCHANGE;
task->thread.regs->msr |= msr & MSR_DEBUGCHANGE;
@@ -147,7 +147,7 @@ static int set_user_dscr(struct task_struct *task, unsigned long dscr)
* We prevent mucking around with the reserved area of trap
* which are used internally by the kernel.
*/
-static int set_user_trap(struct task_struct *task, unsigned long trap)
+static __always_inline int set_user_trap(struct task_struct *task, unsigned long trap)
{
set_trap(task->thread.regs, trap);
return 0;
@@ -661,6 +661,9 @@ int gpr32_set_common(struct task_struct *target,
const compat_ulong_t __user *u = ubuf;
compat_ulong_t reg;
+ if (!kbuf && !user_read_access_begin(u, count))
+ return -EFAULT;
+
pos /= sizeof(reg);
count /= sizeof(reg);
@@ -669,8 +672,7 @@ int gpr32_set_common(struct task_struct *target,
regs[pos++] = *k++;
else
for (; count > 0 && pos < PT_MSR; --count) {
- if (__get_user(reg, u++))
- return -EFAULT;
+ unsafe_get_user(reg, u++, Efault);
regs[pos++] = reg;
}
@@ -678,8 +680,8 @@ int gpr32_set_common(struct task_struct *target,
if (count > 0 && pos == PT_MSR) {
if (kbuf)
reg = *k++;
- else if (__get_user(reg, u++))
- return -EFAULT;
+ else
+ unsafe_get_user(reg, u++, Efault);
set_user_msr(target, reg);
++pos;
--count;
@@ -692,24 +694,24 @@ int gpr32_set_common(struct task_struct *target,
++k;
} else {
for (; count > 0 && pos <= PT_MAX_PUT_REG; --count) {
- if (__get_user(reg, u++))
- return -EFAULT;
+ unsafe_get_user(reg, u++, Efault);
regs[pos++] = reg;
}
for (; count > 0 && pos < PT_TRAP; --count, ++pos)
- if (__get_user(reg, u++))
- return -EFAULT;
+ unsafe_get_user(reg, u++, Efault);
}
if (count > 0 && pos == PT_TRAP) {
if (kbuf)
reg = *k++;
- else if (__get_user(reg, u++))
- return -EFAULT;
+ else
+ unsafe_get_user(reg, u++, Efault);
set_user_trap(target, reg);
++pos;
--count;
}
+ if (!kbuf)
+ user_read_access_end();
kbuf = k;
ubuf = u;
@@ -717,6 +719,10 @@ int gpr32_set_common(struct task_struct *target,
count *= sizeof(reg);
return user_regset_copyin_ignore(&pos, &count, &kbuf, &ubuf,
(PT_TRAP + 1) * sizeof(reg), -1);
+
+Efault:
+ user_read_access_end();
+ return -EFAULT;
}
static int gpr32_get(struct task_struct *target,
--
2.25.0
Use user_access_begin() instead of the
might_sleep/access_ok/allow_access sequence.
Signed-off-by: Christophe Leroy <[email protected]>
---
arch/powerpc/lib/checksum_wrappers.c | 15 ++++-----------
1 file changed, 4 insertions(+), 11 deletions(-)
diff --git a/arch/powerpc/lib/checksum_wrappers.c b/arch/powerpc/lib/checksum_wrappers.c
index b895166afc82..f3999cbb2fcc 100644
--- a/arch/powerpc/lib/checksum_wrappers.c
+++ b/arch/powerpc/lib/checksum_wrappers.c
@@ -16,16 +16,12 @@ __wsum csum_and_copy_from_user(const void __user *src, void *dst,
{
__wsum csum;
- might_sleep();
-
- if (unlikely(!access_ok(src, len)))
+ if (unlikely(!user_read_access_begin(src, len)))
return 0;
- allow_read_from_user(src, len);
-
csum = csum_partial_copy_generic((void __force *)src, dst, len);
- prevent_read_from_user(src, len);
+ user_read_access_end();
return csum;
}
EXPORT_SYMBOL(csum_and_copy_from_user);
@@ -34,15 +30,12 @@ __wsum csum_and_copy_to_user(const void *src, void __user *dst, int len)
{
__wsum csum;
- might_sleep();
- if (unlikely(!access_ok(dst, len)))
+ if (unlikely(!user_write_access_begin(dst, len)))
return 0;
- allow_write_to_user(dst, len);
-
csum = csum_partial_copy_generic(src, (void __force *)dst, len);
- prevent_write_to_user(dst, len);
+ user_write_access_end();
return csum;
}
EXPORT_SYMBOL(csum_and_copy_to_user);
--
2.25.0
Use user_access_begin() instead of the access_ok/allow_access sequence.
This brings the missing might_fault() check.
Signed-off-by: Christophe Leroy <[email protected]>
---
arch/powerpc/include/asm/futex.h | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/arch/powerpc/include/asm/futex.h b/arch/powerpc/include/asm/futex.h
index e93ee3202e4c..b3001f8b2c1e 100644
--- a/arch/powerpc/include/asm/futex.h
+++ b/arch/powerpc/include/asm/futex.h
@@ -33,9 +33,8 @@ static inline int arch_futex_atomic_op_inuser(int op, int oparg, int *oval,
{
int oldval = 0, ret;
- if (!access_ok(uaddr, sizeof(u32)))
+ if (!user_access_begin(uaddr, sizeof(u32)))
return -EFAULT;
- allow_read_write_user(uaddr, uaddr, sizeof(*uaddr));
switch (op) {
case FUTEX_OP_SET:
@@ -56,10 +55,10 @@ static inline int arch_futex_atomic_op_inuser(int op, int oparg, int *oval,
default:
ret = -ENOSYS;
}
+ user_access_end();
*oval = oldval;
- prevent_read_write_user(uaddr, uaddr, sizeof(*uaddr));
return ret;
}
@@ -70,11 +69,9 @@ futex_atomic_cmpxchg_inatomic(u32 *uval, u32 __user *uaddr,
int ret = 0;
u32 prev;
- if (!access_ok(uaddr, sizeof(u32)))
+ if (!user_access_begin(uaddr, sizeof(u32)))
return -EFAULT;
- allow_read_write_user(uaddr, uaddr, sizeof(*uaddr));
-
__asm__ __volatile__ (
PPC_ATOMIC_ENTRY_BARRIER
"1: lwarx %1,0,%3 # futex_atomic_cmpxchg_inatomic\n\
@@ -93,8 +90,9 @@ futex_atomic_cmpxchg_inatomic(u32 *uval, u32 __user *uaddr,
: "r" (uaddr), "r" (oldval), "r" (newval), "i" (-EFAULT)
: "cc", "memory");
+ user_access_end();
+
*uval = prev;
- prevent_read_write_user(uaddr, uaddr, sizeof(*uaddr));
return ret;
}
--
2.25.0
On Wed, 10 Mar 2021 17:56:59 +0000 (UTC), Christophe Leroy wrote:
> Patches 1-3 are cleaning parts of uaccess.h not related
> to put_user/get_user
> Patch 4 removes some usage of consecutives __get_user
> Patches 5 rewrite __patch_instruction to not use uaccess.h internals.
> Patches 6-8 switch some parts of code to user_access_begin/end blocks
>
> All patches are independant.
>
> [...]
Applied to powerpc/next.
[1/8] powerpc/uaccess: Also perform 64 bits copies in unsafe_copy_to_user() on ppc32
https://git.kernel.org/powerpc/c/c6adc835c68b713360f918d21372c2f34fc228e2
[2/8] powerpc/uaccess: Swap clear_user() and __clear_user()
https://git.kernel.org/powerpc/c/7472199a6eda6a79f9e3b126f52f67f9ce3e1f77
[3/8] powerpc/uaccess: Move copy_mc_xxx() functions down
https://git.kernel.org/powerpc/c/4b8cda58812c1e1bf79d37f2ddff3cf03b7025da
[4/8] powerpc/syscalls: Use sys_old_select() in ppc_select()
https://git.kernel.org/powerpc/c/fd69d544b0e785b11699675154bdfe01a04538cd
[5/8] powerpc/lib: Don't use __put_user_asm_goto() outside of uaccess.h
https://git.kernel.org/powerpc/c/e63ceebdad82f85e48b018abfc6af4ed6958179e
[6/8] powerpc/net: Switch csum_and_copy_{to/from}_user to user_access block
https://git.kernel.org/powerpc/c/164dc6ce368fa23b0aae0e5d12883fff9bf80458
[7/8] powerpc/futex: Switch to user_access block
https://git.kernel.org/powerpc/c/870779f40e99c795ddfafa0dfc43318e51f15127
[8/8] powerpc/ptrace: Convert gpr32_set_common() to user access block
https://git.kernel.org/powerpc/c/93c043e393af7fa218c928d8c62396ba28f1bb84
cheers