Re: KASAN: use-after-free Read in rds_tcp_dev_event
On Tue, Nov 14, 2017 at 4:30 AM, Girish Moodalbailwrote: > On 11/7/17 12:28 PM, syzbot wrote: >> >> Hello, >> >> syzkaller hit the following crash on >> 287683d027a3ff83feb6c7044430c79881664ecf >> git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/master >> compiler: gcc (GCC) 7.1.1 20170620 >> .config is attached >> Raw console output is attached. >> >> >> >> >> == >> BUG: KASAN: use-after-free in rds_tcp_kill_sock net/rds/tcp.c:530 [inline] >> BUG: KASAN: use-after-free in rds_tcp_dev_event+0xc01/0xc90 >> net/rds/tcp.c:568 >> Read of size 8 at addr 8801cd879200 by task kworker/u4:3/147 >> >> CPU: 0 PID: 147 Comm: kworker/u4:3 Not tainted 4.14.0-rc7+ #156 >> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS >> Google 01/01/2011 >> Workqueue: netns cleanup_net >> Call Trace: >> __dump_stack lib/dump_stack.c:16 [inline] >> dump_stack+0x194/0x257 lib/dump_stack.c:52 >> print_address_description+0x73/0x250 mm/kasan/report.c:252 >> kasan_report_error mm/kasan/report.c:351 [inline] >> kasan_report+0x25b/0x340 mm/kasan/report.c:409 >> __asan_report_load8_noabort+0x14/0x20 mm/kasan/report.c:430 >> rds_tcp_kill_sock net/rds/tcp.c:530 [inline] >> rds_tcp_dev_event+0xc01/0xc90 net/rds/tcp.c:568 > > > The issue here is that we are trying to reference a network namespace > (struct net *) that is long gone (i.e., L532 below -- c_net is the culprit). > > 528 spin_lock_irq(_tcp_conn_lock); > 529 list_for_each_entry_safe(tc, _tc, _tcp_conn_list, > t_tcp_node) { > 530 struct net *c_net = tc->t_cpath->cp_conn->c_net; > 531 > 532 if (net != c_net || !tc->t_sock) > 533 continue; > 534 if (!list_has_conn(_list, tc->t_cpath->cp_conn)) > 535 list_move_tail(>t_tcp_node, _list); > 536 } > 537 spin_unlock_irq(_tcp_conn_lock); > 538 list_for_each_entry_safe(tc, _tc, _list, t_tcp_node) { > 539 rds_tcp_conn_paths_destroy(tc->t_cpath->cp_conn); > 540 rds_conn_destroy(tc->t_cpath->cp_conn); > 541 } > > When a network namespace is deleted, devices within that namespace are > unregistered and removed one by one. RDS is notified about this event > through rds_tcp_dev_event() callback. When the loopback device is removed > from the namespace, the above RDS callback function destroys all the RDS > connections in that namespace. > > The loop@L529 above walks through each of the rds_tcp connection in the > global list (rds_tcp_conn_list) to see if that connection belongs to the > namespace in question. It collects all such connections and destroys them > (L538-540). However, it leaves behind some of the rds_tcp connections that > shared the same underlying RDS connection (L534 and 535). These connections > with pointer to stale network namespace are left behind in the global list. > When the 2nd network namespace is deleted, we will hit the above stale > pointer and hit UAF panic. > > I think we should move away from global list to a per-namespace list. The > global list are used only in two places (both of which are per-namespace > operations): > > - to destroy all the RDS connections belonging to a namespace when the >network namespace is being deleted. > - to reset all the RDS connections when socket parameters for a namespace > are >modified using sysctl > > Thanks, > ~Girish This seems to be fixed with: #syz fix: rds: tcp: correctly sequence cleanup on netns deletion.
Re: KASAN: use-after-free Read in rds_tcp_dev_event
On Tue, Nov 14, 2017 at 4:30 AM, Girish Moodalbail wrote: > On 11/7/17 12:28 PM, syzbot wrote: >> >> Hello, >> >> syzkaller hit the following crash on >> 287683d027a3ff83feb6c7044430c79881664ecf >> git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/master >> compiler: gcc (GCC) 7.1.1 20170620 >> .config is attached >> Raw console output is attached. >> >> >> >> >> == >> BUG: KASAN: use-after-free in rds_tcp_kill_sock net/rds/tcp.c:530 [inline] >> BUG: KASAN: use-after-free in rds_tcp_dev_event+0xc01/0xc90 >> net/rds/tcp.c:568 >> Read of size 8 at addr 8801cd879200 by task kworker/u4:3/147 >> >> CPU: 0 PID: 147 Comm: kworker/u4:3 Not tainted 4.14.0-rc7+ #156 >> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS >> Google 01/01/2011 >> Workqueue: netns cleanup_net >> Call Trace: >> __dump_stack lib/dump_stack.c:16 [inline] >> dump_stack+0x194/0x257 lib/dump_stack.c:52 >> print_address_description+0x73/0x250 mm/kasan/report.c:252 >> kasan_report_error mm/kasan/report.c:351 [inline] >> kasan_report+0x25b/0x340 mm/kasan/report.c:409 >> __asan_report_load8_noabort+0x14/0x20 mm/kasan/report.c:430 >> rds_tcp_kill_sock net/rds/tcp.c:530 [inline] >> rds_tcp_dev_event+0xc01/0xc90 net/rds/tcp.c:568 > > > The issue here is that we are trying to reference a network namespace > (struct net *) that is long gone (i.e., L532 below -- c_net is the culprit). > > 528 spin_lock_irq(_tcp_conn_lock); > 529 list_for_each_entry_safe(tc, _tc, _tcp_conn_list, > t_tcp_node) { > 530 struct net *c_net = tc->t_cpath->cp_conn->c_net; > 531 > 532 if (net != c_net || !tc->t_sock) > 533 continue; > 534 if (!list_has_conn(_list, tc->t_cpath->cp_conn)) > 535 list_move_tail(>t_tcp_node, _list); > 536 } > 537 spin_unlock_irq(_tcp_conn_lock); > 538 list_for_each_entry_safe(tc, _tc, _list, t_tcp_node) { > 539 rds_tcp_conn_paths_destroy(tc->t_cpath->cp_conn); > 540 rds_conn_destroy(tc->t_cpath->cp_conn); > 541 } > > When a network namespace is deleted, devices within that namespace are > unregistered and removed one by one. RDS is notified about this event > through rds_tcp_dev_event() callback. When the loopback device is removed > from the namespace, the above RDS callback function destroys all the RDS > connections in that namespace. > > The loop@L529 above walks through each of the rds_tcp connection in the > global list (rds_tcp_conn_list) to see if that connection belongs to the > namespace in question. It collects all such connections and destroys them > (L538-540). However, it leaves behind some of the rds_tcp connections that > shared the same underlying RDS connection (L534 and 535). These connections > with pointer to stale network namespace are left behind in the global list. > When the 2nd network namespace is deleted, we will hit the above stale > pointer and hit UAF panic. > > I think we should move away from global list to a per-namespace list. The > global list are used only in two places (both of which are per-namespace > operations): > > - to destroy all the RDS connections belonging to a namespace when the >network namespace is being deleted. > - to reset all the RDS connections when socket parameters for a namespace > are >modified using sysctl > > Thanks, > ~Girish This seems to be fixed with: #syz fix: rds: tcp: correctly sequence cleanup on netns deletion.
Re: KASAN: use-after-free Read in rds_tcp_dev_event
On (11/13/17 19:30), Girish Moodalbail wrote: > (L538-540). However, it leaves behind some of the rds_tcp connections that > shared the same underlying RDS connection (L534 and 535). These connections > with pointer to stale network namespace are left behind in the global list. It leaves behind no such thing. After mprds, you want to collect only one instance of the conn that is being removed, that's why lines 534-535 skips over duplicat instances of the same conn (for multiple paths in the same conn). > When the 2nd network namespace is deleted, we will hit the above stale > pointer and hit UAF panic. > I think we should move away from global list to a per-namespace list. The > global list are used only in two places (both of which are per-namespace > operations): Nice try, but not so. Let me look at this tomorrow, I missed this mail in my mbox. --Sowmini
Re: KASAN: use-after-free Read in rds_tcp_dev_event
On (11/13/17 19:30), Girish Moodalbail wrote: > (L538-540). However, it leaves behind some of the rds_tcp connections that > shared the same underlying RDS connection (L534 and 535). These connections > with pointer to stale network namespace are left behind in the global list. It leaves behind no such thing. After mprds, you want to collect only one instance of the conn that is being removed, that's why lines 534-535 skips over duplicat instances of the same conn (for multiple paths in the same conn). > When the 2nd network namespace is deleted, we will hit the above stale > pointer and hit UAF panic. > I think we should move away from global list to a per-namespace list. The > global list are used only in two places (both of which are per-namespace > operations): Nice try, but not so. Let me look at this tomorrow, I missed this mail in my mbox. --Sowmini
Re: KASAN: use-after-free Read in rds_tcp_dev_event
On 11/7/17 12:28 PM, syzbot wrote: Hello, syzkaller hit the following crash on 287683d027a3ff83feb6c7044430c79881664ecf git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/master compiler: gcc (GCC) 7.1.1 20170620 .config is attached Raw console output is attached. == BUG: KASAN: use-after-free in rds_tcp_kill_sock net/rds/tcp.c:530 [inline] BUG: KASAN: use-after-free in rds_tcp_dev_event+0xc01/0xc90 net/rds/tcp.c:568 Read of size 8 at addr 8801cd879200 by task kworker/u4:3/147 CPU: 0 PID: 147 Comm: kworker/u4:3 Not tainted 4.14.0-rc7+ #156 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 Workqueue: netns cleanup_net Call Trace: __dump_stack lib/dump_stack.c:16 [inline] dump_stack+0x194/0x257 lib/dump_stack.c:52 print_address_description+0x73/0x250 mm/kasan/report.c:252 kasan_report_error mm/kasan/report.c:351 [inline] kasan_report+0x25b/0x340 mm/kasan/report.c:409 __asan_report_load8_noabort+0x14/0x20 mm/kasan/report.c:430 rds_tcp_kill_sock net/rds/tcp.c:530 [inline] rds_tcp_dev_event+0xc01/0xc90 net/rds/tcp.c:568 The issue here is that we are trying to reference a network namespace (struct net *) that is long gone (i.e., L532 below -- c_net is the culprit). 528 spin_lock_irq(_tcp_conn_lock); 529 list_for_each_entry_safe(tc, _tc, _tcp_conn_list, t_tcp_node) { 530 struct net *c_net = tc->t_cpath->cp_conn->c_net; 531 532 if (net != c_net || !tc->t_sock) 533 continue; 534 if (!list_has_conn(_list, tc->t_cpath->cp_conn)) 535 list_move_tail(>t_tcp_node, _list); 536 } 537 spin_unlock_irq(_tcp_conn_lock); 538 list_for_each_entry_safe(tc, _tc, _list, t_tcp_node) { 539 rds_tcp_conn_paths_destroy(tc->t_cpath->cp_conn); 540 rds_conn_destroy(tc->t_cpath->cp_conn); 541 } When a network namespace is deleted, devices within that namespace are unregistered and removed one by one. RDS is notified about this event through rds_tcp_dev_event() callback. When the loopback device is removed from the namespace, the above RDS callback function destroys all the RDS connections in that namespace. The loop@L529 above walks through each of the rds_tcp connection in the global list (rds_tcp_conn_list) to see if that connection belongs to the namespace in question. It collects all such connections and destroys them (L538-540). However, it leaves behind some of the rds_tcp connections that shared the same underlying RDS connection (L534 and 535). These connections with pointer to stale network namespace are left behind in the global list. When the 2nd network namespace is deleted, we will hit the above stale pointer and hit UAF panic. I think we should move away from global list to a per-namespace list. The global list are used only in two places (both of which are per-namespace operations): - to destroy all the RDS connections belonging to a namespace when the network namespace is being deleted. - to reset all the RDS connections when socket parameters for a namespace are modified using sysctl Thanks, ~Girish
Re: KASAN: use-after-free Read in rds_tcp_dev_event
On 11/7/17 12:28 PM, syzbot wrote: Hello, syzkaller hit the following crash on 287683d027a3ff83feb6c7044430c79881664ecf git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/master compiler: gcc (GCC) 7.1.1 20170620 .config is attached Raw console output is attached. == BUG: KASAN: use-after-free in rds_tcp_kill_sock net/rds/tcp.c:530 [inline] BUG: KASAN: use-after-free in rds_tcp_dev_event+0xc01/0xc90 net/rds/tcp.c:568 Read of size 8 at addr 8801cd879200 by task kworker/u4:3/147 CPU: 0 PID: 147 Comm: kworker/u4:3 Not tainted 4.14.0-rc7+ #156 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 Workqueue: netns cleanup_net Call Trace: __dump_stack lib/dump_stack.c:16 [inline] dump_stack+0x194/0x257 lib/dump_stack.c:52 print_address_description+0x73/0x250 mm/kasan/report.c:252 kasan_report_error mm/kasan/report.c:351 [inline] kasan_report+0x25b/0x340 mm/kasan/report.c:409 __asan_report_load8_noabort+0x14/0x20 mm/kasan/report.c:430 rds_tcp_kill_sock net/rds/tcp.c:530 [inline] rds_tcp_dev_event+0xc01/0xc90 net/rds/tcp.c:568 The issue here is that we are trying to reference a network namespace (struct net *) that is long gone (i.e., L532 below -- c_net is the culprit). 528 spin_lock_irq(_tcp_conn_lock); 529 list_for_each_entry_safe(tc, _tc, _tcp_conn_list, t_tcp_node) { 530 struct net *c_net = tc->t_cpath->cp_conn->c_net; 531 532 if (net != c_net || !tc->t_sock) 533 continue; 534 if (!list_has_conn(_list, tc->t_cpath->cp_conn)) 535 list_move_tail(>t_tcp_node, _list); 536 } 537 spin_unlock_irq(_tcp_conn_lock); 538 list_for_each_entry_safe(tc, _tc, _list, t_tcp_node) { 539 rds_tcp_conn_paths_destroy(tc->t_cpath->cp_conn); 540 rds_conn_destroy(tc->t_cpath->cp_conn); 541 } When a network namespace is deleted, devices within that namespace are unregistered and removed one by one. RDS is notified about this event through rds_tcp_dev_event() callback. When the loopback device is removed from the namespace, the above RDS callback function destroys all the RDS connections in that namespace. The loop@L529 above walks through each of the rds_tcp connection in the global list (rds_tcp_conn_list) to see if that connection belongs to the namespace in question. It collects all such connections and destroys them (L538-540). However, it leaves behind some of the rds_tcp connections that shared the same underlying RDS connection (L534 and 535). These connections with pointer to stale network namespace are left behind in the global list. When the 2nd network namespace is deleted, we will hit the above stale pointer and hit UAF panic. I think we should move away from global list to a per-namespace list. The global list are used only in two places (both of which are per-namespace operations): - to destroy all the RDS connections belonging to a namespace when the network namespace is being deleted. - to reset all the RDS connections when socket parameters for a namespace are modified using sysctl Thanks, ~Girish