Re: [PATCH] sctp: Add rcu lock to protect dst entry in sctp_transport_route
在 2019/6/12 21:13, Neil Horman 写道:
On Tue, Jun 11, 2019 at 10:33:17AM +0800, Su Yanjun wrote:
在 2019/6/10 19:12, Neil Horman 写道:
On Mon, Jun 10, 2019 at 11:20:00AM +0800, Su Yanjun wrote:
syzbot found a crash in rt_cache_valid. Problem is that when more
threads release dst in sctp_transport_route, the route cache can
be freed.
As follows,
p1:
sctp_transport_route
dst_release
get_dst
p2:
sctp_transport_route
dst_release
get_dst
...
If enough threads calling dst_release will cause dst->refcnt==0
then rcu softirq will reclaim the dst entry,get_dst then use
the freed memory.
This patch adds rcu lock to protect the dst_entry here.
Fixes: 6e91b578bf3f("sctp: re-use sctp_transport_pmtu in
sctp_transport_route")
Signed-off-by: Su Yanjun
Reported-by: syzbot+a9e23ea2aa21044c2...@syzkaller.appspotmail.com
---
net/sctp/transport.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/net/sctp/transport.c b/net/sctp/transport.c
index ad158d3..5ad7e20 100644
--- a/net/sctp/transport.c
+++ b/net/sctp/transport.c
@@ -308,8 +308,13 @@ void sctp_transport_route(struct sctp_transport
*transport,
struct sctp_association *asoc = transport->asoc;
struct sctp_af *af = transport->af_specific;
+ /* When dst entry is being released, route cache may be referred
+* again. Add rcu lock here to protect dst entry.
+*/
+ rcu_read_lock();
sctp_transport_dst_release(transport);
af->get_dst(transport, saddr, >fl, sctp_opt2sk(opt));
+ rcu_read_unlock();
What is the exact error that syzbot reported? This doesn't seem like it
fixes
BUG: KASAN: slab-out-of-bounds in rt_cache_valid+0x158/0x190
net/ipv4/route.c:1556
Read of size 2 at addr 8880654f3ac7 by task syz-executor.0/26603
CPU: 0 PID: 26603 Comm: syz-executor.0 Not tainted 5.2.0-rc2+ #9
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x172/0x1f0 lib/dump_stack.c:113
print_address_description.cold+0x7c/0x20d mm/kasan/report.c:188
__kasan_report.cold+0x1b/0x40 mm/kasan/report.c:317
kasan_report+0x12/0x20 mm/kasan/common.c:614
__asan_report_load2_noabort+0x14/0x20 mm/kasan/generic_report.c:130
rt_cache_valid+0x158/0x190 net/ipv4/route.c:1556
__mkroute_output net/ipv4/route.c:2332 [inline]
ip_route_output_key_hash_rcu+0x819/0x2d50 net/ipv4/route.c:2564
ip_route_output_key_hash+0x1ef/0x360 net/ipv4/route.c:2393
__ip_route_output_key include/net/route.h:125 [inline]
ip_route_output_flow+0x28/0xc0 net/ipv4/route.c:2651
ip_route_output_key include/net/route.h:135 [inline]
sctp_v4_get_dst+0x467/0x1260 net/sctp/protocol.c:435
sctp_transport_route+0x12d/0x360 net/sctp/transport.c:297
sctp_assoc_add_peer+0x53e/0xfc0 net/sctp/associola.c:663
sctp_process_param net/sctp/sm_make_chunk.c:2531 [inline]
sctp_process_init+0x2491/0x2b10 net/sctp/sm_make_chunk.c:2344
sctp_cmd_process_init net/sctp/sm_sideeffect.c:667 [inline]
sctp_cmd_interpreter net/sctp/sm_sideeffect.c:1369 [inline]
sctp_side_effects net/sctp/sm_sideeffect.c:1179 [inline]
sctp_do_sm+0x3a30/0x50e0 net/sctp/sm_sideeffect.c:1150
sctp_assoc_bh_rcv+0x343/0x660 net/sctp/associola.c:1059
sctp_inq_push+0x1e4/0x280 net/sctp/inqueue.c:80
sctp_backlog_rcv+0x196/0xbe0 net/sctp/input.c:339
sk_backlog_rcv include/net/sock.h:945 [inline]
__release_sock+0x129/0x390 net/core/sock.c:2412
release_sock+0x59/0x1c0 net/core/sock.c:2928
sctp_wait_for_connect+0x316/0x540 net/sctp/socket.c:9039
__sctp_connect+0xab2/0xcd0 net/sctp/socket.c:1226
__sctp_setsockopt_connectx+0x133/0x1a0 net/sctp/socket.c:1334
sctp_setsockopt_connectx_old net/sctp/socket.c:1350 [inline]
sctp_setsockopt net/sctp/socket.c:4644 [inline]
sctp_setsockopt+0x22c0/0x6d10 net/sctp/socket.c:4608
compat_sock_common_setsockopt+0x106/0x140 net/core/sock.c:3137
__compat_sys_setsockopt+0x185/0x380 net/compat.c:383
__do_compat_sys_setsockopt net/compat.c:396 [inline]
__se_compat_sys_setsockopt net/compat.c:393 [inline]
__ia32_compat_sys_setsockopt+0xbd/0x150 net/compat.c:393
do_syscall_32_irqs_on arch/x86/entry/common.c:337 [inline]
do_fast_syscall_32+0x27b/0xd7d arch/x86/entry/common.c:408
entry_SYSENTER_compat+0x70/0x7f arch/x86/entry/entry_64_compat.S:139
RIP: 0023:0xf7ff5849
Code: 85 d2 74 02 89 0a 5b 5d c3 8b 04 24 c3 8b 14 24 c3 8b 3c 24 c3 90 90
90 90 90 90 90 90 90 90 90 90 51 52 55 89 e5 0f 34 cd 80 <5d> 5a 59 c3 90
90 90 90 eb 0d 90 90 90 90 90 90 90 90 90 90 90 90
RSP: 002b:f5df10cc EFLAGS: 0296 ORIG_RAX: 016e
RAX: ffda RBX: 0007 RCX: 0084
RDX: 006b RSI: 2055bfe4 RDI: 001c
RBP: R08: R09:
R10: R11: R12:
R13: R14: R15:
Allocated by task 480:
Re: [PATCH] sctp: Add rcu lock to protect dst entry in sctp_transport_route
On Thu, Jun 13, 2019 at 10:37:51AM +0800, Su Yanjun wrote:
>
> 在 2019/6/12 21:13, Neil Horman 写道:
> > On Tue, Jun 11, 2019 at 10:33:17AM +0800, Su Yanjun wrote:
> > > 在 2019/6/10 19:12, Neil Horman 写道:
> > > > On Mon, Jun 10, 2019 at 11:20:00AM +0800, Su Yanjun wrote:
> > > > > syzbot found a crash in rt_cache_valid. Problem is that when more
> > > > > threads release dst in sctp_transport_route, the route cache can
> > > > > be freed.
> > > > >
> > > > > As follows,
> > > > > p1:
> > > > > sctp_transport_route
> > > > > dst_release
> > > > > get_dst
> > > > >
> > > > > p2:
> > > > > sctp_transport_route
> > > > > dst_release
> > > > > get_dst
> > > > > ...
> > > > >
> > > > > If enough threads calling dst_release will cause dst->refcnt==0
> > > > > then rcu softirq will reclaim the dst entry,get_dst then use
> > > > > the freed memory.
> > > > >
> > > > > This patch adds rcu lock to protect the dst_entry here.
> > > > >
> > > > > Fixes: 6e91b578bf3f("sctp: re-use sctp_transport_pmtu in
> > > > sctp_transport_route")
> > > > > Signed-off-by: Su Yanjun
> > > > > Reported-by: syzbot+a9e23ea2aa21044c2...@syzkaller.appspotmail.com
> > > > > ---
> > > > >net/sctp/transport.c | 5 +
> > > > >1 file changed, 5 insertions(+)
> > > > >
> > > > > diff --git a/net/sctp/transport.c b/net/sctp/transport.c
> > > > > index ad158d3..5ad7e20 100644
> > > > > --- a/net/sctp/transport.c
> > > > > +++ b/net/sctp/transport.c
> > > > > @@ -308,8 +308,13 @@ void sctp_transport_route(struct sctp_transport
> > > > *transport,
> > > > > struct sctp_association *asoc = transport->asoc;
> > > > > struct sctp_af *af = transport->af_specific;
> > > > > + /* When dst entry is being released, route cache may be referred
> > > > > + * again. Add rcu lock here to protect dst entry.
> > > > > + */
> > > > > + rcu_read_lock();
> > > > > sctp_transport_dst_release(transport);
> > > > > af->get_dst(transport, saddr, >fl, sctp_opt2sk(opt));
> > > > > + rcu_read_unlock();
> > > > What is the exact error that syzbot reported? This doesn't seem like it
> > > > fixes
> > > BUG: KASAN: slab-out-of-bounds in rt_cache_valid+0x158/0x190
> > > net/ipv4/route.c:1556
> > > Read of size 2 at addr 8880654f3ac7 by task syz-executor.0/26603
> > >
> > > CPU: 0 PID: 26603 Comm: syz-executor.0 Not tainted 5.2.0-rc2+ #9
> > > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> > > Google 01/01/2011
> > > Call Trace:
> > > __dump_stack lib/dump_stack.c:77 [inline]
> > > dump_stack+0x172/0x1f0 lib/dump_stack.c:113
> > > print_address_description.cold+0x7c/0x20d mm/kasan/report.c:188
> > > __kasan_report.cold+0x1b/0x40 mm/kasan/report.c:317
> > > kasan_report+0x12/0x20 mm/kasan/common.c:614
> > > __asan_report_load2_noabort+0x14/0x20 mm/kasan/generic_report.c:130
> > > rt_cache_valid+0x158/0x190 net/ipv4/route.c:1556
> > > __mkroute_output net/ipv4/route.c:2332 [inline]
> > > ip_route_output_key_hash_rcu+0x819/0x2d50 net/ipv4/route.c:2564
> > > ip_route_output_key_hash+0x1ef/0x360 net/ipv4/route.c:2393
> > > __ip_route_output_key include/net/route.h:125 [inline]
> > > ip_route_output_flow+0x28/0xc0 net/ipv4/route.c:2651
> > > ip_route_output_key include/net/route.h:135 [inline]
> > > sctp_v4_get_dst+0x467/0x1260 net/sctp/protocol.c:435
> > > sctp_transport_route+0x12d/0x360 net/sctp/transport.c:297
> > > sctp_assoc_add_peer+0x53e/0xfc0 net/sctp/associola.c:663
> > > sctp_process_param net/sctp/sm_make_chunk.c:2531 [inline]
> > > sctp_process_init+0x2491/0x2b10 net/sctp/sm_make_chunk.c:2344
> > > sctp_cmd_process_init net/sctp/sm_sideeffect.c:667 [inline]
> > > sctp_cmd_interpreter net/sctp/sm_sideeffect.c:1369 [inline]
> > > sctp_side_effects net/sctp/sm_sideeffect.c:1179 [inline]
> > > sctp_do_sm+0x3a30/0x50e0 net/sctp/sm_sideeffect.c:1150
> > > sctp_assoc_bh_rcv+0x343/0x660 net/sctp/associola.c:1059
> > > sctp_inq_push+0x1e4/0x280 net/sctp/inqueue.c:80
> > > sctp_backlog_rcv+0x196/0xbe0 net/sctp/input.c:339
> > > sk_backlog_rcv include/net/sock.h:945 [inline]
> > > __release_sock+0x129/0x390 net/core/sock.c:2412
> > > release_sock+0x59/0x1c0 net/core/sock.c:2928
> > > sctp_wait_for_connect+0x316/0x540 net/sctp/socket.c:9039
> > > __sctp_connect+0xab2/0xcd0 net/sctp/socket.c:1226
> > > __sctp_setsockopt_connectx+0x133/0x1a0 net/sctp/socket.c:1334
> > > sctp_setsockopt_connectx_old net/sctp/socket.c:1350 [inline]
> > > sctp_setsockopt net/sctp/socket.c:4644 [inline]
> > > sctp_setsockopt+0x22c0/0x6d10 net/sctp/socket.c:4608
> > > compat_sock_common_setsockopt+0x106/0x140 net/core/sock.c:3137
> > > __compat_sys_setsockopt+0x185/0x380 net/compat.c:383
> > > __do_compat_sys_setsockopt net/compat.c:396 [inline]
> > > __se_compat_sys_setsockopt net/compat.c:393 [inline]
> > > __ia32_compat_sys_setsockopt+0xbd/0x150 net/compat.c:393
> > > do_syscall_32_irqs_on
Re: [PATCH] sctp: Add rcu lock to protect dst entry in sctp_transport_route
On Thu, Jun 13, 2019 at 07:35:44AM -0400, Neil Horman wrote:
> On Thu, Jun 13, 2019 at 10:37:51AM +0800, Su Yanjun wrote:
> >
> > 在 2019/6/12 21:13, Neil Horman 写道:
> > > On Tue, Jun 11, 2019 at 10:33:17AM +0800, Su Yanjun wrote:
> > > > 在 2019/6/10 19:12, Neil Horman 写道:
> > > > > On Mon, Jun 10, 2019 at 11:20:00AM +0800, Su Yanjun wrote:
> > > > > > syzbot found a crash in rt_cache_valid. Problem is that when more
> > > > > > threads release dst in sctp_transport_route, the route cache can
> > > > > > be freed.
> > > > > >
> > > > > > As follows,
> > > > > > p1:
> > > > > > sctp_transport_route
> > > > > > dst_release
> > > > > > get_dst
> > > > > >
> > > > > > p2:
> > > > > > sctp_transport_route
> > > > > > dst_release
> > > > > > get_dst
> > > > > > ...
> > > > > >
> > > > > > If enough threads calling dst_release will cause dst->refcnt==0
> > > > > > then rcu softirq will reclaim the dst entry,get_dst then use
> > > > > > the freed memory.
> > > > > >
> > > > > > This patch adds rcu lock to protect the dst_entry here.
> > > > > >
> > > > > > Fixes: 6e91b578bf3f("sctp: re-use sctp_transport_pmtu in
> > > > > sctp_transport_route")
> > > > > > Signed-off-by: Su Yanjun
> > > > > > Reported-by: syzbot+a9e23ea2aa21044c2...@syzkaller.appspotmail.com
> > > > > > ---
> > > > > >net/sctp/transport.c | 5 +
> > > > > >1 file changed, 5 insertions(+)
> > > > > >
> > > > > > diff --git a/net/sctp/transport.c b/net/sctp/transport.c
> > > > > > index ad158d3..5ad7e20 100644
> > > > > > --- a/net/sctp/transport.c
> > > > > > +++ b/net/sctp/transport.c
> > > > > > @@ -308,8 +308,13 @@ void sctp_transport_route(struct sctp_transport
> > > > > *transport,
> > > > > > struct sctp_association *asoc = transport->asoc;
> > > > > > struct sctp_af *af = transport->af_specific;
> > > > > > + /* When dst entry is being released, route cache may be referred
> > > > > > +* again. Add rcu lock here to protect dst entry.
> > > > > > +*/
> > > > > > + rcu_read_lock();
> > > > > > sctp_transport_dst_release(transport);
> > > > > > af->get_dst(transport, saddr, >fl, sctp_opt2sk(opt));
> > > > > > + rcu_read_unlock();
> > > > > What is the exact error that syzbot reported? This doesn't seem like
> > > > > it
> > > > > fixes
> > > > BUG: KASAN: slab-out-of-bounds in rt_cache_valid+0x158/0x190
> > > > net/ipv4/route.c:1556
> > > > Read of size 2 at addr 8880654f3ac7 by task syz-executor.0/26603
> > > >
> > > > CPU: 0 PID: 26603 Comm: syz-executor.0 Not tainted 5.2.0-rc2+ #9
> > > > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> > > > Google 01/01/2011
> > > > Call Trace:
> > > > __dump_stack lib/dump_stack.c:77 [inline]
> > > > dump_stack+0x172/0x1f0 lib/dump_stack.c:113
> > > > print_address_description.cold+0x7c/0x20d mm/kasan/report.c:188
> > > > __kasan_report.cold+0x1b/0x40 mm/kasan/report.c:317
> > > > kasan_report+0x12/0x20 mm/kasan/common.c:614
> > > > __asan_report_load2_noabort+0x14/0x20 mm/kasan/generic_report.c:130
> > > > rt_cache_valid+0x158/0x190 net/ipv4/route.c:1556
> > > > __mkroute_output net/ipv4/route.c:2332 [inline]
> > > > ip_route_output_key_hash_rcu+0x819/0x2d50 net/ipv4/route.c:2564
> > > > ip_route_output_key_hash+0x1ef/0x360 net/ipv4/route.c:2393
> > > > __ip_route_output_key include/net/route.h:125 [inline]
> > > > ip_route_output_flow+0x28/0xc0 net/ipv4/route.c:2651
> > > > ip_route_output_key include/net/route.h:135 [inline]
> > > > sctp_v4_get_dst+0x467/0x1260 net/sctp/protocol.c:435
> > > > sctp_transport_route+0x12d/0x360 net/sctp/transport.c:297
> > > > sctp_assoc_add_peer+0x53e/0xfc0 net/sctp/associola.c:663
> > > > sctp_process_param net/sctp/sm_make_chunk.c:2531 [inline]
> > > > sctp_process_init+0x2491/0x2b10 net/sctp/sm_make_chunk.c:2344
> > > > sctp_cmd_process_init net/sctp/sm_sideeffect.c:667 [inline]
> > > > sctp_cmd_interpreter net/sctp/sm_sideeffect.c:1369 [inline]
> > > > sctp_side_effects net/sctp/sm_sideeffect.c:1179 [inline]
> > > > sctp_do_sm+0x3a30/0x50e0 net/sctp/sm_sideeffect.c:1150
> > > > sctp_assoc_bh_rcv+0x343/0x660 net/sctp/associola.c:1059
> > > > sctp_inq_push+0x1e4/0x280 net/sctp/inqueue.c:80
> > > > sctp_backlog_rcv+0x196/0xbe0 net/sctp/input.c:339
> > > > sk_backlog_rcv include/net/sock.h:945 [inline]
> > > > __release_sock+0x129/0x390 net/core/sock.c:2412
> > > > release_sock+0x59/0x1c0 net/core/sock.c:2928
> > > > sctp_wait_for_connect+0x316/0x540 net/sctp/socket.c:9039
> > > > __sctp_connect+0xab2/0xcd0 net/sctp/socket.c:1226
> > > > __sctp_setsockopt_connectx+0x133/0x1a0 net/sctp/socket.c:1334
> > > > sctp_setsockopt_connectx_old net/sctp/socket.c:1350 [inline]
> > > > sctp_setsockopt net/sctp/socket.c:4644 [inline]
> > > > sctp_setsockopt+0x22c0/0x6d10 net/sctp/socket.c:4608
> > > > compat_sock_common_setsockopt+0x106/0x140 net/core/sock.c:3137
> > > > __compat_sys_setsockopt+0x185/0x380
Re: [PATCH] sctp: Add rcu lock to protect dst entry in sctp_transport_route
On Tue, Jun 11, 2019 at 10:33:17AM +0800, Su Yanjun wrote:
>
> 在 2019/6/10 19:12, Neil Horman 写道:
> > On Mon, Jun 10, 2019 at 11:20:00AM +0800, Su Yanjun wrote:
> > > syzbot found a crash in rt_cache_valid. Problem is that when more
> > > threads release dst in sctp_transport_route, the route cache can
> > > be freed.
> > >
> > > As follows,
> > > p1:
> > > sctp_transport_route
> > >dst_release
> > >get_dst
> > >
> > > p2:
> > > sctp_transport_route
> > >dst_release
> > >get_dst
> > > ...
> > >
> > > If enough threads calling dst_release will cause dst->refcnt==0
> > > then rcu softirq will reclaim the dst entry,get_dst then use
> > > the freed memory.
> > >
> > > This patch adds rcu lock to protect the dst_entry here.
> > >
> > > Fixes: 6e91b578bf3f("sctp: re-use sctp_transport_pmtu in
> > sctp_transport_route")
> > > Signed-off-by: Su Yanjun
> > > Reported-by: syzbot+a9e23ea2aa21044c2...@syzkaller.appspotmail.com
> > > ---
> > > net/sctp/transport.c | 5 +
> > > 1 file changed, 5 insertions(+)
> > >
> > > diff --git a/net/sctp/transport.c b/net/sctp/transport.c
> > > index ad158d3..5ad7e20 100644
> > > --- a/net/sctp/transport.c
> > > +++ b/net/sctp/transport.c
> > > @@ -308,8 +308,13 @@ void sctp_transport_route(struct sctp_transport
> > *transport,
> > > struct sctp_association *asoc = transport->asoc;
> > > struct sctp_af *af = transport->af_specific;
> > > + /* When dst entry is being released, route cache may be referred
> > > + * again. Add rcu lock here to protect dst entry.
> > > + */
> > > + rcu_read_lock();
> > > sctp_transport_dst_release(transport);
> > > af->get_dst(transport, saddr, >fl, sctp_opt2sk(opt));
> > > + rcu_read_unlock();
> > What is the exact error that syzbot reported? This doesn't seem like it
> > fixes
> BUG: KASAN: slab-out-of-bounds in rt_cache_valid+0x158/0x190
> net/ipv4/route.c:1556
> Read of size 2 at addr 8880654f3ac7 by task syz-executor.0/26603
>
> CPU: 0 PID: 26603 Comm: syz-executor.0 Not tainted 5.2.0-rc2+ #9
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> Google 01/01/2011
> Call Trace:
> __dump_stack lib/dump_stack.c:77 [inline]
> dump_stack+0x172/0x1f0 lib/dump_stack.c:113
> print_address_description.cold+0x7c/0x20d mm/kasan/report.c:188
> __kasan_report.cold+0x1b/0x40 mm/kasan/report.c:317
> kasan_report+0x12/0x20 mm/kasan/common.c:614
> __asan_report_load2_noabort+0x14/0x20 mm/kasan/generic_report.c:130
> rt_cache_valid+0x158/0x190 net/ipv4/route.c:1556
> __mkroute_output net/ipv4/route.c:2332 [inline]
> ip_route_output_key_hash_rcu+0x819/0x2d50 net/ipv4/route.c:2564
> ip_route_output_key_hash+0x1ef/0x360 net/ipv4/route.c:2393
> __ip_route_output_key include/net/route.h:125 [inline]
> ip_route_output_flow+0x28/0xc0 net/ipv4/route.c:2651
> ip_route_output_key include/net/route.h:135 [inline]
> sctp_v4_get_dst+0x467/0x1260 net/sctp/protocol.c:435
> sctp_transport_route+0x12d/0x360 net/sctp/transport.c:297
> sctp_assoc_add_peer+0x53e/0xfc0 net/sctp/associola.c:663
> sctp_process_param net/sctp/sm_make_chunk.c:2531 [inline]
> sctp_process_init+0x2491/0x2b10 net/sctp/sm_make_chunk.c:2344
> sctp_cmd_process_init net/sctp/sm_sideeffect.c:667 [inline]
> sctp_cmd_interpreter net/sctp/sm_sideeffect.c:1369 [inline]
> sctp_side_effects net/sctp/sm_sideeffect.c:1179 [inline]
> sctp_do_sm+0x3a30/0x50e0 net/sctp/sm_sideeffect.c:1150
> sctp_assoc_bh_rcv+0x343/0x660 net/sctp/associola.c:1059
> sctp_inq_push+0x1e4/0x280 net/sctp/inqueue.c:80
> sctp_backlog_rcv+0x196/0xbe0 net/sctp/input.c:339
> sk_backlog_rcv include/net/sock.h:945 [inline]
> __release_sock+0x129/0x390 net/core/sock.c:2412
> release_sock+0x59/0x1c0 net/core/sock.c:2928
> sctp_wait_for_connect+0x316/0x540 net/sctp/socket.c:9039
> __sctp_connect+0xab2/0xcd0 net/sctp/socket.c:1226
> __sctp_setsockopt_connectx+0x133/0x1a0 net/sctp/socket.c:1334
> sctp_setsockopt_connectx_old net/sctp/socket.c:1350 [inline]
> sctp_setsockopt net/sctp/socket.c:4644 [inline]
> sctp_setsockopt+0x22c0/0x6d10 net/sctp/socket.c:4608
> compat_sock_common_setsockopt+0x106/0x140 net/core/sock.c:3137
> __compat_sys_setsockopt+0x185/0x380 net/compat.c:383
> __do_compat_sys_setsockopt net/compat.c:396 [inline]
> __se_compat_sys_setsockopt net/compat.c:393 [inline]
> __ia32_compat_sys_setsockopt+0xbd/0x150 net/compat.c:393
> do_syscall_32_irqs_on arch/x86/entry/common.c:337 [inline]
> do_fast_syscall_32+0x27b/0xd7d arch/x86/entry/common.c:408
> entry_SYSENTER_compat+0x70/0x7f arch/x86/entry/entry_64_compat.S:139
> RIP: 0023:0xf7ff5849
> Code: 85 d2 74 02 89 0a 5b 5d c3 8b 04 24 c3 8b 14 24 c3 8b 3c 24 c3 90 90
> 90 90 90 90 90 90 90 90 90 90 51 52 55 89 e5 0f 34 cd 80 <5d> 5a 59 c3 90
> 90 90 90 eb 0d 90 90 90 90 90 90 90 90 90 90 90 90
> RSP: 002b:f5df10cc EFLAGS: 0296 ORIG_RAX: 016e
> RAX: ffda RBX: 0007 RCX: 0084
> RDX:
Re: [PATCH] sctp: Add rcu lock to protect dst entry in sctp_transport_route
On Mon, Jun 10, 2019 at 11:20:00AM +0800, Su Yanjun wrote:
> syzbot found a crash in rt_cache_valid. Problem is that when more
> threads release dst in sctp_transport_route, the route cache can
> be freed.
>
> As follows,
> p1:
> sctp_transport_route
> dst_release
> get_dst
>
> p2:
> sctp_transport_route
> dst_release
> get_dst
> ...
>
> If enough threads calling dst_release will cause dst->refcnt==0
> then rcu softirq will reclaim the dst entry,get_dst then use
> the freed memory.
>
> This patch adds rcu lock to protect the dst_entry here.
>
> Fixes: 6e91b578bf3f("sctp: re-use sctp_transport_pmtu in
> sctp_transport_route")
> Signed-off-by: Su Yanjun
> Reported-by: syzbot+a9e23ea2aa21044c2...@syzkaller.appspotmail.com
> ---
> net/sctp/transport.c | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/net/sctp/transport.c b/net/sctp/transport.c
> index ad158d3..5ad7e20 100644
> --- a/net/sctp/transport.c
> +++ b/net/sctp/transport.c
> @@ -308,8 +308,13 @@ void sctp_transport_route(struct sctp_transport
> *transport,
> struct sctp_association *asoc = transport->asoc;
> struct sctp_af *af = transport->af_specific;
>
> + /* When dst entry is being released, route cache may be referred
> + * again. Add rcu lock here to protect dst entry.
> + */
> + rcu_read_lock();
> sctp_transport_dst_release(transport);
> af->get_dst(transport, saddr, >fl, sctp_opt2sk(opt));
> + rcu_read_unlock();
>
What is the exact error that syzbot reported? This doesn't seem like it fixes
anything. Based on what you've said above, we have multiple processes looking
up and releasing routes in parallel (which IIRC should never happen, as only one
process should traverse the sctp state machine for a given association at any
one time). Protecting the lookup/release operations with a read side rcu lock
won't fix that.
Neil
> if (saddr)
> memcpy(>saddr, saddr, sizeof(union sctp_addr));
> --
> 2.7.4
>
>
>
>
[PATCH] sctp: Add rcu lock to protect dst entry in sctp_transport_route
syzbot found a crash in rt_cache_valid. Problem is that when more
threads release dst in sctp_transport_route, the route cache can
be freed.
As follows,
p1:
sctp_transport_route
dst_release
get_dst
p2:
sctp_transport_route
dst_release
get_dst
...
If enough threads calling dst_release will cause dst->refcnt==0
then rcu softirq will reclaim the dst entry,get_dst then use
the freed memory.
This patch adds rcu lock to protect the dst_entry here.
Fixes: 6e91b578bf3f("sctp: re-use sctp_transport_pmtu in sctp_transport_route")
Signed-off-by: Su Yanjun
Reported-by: syzbot+a9e23ea2aa21044c2...@syzkaller.appspotmail.com
---
net/sctp/transport.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/net/sctp/transport.c b/net/sctp/transport.c
index ad158d3..5ad7e20 100644
--- a/net/sctp/transport.c
+++ b/net/sctp/transport.c
@@ -308,8 +308,13 @@ void sctp_transport_route(struct sctp_transport *transport,
struct sctp_association *asoc = transport->asoc;
struct sctp_af *af = transport->af_specific;
+ /* When dst entry is being released, route cache may be referred
+* again. Add rcu lock here to protect dst entry.
+*/
+ rcu_read_lock();
sctp_transport_dst_release(transport);
af->get_dst(transport, saddr, >fl, sctp_opt2sk(opt));
+ rcu_read_unlock();
if (saddr)
memcpy(>saddr, saddr, sizeof(union sctp_addr));
--
2.7.4
