On 06/28, Mathieu Desnoyers wrote:
>
> This __put_user() could be used by unprivileged processes to write into
> kernel memory. The issue here is that even if copy_siginfo_to_user()
> fails, the error code is not checked before __put_user() is executed.
> Luckily, ptrace_peek_siginfo() has been added within the 3.10-rc cycle,
> so it has not hit a stable release yet.

Agreed, this looks like 3.10 material.

Acked-by: Oleg Nesterov <[email protected]>

> Signed-off-by: Mathieu Desnoyers <[email protected]>
> CC: Andrey Vagin <[email protected]>
> CC: Roland McGrath <[email protected]>
> CC: Oleg Nesterov <[email protected]>
> CC: "Paul E. McKenney" <[email protected]>
> CC: David Howells <[email protected]>
> CC: Dave Jones <[email protected]>
> CC: Pavel Emelyanov <[email protected]>
> CC: Linus Torvalds <[email protected]>
> Cc: Pedro Alves <[email protected]>
> CC: Andrew Morton <[email protected]>
> CC: Linus Torvalds <[email protected]>
> ---
>  kernel/ptrace.c |   20 +++++++++++---------
>  1 file changed, 11 insertions(+), 9 deletions(-)
> 
> Index: linux/kernel/ptrace.c
> ===================================================================
> --- linux.orig/kernel/ptrace.c
> +++ linux/kernel/ptrace.c
> @@ -665,20 +665,22 @@ static int ptrace_peek_siginfo(struct ta
>               if (unlikely(is_compat_task())) {
>                       compat_siginfo_t __user *uinfo = compat_ptr(data);
>  
> -                     ret = copy_siginfo_to_user32(uinfo, &info);
> -                     ret |= __put_user(info.si_code, &uinfo->si_code);
> +                     if (copy_siginfo_to_user32(uinfo, &info) ||
> +                         __put_user(info.si_code, &uinfo->si_code)) {
> +                             ret = -EFAULT;
> +                             break;
> +                     }
> +
>               } else
>  #endif
>               {
>                       siginfo_t __user *uinfo = (siginfo_t __user *) data;
>  
> -                     ret = copy_siginfo_to_user(uinfo, &info);
> -                     ret |= __put_user(info.si_code, &uinfo->si_code);
> -             }
> -
> -             if (ret) {
> -                     ret = -EFAULT;
> -                     break;
> +                     if (copy_siginfo_to_user(uinfo, &info) ||
> +                         __put_user(info.si_code, &uinfo->si_code)) {
> +                             ret = -EFAULT;
> +                             break;
> +                     }
>               }
>  
>               data += sizeof(siginfo_t);
> 
> -- 
> Mathieu Desnoyers
> EfficiOS Inc.
> http://www.efficios.com

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to