Messages in this thread | | | Subject | Re: [PATCH] powerpc/signal32: Use fault_in_pages_readable() to prefault user context | From | Christophe LEROY <> | Date | Tue, 24 Apr 2018 18:05:12 +0200 |
| |
Le 24/04/2018 à 16:50, Mathieu Malaterre a écrit : > On Tue, Apr 24, 2018 at 4:45 PM, Mathieu Malaterre <malat@debian.org> wrote: >> On Tue, Apr 24, 2018 at 4:17 PM, Christophe Leroy >> <christophe.leroy@c-s.fr> wrote: >>> Use fault_in_pages_readable() to prefault user context >>> instead of open coding >>> >>> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr> >>> --- >>> arch/powerpc/kernel/signal_32.c | 13 +++++-------- >>> 1 file changed, 5 insertions(+), 8 deletions(-) >>> >>> diff --git a/arch/powerpc/kernel/signal_32.c b/arch/powerpc/kernel/signal_32.c >>> index 492f03451877..cfacb2726152 100644 >>> --- a/arch/powerpc/kernel/signal_32.c >>> +++ b/arch/powerpc/kernel/signal_32.c >>> @@ -25,6 +25,7 @@ >>> #include <linux/errno.h> >>> #include <linux/elf.h> >>> #include <linux/ptrace.h> >>> +#include <linux/pagemap.h> >>> #include <linux/ratelimit.h> >>> #ifdef CONFIG_PPC64 >>> #include <linux/syscalls.h> >>> @@ -1045,7 +1046,6 @@ long sys_swapcontext(struct ucontext __user *old_ctx, >>> struct ucontext __user *new_ctx, >>> int ctx_size, int r6, int r7, int r8, struct pt_regs *regs) >>> { >>> - unsigned char tmp __maybe_unused; >>> int ctx_has_vsx_region = 0; >>> >>> #ifdef CONFIG_PPC64 >>> @@ -1109,9 +1109,8 @@ long sys_swapcontext(struct ucontext __user *old_ctx, >>> } >>> if (new_ctx == NULL) >>> return 0; >>> - if (!access_ok(VERIFY_READ, new_ctx, ctx_size) >>> - || __get_user(tmp, (u8 __user *) new_ctx) >>> - || __get_user(tmp, (u8 __user *) new_ctx + ctx_size - 1)) >>> + if (!access_ok(VERIFY_READ, new_ctx, ctx_size) || >>> + fault_in_pages_readable((u8 __user *)new_ctx, ctx_size)) >>> return -EFAULT; >>> >>> /* >>> @@ -1231,7 +1230,6 @@ int sys_debug_setcontext(struct ucontext __user *ctx, >>> { >>> struct sig_dbg_op op; >>> int i; >>> - unsigned char tmp __maybe_unused; >>> unsigned long new_msr = regs->msr; >>> #ifdef CONFIG_PPC_ADV_DEBUG_REGS >>> unsigned long new_dbcr0 = current->thread.debug.dbcr0; >>> @@ -1287,9 +1285,8 @@ int sys_debug_setcontext(struct ucontext __user *ctx, >>> current->thread.debug.dbcr0 = new_dbcr0; >>> #endif >>> >>> - if (!access_ok(VERIFY_READ, ctx, sizeof(*ctx)) >>> - || __get_user(tmp, (u8 __user *) ctx) >>> - || __get_user(tmp, (u8 __user *) (ctx + 1) - 1)) >>> + if (!access_ok(VERIFY_READ, ctx, sizeof(*ctx)) || >>> + fault_in_pages_readable((u8 __user *)ctx, 1)) >> >> I believe you meant: >> >> fault_in_pages_readable((u8 __user *)new_ctx, ctx_size) >> >> Since (u8 __user *) (ctx + 1) - 1 really is (u8 __user *) new_ctx + ctx_size - 1 > > Without the copy/paste errors: > > [...] > fault_in_pages_readable((u8 __user *)ctx, sizeof(*ctx)) > > Since (u8 __user *) (ctx + 1) - 1 really is (u8 __user *) ctx + sizeof(*ctx) - 1 > [...]
Oops you're right thanks. v2 submitted
Christophe
>>> return -EFAULT; >>> >>> /* >>> -- >>> 2.13.3 >>>
| |