* [PATCH] net: ping: check minimum size on ICMP header length
@ 2016-12-03 0:58 Kees Cook
2016-12-05 3:35 ` Lorenzo Colitti
2016-12-05 18:19 ` David Miller
0 siblings, 2 replies; 3+ messages in thread
From: Kees Cook @ 2016-12-03 0:58 UTC (permalink / raw)
To: David S. Miller
Cc: netdev, Min Chong, Qidan He, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy, linux-kernel
Prior to commit c0371da6047a ("put iov_iter into msghdr") in v3.19, there
was no check that the iovec contained enough bytes for a icmp header,
and the read loop would walk across neighboring stack contents. Since
the iov_iter conversion, bad arguments are noticed, but the returned
error is EFAULT. Returning EMSGSIZE is a clearer fix and solves the
problem prior to v3.19.
This was found using trinity with KASAN on v3.18:
BUG: KASAN: stack-out-of-bounds in memcpy_fromiovec+0x60/0x114 at addr ffffffc071077da0
Read of size 8 by task trinity-c2/9623
page:ffffffbe034b9a08 count:0 mapcount:0 mapping: (null) index:0x0
flags: 0x0()
page dumped because: kasan: bad access detected
CPU: 0 PID: 9623 Comm: trinity-c2 Tainted: G BU 3.18.0-dirty #15
Hardware name: Google Tegra210 Smaug Rev 1,3+ (DT)
Call trace:
[<ffffffc000209c98>] dump_backtrace+0x0/0x1ac arch/arm64/kernel/traps.c:90
[<ffffffc000209e54>] show_stack+0x10/0x1c arch/arm64/kernel/traps.c:171
[< inline >] __dump_stack lib/dump_stack.c:15
[<ffffffc000f18dc4>] dump_stack+0x7c/0xd0 lib/dump_stack.c:50
[< inline >] print_address_description mm/kasan/report.c:147
[< inline >] kasan_report_error mm/kasan/report.c:236
[<ffffffc000373dcc>] kasan_report+0x380/0x4b8 mm/kasan/report.c:259
[< inline >] check_memory_region mm/kasan/kasan.c:264
[<ffffffc00037352c>] __asan_load8+0x20/0x70 mm/kasan/kasan.c:507
[<ffffffc0005b9624>] memcpy_fromiovec+0x5c/0x114 lib/iovec.c:15
[< inline >] memcpy_from_msg include/linux/skbuff.h:2667
[<ffffffc000ddeba0>] ping_common_sendmsg+0x50/0x108 net/ipv4/ping.c:674
[<ffffffc000dded30>] ping_v4_sendmsg+0xd8/0x698 net/ipv4/ping.c:714
[<ffffffc000dc91dc>] inet_sendmsg+0xe0/0x12c net/ipv4/af_inet.c:749
[< inline >] __sock_sendmsg_nosec net/socket.c:624
[< inline >] __sock_sendmsg net/socket.c:632
[<ffffffc000cab61c>] sock_sendmsg+0x124/0x164 net/socket.c:643
[< inline >] SYSC_sendto net/socket.c:1797
[<ffffffc000cad270>] SyS_sendto+0x178/0x1d8 net/socket.c:1761
CVE-2016-8399
Reported-by: Qidan He <i@flanker017.me>
Fixes: c319b4d76b9e ("net: ipv4: add IPPROTO_ICMP socket kind")
Cc: stable@vger.kernel.org
Signed-off-by: Kees Cook <keescook@chromium.org>
---
net/ipv4/ping.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/ipv4/ping.c b/net/ipv4/ping.c
index 205e2000d395..8257be3f032c 100644
--- a/net/ipv4/ping.c
+++ b/net/ipv4/ping.c
@@ -654,7 +654,7 @@ int ping_common_sendmsg(int family, struct msghdr *msg, size_t len,
void *user_icmph, size_t icmph_len) {
u8 type, code;
- if (len > 0xFFFF)
+ if (len > 0xFFFF || len < icmph_len)
return -EMSGSIZE;
/*
--
2.7.4
--
Kees Cook
Nexus Security
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] net: ping: check minimum size on ICMP header length
2016-12-03 0:58 [PATCH] net: ping: check minimum size on ICMP header length Kees Cook
@ 2016-12-05 3:35 ` Lorenzo Colitti
2016-12-05 18:19 ` David Miller
1 sibling, 0 replies; 3+ messages in thread
From: Lorenzo Colitti @ 2016-12-05 3:35 UTC (permalink / raw)
To: Kees Cook
Cc: David S. Miller, netdev, Min Chong, Qidan He, Alexey Kuznetsov,
James Morris, Hideaki YOSHIFUJI, Patrick McHardy, lkml
On Sat, Dec 3, 2016 at 9:58 AM, Kees Cook <keescook@chromium.org> wrote:
> - if (len > 0xFFFF)
> + if (len > 0xFFFF || len < icmph_len)
> return -EMSGSIZE;
EMSGSIZE usually means the message is too long. Maybe use EINVAL?
That's what the code will return if the passed-in ICMP header is
invalid (e.g., is not an echo request).
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] net: ping: check minimum size on ICMP header length
2016-12-03 0:58 [PATCH] net: ping: check minimum size on ICMP header length Kees Cook
2016-12-05 3:35 ` Lorenzo Colitti
@ 2016-12-05 18:19 ` David Miller
1 sibling, 0 replies; 3+ messages in thread
From: David Miller @ 2016-12-05 18:19 UTC (permalink / raw)
To: keescook
Cc: netdev, mchong, i, kuznet, jmorris, yoshfuji, kaber, linux-kernel
From: Kees Cook <keescook@chromium.org>
Date: Fri, 2 Dec 2016 16:58:53 -0800
> diff --git a/net/ipv4/ping.c b/net/ipv4/ping.c
> index 205e2000d395..8257be3f032c 100644
> --- a/net/ipv4/ping.c
> +++ b/net/ipv4/ping.c
> @@ -654,7 +654,7 @@ int ping_common_sendmsg(int family, struct msghdr *msg, size_t len,
> void *user_icmph, size_t icmph_len) {
> u8 type, code;
>
> - if (len > 0xFFFF)
> + if (len > 0xFFFF || len < icmph_len)
> return -EMSGSIZE;
As suggested by Lorenzo, please use -EINVAL here.
Thanks.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-12-05 18:19 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-03 0:58 [PATCH] net: ping: check minimum size on ICMP header length Kees Cook
2016-12-05 3:35 ` Lorenzo Colitti
2016-12-05 18:19 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).