KASAN: use-after-free Read in rds_tcp_dev_event

[syzbot]

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 ffff8801cd879200 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
notifier_call_chain+0x136/0x2c0 kernel/notifier.c:93
__raw_notifier_call_chain kernel/notifier.c:394 [inline]
raw_notifier_call_chain+0x2d/0x40 kernel/notifier.c:401
call_netdevice_notifiers_info+0x51/0x90 net/core/dev.c:1671
call_netdevice_notifiers net/core/dev.c:1687 [inline]
netdev_run_todo+0x437/0xca0 net/core/dev.c:7846
rtnl_unlock+0xe/0x10 net/core/rtnetlink.c:106
default_device_exit_batch+0x4b4/0x5d0 net/core/dev.c:8664
ops_exit_list.isra.6+0x100/0x150 net/core/net_namespace.c:145
cleanup_net+0x5c7/0xb50 net/core/net_namespace.c:483
process_one_work+0xbf0/0x1bc0 kernel/workqueue.c:2113
worker_thread+0x223/0x1990 kernel/workqueue.c:2247
kthread+0x35e/0x430 kernel/kthread.c:231
ret_from_fork+0x2a/0x40 arch/x86/entry/entry_64.S:431

Allocated by task 30987:
save_stack_trace+0x16/0x20 arch/x86/kernel/stacktrace.c:59
save_stack+0x43/0xd0 mm/kasan/kasan.c:447
set_track mm/kasan/kasan.c:459 [inline]
kasan_kmalloc+0xad/0xe0 mm/kasan/kasan.c:551
__do_kmalloc_node mm/slab.c:3689 [inline]
__kmalloc_node_track_caller+0x47/0x70 mm/slab.c:3703
__kmalloc_reserve.isra.40+0x41/0xd0 net/core/skbuff.c:138
__alloc_skb+0x13b/0x740 net/core/skbuff.c:206
alloc_skb include/linux/skbuff.h:976 [inline]
netlink_dump+0x2ba/0xb90 net/netlink/af_netlink.c:2162
netlink_recvmsg+0x9b6/0x1300 net/netlink/af_netlink.c:1945
sock_recvmsg_nosec net/socket.c:806 [inline]
sock_recvmsg+0xc9/0x110 net/socket.c:813
SYSC_recvfrom+0x2d6/0x570 net/socket.c:1802
SyS_recvfrom+0x40/0x50 net/socket.c:1774
entry_SYSCALL_64_fastpath+0x1f/0xbe

Freed by task 30987:
save_stack_trace+0x16/0x20 arch/x86/kernel/stacktrace.c:59
save_stack+0x43/0xd0 mm/kasan/kasan.c:447
set_track mm/kasan/kasan.c:459 [inline]
kasan_slab_free+0x71/0xc0 mm/kasan/kasan.c:524
__cache_free mm/slab.c:3503 [inline]
kfree+0xca/0x250 mm/slab.c:3820
skb_free_head+0x74/0xb0 net/core/skbuff.c:554
skb_release_data+0x58c/0x790 net/core/skbuff.c:574
skb_release_all+0x4a/0x60 net/core/skbuff.c:631
__kfree_skb net/core/skbuff.c:645 [inline]
consume_skb+0x153/0x490 net/core/skbuff.c:705
skb_free_datagram+0x1a/0xe0 net/core/datagram.c:330
netlink_recvmsg+0x5c6/0x1300 net/netlink/af_netlink.c:1941
sock_recvmsg_nosec net/socket.c:806 [inline]
sock_recvmsg+0xc9/0x110 net/socket.c:813
SYSC_recvfrom+0x2d6/0x570 net/socket.c:1802
SyS_recvfrom+0x40/0x50 net/socket.c:1774
entry_SYSCALL_64_fastpath+0x1f/0xbe

The buggy address belongs to the object at ffff8801cd879200
which belongs to the cache kmalloc-8192 of size 8192
The buggy address is located 0 bytes inside of
8192-byte region [ffff8801cd879200, ffff8801cd87b200)
The buggy address belongs to the page:
page:ffffea0007361e00 count:1 mapcount:0 mapping:ffff8801cd879200 index:0x0
compound_mapcount: 0
flags: 0x200000000008100(slab|head)
raw: 0200000000008100 ffff8801cd879200 0000000000000000 0000000100000001
raw: ffffea0007120720 ffffea0006757420 ffff8801dac02080 0000000000000000
page dumped because: kasan: bad access detected

Memory state around the buggy address:
ffff8801cd879100: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
ffff8801cd879180: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> ffff8801cd879200: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
^
ffff8801cd879280: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff8801cd879300: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
==================================================================


---
This bug is generated by a dumb bot. It may contain errors.
See https://goo.gl/tpsmEJ for details.
Direct all questions to syzk...@googlegroups.com.
Please credit me with: Reported-by: syzbot <syzk...@googlegroups.com>

syzbot will keep track of this bug report.
Once a fix for this bug is committed, please reply to this email with:
#syz fix: exact-commit-title
To mark this as a duplicate of another syzbot report, please reply with:
#syz dup: exact-subject-of-another-report
If it's a one-off invalid bug report, please reply with:
#syz invalid
Note: if the crash happens again, it will cause creation of a new bug
report.
Note: all commands must start from beginning of the line.

[Girish Moodalbail]

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 ffff8801cd879200 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(&rds_tcp_conn_lock);
529 list_for_each_entry_safe(tc, _tc, &rds_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(&tmp_list, tc->t_cpath->cp_conn))
535 list_move_tail(&tc->t_tcp_node, &tmp_list);
536 }
537 spin_unlock_irq(&rds_tcp_conn_lock);
538 list_for_each_entry_safe(tc, _tc, &tmp_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

[Sowmini Varadhan]

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

[Sowmini Varadhan]

A few questions.

- First off, why am I not seeing the original mail in this thread
even when I search the mail archives, e.g.,
https://lkml.org/lkml/2017/11/13/954

- Girish Moodalbail writes:

> 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).

The netns is not "long gone", we are still processing
the NETDEV_UNREGISTER_FINAL for loopback. As I said in my
earlier mail, the idea is to extract the list of unique conns
that belong to the netns and then destroy both the conn, and
all associated paths. Thus there can only be a single thread
going through rds_tcp_kill_sock at any time (since we should
only get the unregister_final/loopback one time for the netns).
(See alos comment block in rds_tcp_dev_event about network activity
quiescing). Thus there should be no concurrency issue.

However when I just ehecked this, there may be some rds connection
refcounting bug. When I quickly tested this, I'm not seeing the
expected calls to conn_path_destroy. I'll need some time to take
a look, this has been known to work, so something got broken along
the way

> 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):

let's first understand the real root-cause before we start
redesigning data-structures.

--Sowmini

[Dmitry Vyukov]

On Tue, Nov 14, 2017 at 2:22 PM, Sowmini Varadhan
<sowmini....@oracle.com> wrote:
>
>
> A few questions.
>
> - First off, why am I not seeing the original mail in this thread
> even when I search the mail archives, e.g.,
> https://lkml.org/lkml/2017/11/13/954

Good question!

The original message was sent to 4 kernel mailing lists:

---------- Forwarded message ----------
From: syzbot <bot+643ecad3f5bb49700e...@syzkaller.appspotmail.com>
Date: Tue, Nov 7, 2017 at 9:28 PM
Subject: KASAN: use-after-free Read in rds_tcp_dev_event
To: davem@, linux-...@vger.kernel.org, linux...@vger.kernel.org,
net...@vger.kernel.org, rds-...@oss.oracle.com, santosh.shilimkar@,
syzkall...@googlegroups.com

but does not appear to appear on any of them. Only on syzkaller-bugs list:
https://groups.google.com/d/msg/syzkaller-bugs/nEeIAsNLWL4/1GzamOmRAwAJ

I guess it was bounced for some reason. Does anybody know why it could
happen with this particular email? Or how to contact mailing list
owners? LKML faq mentions some Majordomo-owner, but I can't find any
details.

The email wasn't HTML, no multipart/alternative, no windows encoding,
subject did not match taboo regexps as subsequent emails appear on the
lists....

Unfortunately I don't have original (I received a duplicate Message-ID
from mailing list and my email client decided to store only the dup).
Here it is:
https://gist.githubusercontent.com/dvyukov/9b4e19605a80ebd103d94f69631a645c/raw/974891f90ef873a4bf568187c1a85ffe83f50b5a/gistfile1.txt

It indeed contains Mailing-list/List-ID headers which could have been
bounced. But I have original of another syzbot email received
directly, it gives idea about original headers and they don't contain
any mailing list headers:
https://gist.githubusercontent.com/dvyukov/9034b567ed6a2a88aec4d5cce1a0b2ac/raw/c81b4b4efde0d44983d263e594a1fc746c556607/gistfile1.txt

[Dmitry Vyukov]

Humm... I see that another message from syzbot (generated the same
way) did appear on lkml:
https://lkml.org/lkml/2017/11/8/132

[Girish Moodalbail]

On 11/14/17 5:22 AM, Sowmini Varadhan wrote:
>
>
> A few questions.
>
> - First off, why am I not seeing the original mail in this thread
> even when I search the mail archives, e.g.,
> https://lkml.org/lkml/2017/11/13/954
>
> - Girish Moodalbail writes:
>
>> 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).
>
> The netns is not "long gone", we are still processing
> the NETDEV_UNREGISTER_FINAL for loopback.

Obviously, I was not talking about the current namespace.

Say there are two namespaces - ns1 and ns2 and that both have RDS connections.
Deletion of ns1 will be fine. However when ns2 is being deleted, in the
rds_tcp_dev_event() callback we walk through the global list and some nodes in
that list will be referring to ns1 (that is "long gone"). If you read my earlier
email, I was talking about ns1 which is already gone, and we are trying to
access it from ns2.

~Girish

[Dmitry Vyukov]

This seems to be fixed with:

#syz fix: rds: tcp: correctly sequence cleanup on netns deletion.