2016-11-22 11:58:44

by Andrey Konovalov

[permalink] [raw]
Subject: net/udp: bug in skb_pull_rcsum

Hi,

I've got the following error report while fuzzing the kernel with syzkaller.

A reproducer is attached.

On commit 9c763584b7c8911106bb77af7e648bef09af9d80 (4.9-rc6, Nov 20).

------------[ cut here ]------------
kernel BUG at net/core/skbuff.c:3029!
invalid opcode: 0000 [#1] SMP KASAN
Modules linked in:
CPU: 1 PID: 3854 Comm: a.out Not tainted 4.9.0-rc6+ #431
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
task: ffff880068472c00 task.stack: ffff880063ec8000
RIP: 0010:[<ffffffff82b8fd85>] [<ffffffff82b8fd85>]
skb_pull_rcsum+0x255/0x350 net/core/skbuff.c:3029
RSP: 0018:ffff880063ecf660 EFLAGS: 00010297
RAX: ffff880068472c00 RBX: ffff880065a2da00 RCX: 0000000000000000
RDX: 0000000000000000 RSI: 000000000000000d RDI: ffffed000c7d9ec0
RBP: ffff880063ecf690 R08: 1ffff1000d08e67e R09: 1ffff1000cb45b50
R10: dffffc0000000000 R11: 0000000000000000 R12: ffff880065a2da80
R13: 0000000000000008 R14: ffff880065a2dad8 R15: 0000000000000001
FS: 00007fbb006497c0(0000) GS:ffff88006cd00000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000020032fe0 CR3: 00000000636d9000 CR4: 00000000000006e0
Stack:
ffff88006bfbb948 ffff880065a2da00 ffff880064160000 1ffff1000cb45b52
0000000000000000 1ffff1000d4d3933 ffff880063ecf6f8 ffffffff83354ced
00000000fffffe00 ffff880065a2da90 ffff880063ecf6c0 ffffffff00000001
Call Trace:
[< inline >] udp_csum_pull_header ./include/net/udp.h:166
[<ffffffff83354ced>] udpv6_queue_rcv_skb+0x37d/0x17b0 net/ipv6/udp.c:625
[< inline >] sk_backlog_rcv ./include/net/sock.h:874
[<ffffffff82b7eec6>] __release_sock+0x126/0x3a0 net/core/sock.c:2046
[<ffffffff82b7f199>] release_sock+0x59/0x1c0 net/core/sock.c:2504
[<ffffffff8334fc50>] udpv6_sendmsg+0x1310/0x24a0 net/ipv6/udp.c:1273
[<ffffffff83174fa7>] inet_sendmsg+0x317/0x4e0 net/ipv4/af_inet.c:734
[< inline >] sock_sendmsg_nosec net/socket.c:621
[<ffffffff82b7176c>] sock_sendmsg+0xcc/0x110 net/socket.c:631
[<ffffffff82b719d1>] sock_write_iter+0x221/0x3b0 net/socket.c:829
[<ffffffff8151e69b>] do_iter_readv_writev+0x2bb/0x3f0 fs/read_write.c:695
[<ffffffff81520501>] do_readv_writev+0x431/0x730 fs/read_write.c:872
[<ffffffff81520d2f>] vfs_writev+0x8f/0xc0 fs/read_write.c:911
[<ffffffff81520e41>] do_writev+0xe1/0x240 fs/read_write.c:944
[< inline >] SYSC_writev fs/read_write.c:1017
[<ffffffff81523ca7>] SyS_writev+0x27/0x30 fs/read_write.c:1014
[<ffffffff83fc4381>] entry_SYSCALL_64_fastpath+0x1f/0xc2
arch/x86/entry/entry_64.S:209
Code: 89 f8 49 c1 e8 03 47 0f b6 14 08 45 84 d2 74 0a 41 80 fa 03 0f
8e cf 00 00 00 80 a3 91 00 00 00 f9 e9 43 ff ff ff e8 3b 79 79 fe <0f>
0b e8 34 79 79 fe 0f 0b e8 2d 79 79 fe 48 8b 7d d0 31 d2 44
RIP [<ffffffff82b8fd85>] skb_pull_rcsum+0x255/0x350 net/core/skbuff.c:3029
RSP <ffff880063ecf660>
---[ end trace a5d5d2cef6a25ecb ]---
==================================================================


Attachments:
skb-pull-bug-poc.c (7.67 kB)

2016-11-22 16:41:37

by Eric Dumazet

[permalink] [raw]
Subject: Re: net/udp: bug in skb_pull_rcsum

On Tue, Nov 22, 2016 at 3:58 AM, Andrey Konovalov <[email protected]> wrote:
> Hi,
>
> I've got the following error report while fuzzing the kernel with syzkaller.
>
> A reproducer is attached.
>
> On commit 9c763584b7c8911106bb77af7e648bef09af9d80 (4.9-rc6, Nov 20).
>
> ------------[ cut here ]------------
> kernel BUG at net/core/skbuff.c:3029!
> invalid opcode: 0000 [#1] SMP KASAN
> Modules linked in:
> CPU: 1 PID: 3854 Comm: a.out Not tainted 4.9.0-rc6+ #431
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
> task: ffff880068472c00 task.stack: ffff880063ec8000
> RIP: 0010:[<ffffffff82b8fd85>] [<ffffffff82b8fd85>]
> skb_pull_rcsum+0x255/0x350 net/core/skbuff.c:3029
> RSP: 0018:ffff880063ecf660 EFLAGS: 00010297
> RAX: ffff880068472c00 RBX: ffff880065a2da00 RCX: 0000000000000000
> RDX: 0000000000000000 RSI: 000000000000000d RDI: ffffed000c7d9ec0
> RBP: ffff880063ecf690 R08: 1ffff1000d08e67e R09: 1ffff1000cb45b50
> R10: dffffc0000000000 R11: 0000000000000000 R12: ffff880065a2da80
> R13: 0000000000000008 R14: ffff880065a2dad8 R15: 0000000000000001
> FS: 00007fbb006497c0(0000) GS:ffff88006cd00000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 0000000020032fe0 CR3: 00000000636d9000 CR4: 00000000000006e0
> Stack:
> ffff88006bfbb948 ffff880065a2da00 ffff880064160000 1ffff1000cb45b52
> 0000000000000000 1ffff1000d4d3933 ffff880063ecf6f8 ffffffff83354ced
> 00000000fffffe00 ffff880065a2da90 ffff880063ecf6c0 ffffffff00000001
> Call Trace:
> [< inline >] udp_csum_pull_header ./include/net/udp.h:166
> [<ffffffff83354ced>] udpv6_queue_rcv_skb+0x37d/0x17b0 net/ipv6/udp.c:625
> [< inline >] sk_backlog_rcv ./include/net/sock.h:874
> [<ffffffff82b7eec6>] __release_sock+0x126/0x3a0 net/core/sock.c:2046
> [<ffffffff82b7f199>] release_sock+0x59/0x1c0 net/core/sock.c:2504
> [<ffffffff8334fc50>] udpv6_sendmsg+0x1310/0x24a0 net/ipv6/udp.c:1273
> [<ffffffff83174fa7>] inet_sendmsg+0x317/0x4e0 net/ipv4/af_inet.c:734
> [< inline >] sock_sendmsg_nosec net/socket.c:621
> [<ffffffff82b7176c>] sock_sendmsg+0xcc/0x110 net/socket.c:631
> [<ffffffff82b719d1>] sock_write_iter+0x221/0x3b0 net/socket.c:829
> [<ffffffff8151e69b>] do_iter_readv_writev+0x2bb/0x3f0 fs/read_write.c:695
> [<ffffffff81520501>] do_readv_writev+0x431/0x730 fs/read_write.c:872
> [<ffffffff81520d2f>] vfs_writev+0x8f/0xc0 fs/read_write.c:911
> [<ffffffff81520e41>] do_writev+0xe1/0x240 fs/read_write.c:944
> [< inline >] SYSC_writev fs/read_write.c:1017
> [<ffffffff81523ca7>] SyS_writev+0x27/0x30 fs/read_write.c:1014
> [<ffffffff83fc4381>] entry_SYSCALL_64_fastpath+0x1f/0xc2
> arch/x86/entry/entry_64.S:209
> Code: 89 f8 49 c1 e8 03 47 0f b6 14 08 45 84 d2 74 0a 41 80 fa 03 0f
> 8e cf 00 00 00 80 a3 91 00 00 00 f9 e9 43 ff ff ff e8 3b 79 79 fe <0f>
> 0b e8 34 79 79 fe 0f 0b e8 2d 79 79 fe 48 8b 7d d0 31 d2 44
> RIP [<ffffffff82b8fd85>] skb_pull_rcsum+0x255/0x350 net/core/skbuff.c:3029
> RSP <ffff880063ecf660>
> ---[ end trace a5d5d2cef6a25ecb ]---
> ==================================================================


Thanks for the report.

It seems bug was added in commit f7ad74fef3af6c6e2ef7f01c5589d77fe7db3d7c

I will cook a fix (Note that bug is no longer present in net-next and
linux-4.10+ kernels)

2016-11-22 17:19:11

by Eric Dumazet

[permalink] [raw]
Subject: [PATCH net] udplite: call proper backlog handlers

From: Eric Dumazet <[email protected]>

In commits 93821778def10 ("udp: Fix rcv socket locking") and
f7ad74fef3af ("net/ipv6/udp: UDP encapsulation: break backlog_rcv into
__udpv6_queue_rcv_skb") UDP backlog handlers were renamed, but UDPlite
was forgotten.

This leads to crashes if UDPlite header is pulled twice, which happens
starting from commit e6afc8ace6dd ("udp: remove headers from UDP packets
before queueing")

Bug found by syzkaller team, thanks a lot guys !

Note that backlog use in UDP/UDPlite is scheduled to be removed starting
from linux-4.10, so this patch is only needed up to linux-4.9

Fixes: 93821778def1 ("udp: Fix rcv socket locking")
Fixes: f7ad74fef3af ("net/ipv6/udp: UDP encapsulation: break backlog_rcv into __udpv6_queue_rcv_skb")
Fixes: e6afc8ace6dd ("udp: remove headers from UDP packets before queueing")
Signed-off-by: Eric Dumazet <[email protected]>
Reported-by: Andrey Konovalov <[email protected]>
Cc: Benjamin LaHaise <[email protected]>
Cc: Herbert Xu <[email protected]>
---
net/ipv4/udp.c | 2 +-
net/ipv4/udp_impl.h | 2 +-
net/ipv4/udplite.c | 2 +-
net/ipv6/udp.c | 2 +-
net/ipv6/udp_impl.h | 2 +-
net/ipv6/udplite.c | 2 +-
6 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 0de9d5d2b9ae..5bab6c3f7a2f 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -1455,7 +1455,7 @@ static void udp_v4_rehash(struct sock *sk)
udp_lib_rehash(sk, new_hash);
}

-static int __udp_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
+int __udp_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
{
int rc;

diff --git a/net/ipv4/udp_impl.h b/net/ipv4/udp_impl.h
index 7e0fe4bdd967..feb50a16398d 100644
--- a/net/ipv4/udp_impl.h
+++ b/net/ipv4/udp_impl.h
@@ -25,7 +25,7 @@ int udp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len, int noblock,
int flags, int *addr_len);
int udp_sendpage(struct sock *sk, struct page *page, int offset, size_t size,
int flags);
-int udp_queue_rcv_skb(struct sock *sk, struct sk_buff *skb);
+int __udp_queue_rcv_skb(struct sock *sk, struct sk_buff *skb);
void udp_destroy_sock(struct sock *sk);

#ifdef CONFIG_PROC_FS
diff --git a/net/ipv4/udplite.c b/net/ipv4/udplite.c
index af817158d830..ff450c2aad9b 100644
--- a/net/ipv4/udplite.c
+++ b/net/ipv4/udplite.c
@@ -50,7 +50,7 @@ struct proto udplite_prot = {
.sendmsg = udp_sendmsg,
.recvmsg = udp_recvmsg,
.sendpage = udp_sendpage,
- .backlog_rcv = udp_queue_rcv_skb,
+ .backlog_rcv = __udp_queue_rcv_skb,
.hash = udp_lib_hash,
.unhash = udp_lib_unhash,
.get_port = udp_v4_get_port,
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index e5056d4873d1..e4a8000d59ad 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -514,7 +514,7 @@ void __udp6_lib_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
return;
}

-static int __udpv6_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
+int __udpv6_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
{
int rc;

diff --git a/net/ipv6/udp_impl.h b/net/ipv6/udp_impl.h
index f6eb1ab34f4b..e78bdc76dcc3 100644
--- a/net/ipv6/udp_impl.h
+++ b/net/ipv6/udp_impl.h
@@ -26,7 +26,7 @@ int compat_udpv6_getsockopt(struct sock *sk, int level, int optname,
int udpv6_sendmsg(struct sock *sk, struct msghdr *msg, size_t len);
int udpv6_recvmsg(struct sock *sk, struct msghdr *msg, size_t len, int noblock,
int flags, int *addr_len);
-int udpv6_queue_rcv_skb(struct sock *sk, struct sk_buff *skb);
+int __udpv6_queue_rcv_skb(struct sock *sk, struct sk_buff *skb);
void udpv6_destroy_sock(struct sock *sk);

#ifdef CONFIG_PROC_FS
diff --git a/net/ipv6/udplite.c b/net/ipv6/udplite.c
index 47d0d2b87106..2f5101a12283 100644
--- a/net/ipv6/udplite.c
+++ b/net/ipv6/udplite.c
@@ -45,7 +45,7 @@ struct proto udplitev6_prot = {
.getsockopt = udpv6_getsockopt,
.sendmsg = udpv6_sendmsg,
.recvmsg = udpv6_recvmsg,
- .backlog_rcv = udpv6_queue_rcv_skb,
+ .backlog_rcv = __udpv6_queue_rcv_skb,
.hash = udp_lib_hash,
.unhash = udp_lib_unhash,
.get_port = udp_v6_get_port,


2016-11-24 20:32:51

by David Miller

[permalink] [raw]
Subject: Re: [PATCH net] udplite: call proper backlog handlers

From: Eric Dumazet <[email protected]>
Date: Tue, 22 Nov 2016 09:06:45 -0800

> From: Eric Dumazet <[email protected]>
>
> In commits 93821778def10 ("udp: Fix rcv socket locking") and
> f7ad74fef3af ("net/ipv6/udp: UDP encapsulation: break backlog_rcv into
> __udpv6_queue_rcv_skb") UDP backlog handlers were renamed, but UDPlite
> was forgotten.
>
> This leads to crashes if UDPlite header is pulled twice, which happens
> starting from commit e6afc8ace6dd ("udp: remove headers from UDP packets
> before queueing")
>
> Bug found by syzkaller team, thanks a lot guys !
>
> Note that backlog use in UDP/UDPlite is scheduled to be removed starting
> from linux-4.10, so this patch is only needed up to linux-4.9
>
> Fixes: 93821778def1 ("udp: Fix rcv socket locking")
> Fixes: f7ad74fef3af ("net/ipv6/udp: UDP encapsulation: break backlog_rcv into __udpv6_queue_rcv_skb")
> Fixes: e6afc8ace6dd ("udp: remove headers from UDP packets before queueing")
> Signed-off-by: Eric Dumazet <[email protected]>
> Reported-by: Andrey Konovalov <[email protected]>

Applied and queued up for -stable, thanks Eric.