Fix: kernel/ptrace.c: ptrace_peek_siginfo() missing __put_user() validation
PATCH 1/1 Fix: kernel/ptrace.c: ptrace_peek_siginfo() missing __put_user() validation
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.
--- kernel/ptrace.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-)
kernel/ptrace.c +11 -9
@@ -665,20 +665,22 @@ static int ptrace_peek_siginfo(struct taif (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);
Signature · 3 lines
Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com
Quoted 6 lines · Mathieu Desnoyers
This
__put_user()could be used by unprivileged processes to write into kernel memory. The issue here is that even ifcopy_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.
Quoted 57 lines
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
===================================================================kernel/ptrace.c +11
-9@@ -665,20 +665,22 @@ static int ptrace_peek_siginfo(struct taif (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);Signature · 4 lines
-- Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com
Quoted 5 lines · Mathieu Desnoyers
This
__put_user()could be used by unprivileged processes to write into kernel memory. The issue here is that even ifcopy_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.
Why do those stupid
__put_user(info.si_code, &uinfo->si_code))
things exist at all?
As far as I can tell, copy_siginfo_to_user[32]() already copies si_code. Is the field really so important that it has to be copied twice, just to make sure the write makes it?
Ok, ok, I see the explanation in the commit message that introduced this, but it sure as hell isn't obvious from actually looking at the code. I'm applying the patch, but I think this should have been a comment in the code as well.
Linus