Re: [PATCH 4.13 2/2] waitid(): Add missing access_ok() checks

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 10/12/2017 02:26 PM, Greg Kroah-Hartman wrote:
4.13-stable review patch.  If anyone has any objections, please let me know.

------------------

From: Kees Cook <keescook@xxxxxxxxxxxx>

commit 96ca579a1ecc943b75beba58bebb0356f6cc4b51 upstream.

Adds missing access_ok() checks.

CVE-2017-5123

Reported-by: Chris Salls <chrissalls5@xxxxxxxxx>
Signed-off-by: Kees Cook <keescook@xxxxxxxxxxxx>
Acked-by: Al Viro <viro@xxxxxxxxxxxxxxxxxx>
Fixes: 4c48abe91be0 ("waitid(): switch copyout of siginfo to unsafe_put_user()")
Signed-off-by: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>

---
  kernel/exit.c |    6 ++++++
  1 file changed, 6 insertions(+)

--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -1611,6 +1611,9 @@ SYSCALL_DEFINE5(waitid, int, which, pid_
  	if (!infop)
  		return err;
+ if (!access_ok(VERIFY_WRITE, infop, sizeof(*infop)))
+		goto Efault;

Not to be a pedant, but...

In the case that access_ok() fails, we invoke user_access_end() at the goto target without first invoking user_access_begin(). On x86 this imbalance is probably not a problem.

For other architectures that may want to implement user_access_{begin,end}() in the future, I think we should either specify that unbalanced calls to these two functions are expected and must work, or balance them here and specify that they must be balanced.


Thanks,
David Daney



[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]