From: Leonid Yegoshin <Leonid.Yegoshin@imgtec.com>
To: James Hogan <james.hogan@imgtec.com>, <stable@vger.kernel.org>,
"Tom Herbert" <therbert@google.com>
Cc: Markos Chandras <markos.chandras@imgtec.com>,
Paul Burton <paul.burton@imgtec.com>, <linux-mips@linux-mips.org>,
Ralf Baechle <ralf@linux-mips.org>
Subject: Re: [PATCH backport v3.15..v4.1 1/2] MIPS: uaccess: Take EVA into account in __copy_from_user()
Date: Mon, 4 Jan 2016 13:33:51 -0800 [thread overview]
Message-ID: <568AE53F.80103@imgtec.com> (raw)
In-Reply-To: <1451939344-21557-2-git-send-email-james.hogan@imgtec.com>
On 01/04/2016 12:29 PM, James Hogan wrote:
> commit 6f06a2c45d8d714ea3b11a360b4a7191e52acaa4 upstream.
>
> When EVA is in use, __copy_from_user() was unconditionally using the EVA
> instructions to read the user address space, however this can also be
> used for kernel access. If the address isn't a valid user address it
> will cause an address error or TLB exception, and if it is then user
> memory may be read instead of kernel memory.
>
> For example in the following stack trace from Linux v3.10 (changes since
> then will prevent this particular one still happening) kernel_sendmsg()
> set the user address limit to KERNEL_DS, and tcp_sendmsg() goes on to
> use __copy_from_user() with a kernel address in KSeg0.
>
> [<8002d434>] __copy_fromuser_common+0x10c/0x254
> [<805710e0>] tcp_sendmsg+0x5f4/0xf00
> [<804e8e3c>] sock_sendmsg+0x78/0xa0
> [<804e8f28>] kernel_sendmsg+0x24/0x38
> [<804ee0f8>] sock_no_sendpage+0x70/0x7c
> [<8017c820>] pipe_to_sendpage+0x80/0x98
> [<8017c6b0>] splice_from_pipe_feed+0xa8/0x198
> [<8017cc54>] __splice_from_pipe+0x4c/0x8c
> [<8017e844>] splice_from_pipe+0x58/0x78
> [<8017e884>] generic_splice_sendpage+0x20/0x2c
> [<8017d690>] do_splice_from+0xb4/0x110
> [<8017d710>] direct_splice_actor+0x24/0x30
> [<8017d394>] splice_direct_to_actor+0xd8/0x208
> [<8017d51c>] do_splice_direct+0x58/0x7c
> [<8014eaf4>] do_sendfile+0x1dc/0x39c
> [<8014f82c>] SyS_sendfile+0x90/0xf8
>
> Add the eva_kernel_access() check in __copy_from_user() like the one in
> copy_from_user().
>
I think that the best way to fix this problem is - stop
skb_do_copy_data_nocache() using __copy_from_user_nocache(). All TCP/IP
stuff (beyond SCTP) doesn't use "accelerated" __copy*() functions.
Adding a user space check in __copy_from_user() kills the original
design. And splitting a user space processing in two places
(skb_do_copy_data_nocache() calls access_ok(), BTW) - and it is also a
bad thing in my mind.
- Leonid.
WARNING: multiple messages have this Message-ID (diff)
From: Leonid Yegoshin <Leonid.Yegoshin@imgtec.com>
To: James Hogan <james.hogan@imgtec.com>,
stable@vger.kernel.org, Tom Herbert <therbert@google.com>
Cc: Markos Chandras <markos.chandras@imgtec.com>,
Paul Burton <paul.burton@imgtec.com>,
linux-mips@linux-mips.org, Ralf Baechle <ralf@linux-mips.org>
Subject: Re: [PATCH backport v3.15..v4.1 1/2] MIPS: uaccess: Take EVA into account in __copy_from_user()
Date: Mon, 4 Jan 2016 13:33:51 -0800 [thread overview]
Message-ID: <568AE53F.80103@imgtec.com> (raw)
Message-ID: <20160104213351.TDXPTsSbEz_mki3P1tqAXQenlK3iYg994MNx8gbqdSw@z> (raw)
In-Reply-To: <1451939344-21557-2-git-send-email-james.hogan@imgtec.com>
On 01/04/2016 12:29 PM, James Hogan wrote:
> commit 6f06a2c45d8d714ea3b11a360b4a7191e52acaa4 upstream.
>
> When EVA is in use, __copy_from_user() was unconditionally using the EVA
> instructions to read the user address space, however this can also be
> used for kernel access. If the address isn't a valid user address it
> will cause an address error or TLB exception, and if it is then user
> memory may be read instead of kernel memory.
>
> For example in the following stack trace from Linux v3.10 (changes since
> then will prevent this particular one still happening) kernel_sendmsg()
> set the user address limit to KERNEL_DS, and tcp_sendmsg() goes on to
> use __copy_from_user() with a kernel address in KSeg0.
>
> [<8002d434>] __copy_fromuser_common+0x10c/0x254
> [<805710e0>] tcp_sendmsg+0x5f4/0xf00
> [<804e8e3c>] sock_sendmsg+0x78/0xa0
> [<804e8f28>] kernel_sendmsg+0x24/0x38
> [<804ee0f8>] sock_no_sendpage+0x70/0x7c
> [<8017c820>] pipe_to_sendpage+0x80/0x98
> [<8017c6b0>] splice_from_pipe_feed+0xa8/0x198
> [<8017cc54>] __splice_from_pipe+0x4c/0x8c
> [<8017e844>] splice_from_pipe+0x58/0x78
> [<8017e884>] generic_splice_sendpage+0x20/0x2c
> [<8017d690>] do_splice_from+0xb4/0x110
> [<8017d710>] direct_splice_actor+0x24/0x30
> [<8017d394>] splice_direct_to_actor+0xd8/0x208
> [<8017d51c>] do_splice_direct+0x58/0x7c
> [<8014eaf4>] do_sendfile+0x1dc/0x39c
> [<8014f82c>] SyS_sendfile+0x90/0xf8
>
> Add the eva_kernel_access() check in __copy_from_user() like the one in
> copy_from_user().
>
I think that the best way to fix this problem is - stop
skb_do_copy_data_nocache() using __copy_from_user_nocache(). All TCP/IP
stuff (beyond SCTP) doesn't use "accelerated" __copy*() functions.
Adding a user space check in __copy_from_user() kills the original
design. And splitting a user space processing in two places
(skb_do_copy_data_nocache() calls access_ok(), BTW) - and it is also a
bad thing in my mind.
- Leonid.
next prev parent reply other threads:[~2016-01-04 21:34 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-01-04 20:29 [PATCH backport v3.15..v4.1 0/2] MIPS: uaccess: EVA fixes James Hogan
2016-01-04 20:29 ` James Hogan
2016-01-04 20:29 ` [PATCH backport v3.15..v4.1 1/2] MIPS: uaccess: Take EVA into account in __copy_from_user() James Hogan
2016-01-04 20:29 ` James Hogan
2016-01-04 21:33 ` Leonid Yegoshin [this message]
2016-01-04 21:33 ` Leonid Yegoshin
2016-01-04 22:28 ` James Hogan
2016-01-04 22:28 ` James Hogan
2016-01-05 0:46 ` Leonid Yegoshin
2016-01-05 0:46 ` Leonid Yegoshin
2016-01-04 20:29 ` [PATCH backport v3.15..v4.1 2/2] MIPS: uaccess: Take EVA into account in [__]clear_user James Hogan
2016-01-04 20:29 ` James Hogan
2016-01-15 17:52 ` [PATCH backport v3.15..v4.1 0/2] MIPS: uaccess: EVA fixes Luis Henriques
2016-01-15 17:52 ` Luis Henriques
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=568AE53F.80103@imgtec.com \
--to=leonid.yegoshin@imgtec.com \
--cc=james.hogan@imgtec.com \
--cc=linux-mips@linux-mips.org \
--cc=markos.chandras@imgtec.com \
--cc=paul.burton@imgtec.com \
--cc=ralf@linux-mips.org \
--cc=stable@vger.kernel.org \
--cc=therbert@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.