Re: [PATCH] powerpc/ptrace: Use copy_{from, to}_user() rather than open-coding

[Samuel Mendoza-Jonas]

On Tue, 2018-05-29 at 22:57 +1000, Michael Ellerman wrote:
> From: Al Viro 
> 
> In PPC_PTRACE_GETHWDBGINFO and PPC_PTRACE_SETHWDEBUG we do an
> access_ok() check and then __copy_{from,to}_user().
> 
> Instead we should just use copy_{from,to}_user() which does all that
> for us and is less error prone.
> 
> Signed-off-by: Al Viro 
> Signed-off-by: Michael Ellerman 

Reviewed-by: Samuel Mendoza-Jonas 

> ---
>  arch/powerpc/kernel/ptrace.c | 20 ++--
>  1 file changed, 6 insertions(+), 14 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
> index 0f63dd5972e9..9667666eb18e 100644
> --- a/arch/powerpc/kernel/ptrace.c
> +++ b/arch/powerpc/kernel/ptrace.c
> @@ -3082,27 +3082,19 @@ long arch_ptrace(struct task_struct *child, long 
> request,
>  #endif /* CONFIG_HAVE_HW_BREAKPOINT */
>  #endif /* CONFIG_PPC_ADV_DEBUG_REGS */
>  
> - if (!access_ok(VERIFY_WRITE, datavp,
> -sizeof(struct ppc_debug_info)))
> + if (copy_to_user(datavp, ,
> +  sizeof(struct ppc_debug_info)))
>   return -EFAULT;
> - ret = __copy_to_user(datavp, ,
> -  sizeof(struct ppc_debug_info)) ?
> -   -EFAULT : 0;
> - break;
> + return 0;
>   }
>  
>   case PPC_PTRACE_SETHWDEBUG: {
>   struct ppc_hw_breakpoint bp_info;
>  
> - if (!access_ok(VERIFY_READ, datavp,
> -sizeof(struct ppc_hw_breakpoint)))
> + if (copy_from_user(_info, datavp,
> +sizeof(struct ppc_hw_breakpoint)))
>   return -EFAULT;
> - ret = __copy_from_user(_info, datavp,
> -sizeof(struct ppc_hw_breakpoint)) ?
> -   -EFAULT : 0;
> - if (!ret)
> - ret = ppc_set_hwdebug(child, _info);
> - break;
> + return ppc_set_hwdebug(child, _info);
>   }
>  
>   case PPC_PTRACE_DELHWDEBUG: {

[PATCH] powerpc/ptrace: Use copy_{from, to}_user() rather than open-coding

[Michael Ellerman]

From: Al Viro 

In PPC_PTRACE_GETHWDBGINFO and PPC_PTRACE_SETHWDEBUG we do an
access_ok() check and then __copy_{from,to}_user().

Instead we should just use copy_{from,to}_user() which does all that
for us and is less error prone.

Signed-off-by: Al Viro 
Signed-off-by: Michael Ellerman 
---
 arch/powerpc/kernel/ptrace.c | 20 ++--
 1 file changed, 6 insertions(+), 14 deletions(-)

diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
index 0f63dd5972e9..9667666eb18e 100644
--- a/arch/powerpc/kernel/ptrace.c
+++ b/arch/powerpc/kernel/ptrace.c
@@ -3082,27 +3082,19 @@ long arch_ptrace(struct task_struct *child, long 
request,
 #endif /* CONFIG_HAVE_HW_BREAKPOINT */
 #endif /* CONFIG_PPC_ADV_DEBUG_REGS */
 
-   if (!access_ok(VERIFY_WRITE, datavp,
-  sizeof(struct ppc_debug_info)))
+   if (copy_to_user(datavp, ,
+sizeof(struct ppc_debug_info)))
return -EFAULT;
-   ret = __copy_to_user(datavp, ,
-sizeof(struct ppc_debug_info)) ?
- -EFAULT : 0;
-   break;
+   return 0;
}
 
case PPC_PTRACE_SETHWDEBUG: {
struct ppc_hw_breakpoint bp_info;
 
-   if (!access_ok(VERIFY_READ, datavp,
-  sizeof(struct ppc_hw_breakpoint)))
+   if (copy_from_user(_info, datavp,
+  sizeof(struct ppc_hw_breakpoint)))
return -EFAULT;
-   ret = __copy_from_user(_info, datavp,
-  sizeof(struct ppc_hw_breakpoint)) ?
- -EFAULT : 0;
-   if (!ret)
-   ret = ppc_set_hwdebug(child, _info);
-   break;
+   return ppc_set_hwdebug(child, _info);
}
 
case PPC_PTRACE_DELHWDEBUG: {
-- 
2.14.1