]>
Commit | Line | Data |
---|---|---|
ad3041a8 GKH |
1 | From foo@baz Mon Mar 27 18:18:08 CEST 2017 |
2 | From: Andrey Ulanov <andreyu@google.com> | |
3 | Date: Tue, 14 Mar 2017 20:16:42 -0700 | |
4 | Subject: net: unix: properly re-increment inflight counter of GC discarded candidates | |
5 | ||
6 | From: Andrey Ulanov <andreyu@google.com> | |
7 | ||
8 | ||
9 | [ Upstream commit 7df9c24625b9981779afb8fcdbe2bb4765e61147 ] | |
10 | ||
11 | Dmitry has reported that a BUG_ON() condition in unix_notinflight() | |
12 | may be triggered by a simple code that forwards unix socket in an | |
13 | SCM_RIGHTS message. | |
14 | That is caused by incorrect unix socket GC implementation in unix_gc(). | |
15 | ||
16 | The GC first collects list of candidates, then (a) decrements their | |
17 | "children's" inflight counter, (b) checks which inflight counters are | |
18 | now 0, and then (c) increments all inflight counters back. | |
19 | (a) and (c) are done by calling scan_children() with inc_inflight or | |
20 | dec_inflight as the second argument. | |
21 | ||
22 | Commit 6209344f5a37 ("net: unix: fix inflight counting bug in garbage | |
23 | collector") changed scan_children() such that it no longer considers | |
24 | sockets that do not have UNIX_GC_CANDIDATE flag. It also added a block | |
25 | of code that that unsets this flag _before_ invoking | |
26 | scan_children(, dec_iflight, ). This may lead to incorrect inflight | |
27 | counters for some sockets. | |
28 | ||
29 | This change fixes this bug by changing order of operations: | |
30 | UNIX_GC_CANDIDATE is now unset only after all inflight counters are | |
31 | restored to the original state. | |
32 | ||
33 | kernel BUG at net/unix/garbage.c:149! | |
34 | RIP: 0010:[<ffffffff8717ebf4>] [<ffffffff8717ebf4>] | |
35 | unix_notinflight+0x3b4/0x490 net/unix/garbage.c:149 | |
36 | Call Trace: | |
37 | [<ffffffff8716cfbf>] unix_detach_fds.isra.19+0xff/0x170 net/unix/af_unix.c:1487 | |
38 | [<ffffffff8716f6a9>] unix_destruct_scm+0xf9/0x210 net/unix/af_unix.c:1496 | |
39 | [<ffffffff86a90a01>] skb_release_head_state+0x101/0x200 net/core/skbuff.c:655 | |
40 | [<ffffffff86a9808a>] skb_release_all+0x1a/0x60 net/core/skbuff.c:668 | |
41 | [<ffffffff86a980ea>] __kfree_skb+0x1a/0x30 net/core/skbuff.c:684 | |
42 | [<ffffffff86a98284>] kfree_skb+0x184/0x570 net/core/skbuff.c:705 | |
43 | [<ffffffff871789d5>] unix_release_sock+0x5b5/0xbd0 net/unix/af_unix.c:559 | |
44 | [<ffffffff87179039>] unix_release+0x49/0x90 net/unix/af_unix.c:836 | |
45 | [<ffffffff86a694b2>] sock_release+0x92/0x1f0 net/socket.c:570 | |
46 | [<ffffffff86a6962b>] sock_close+0x1b/0x20 net/socket.c:1017 | |
47 | [<ffffffff81a76b8e>] __fput+0x34e/0x910 fs/file_table.c:208 | |
48 | [<ffffffff81a771da>] ____fput+0x1a/0x20 fs/file_table.c:244 | |
49 | [<ffffffff81483ab0>] task_work_run+0x1a0/0x280 kernel/task_work.c:116 | |
50 | [< inline >] exit_task_work include/linux/task_work.h:21 | |
51 | [<ffffffff8141287a>] do_exit+0x183a/0x2640 kernel/exit.c:828 | |
52 | [<ffffffff8141383e>] do_group_exit+0x14e/0x420 kernel/exit.c:931 | |
53 | [<ffffffff814429d3>] get_signal+0x663/0x1880 kernel/signal.c:2307 | |
54 | [<ffffffff81239b45>] do_signal+0xc5/0x2190 arch/x86/kernel/signal.c:807 | |
55 | [<ffffffff8100666a>] exit_to_usermode_loop+0x1ea/0x2d0 | |
56 | arch/x86/entry/common.c:156 | |
57 | [< inline >] prepare_exit_to_usermode arch/x86/entry/common.c:190 | |
58 | [<ffffffff81009693>] syscall_return_slowpath+0x4d3/0x570 | |
59 | arch/x86/entry/common.c:259 | |
60 | [<ffffffff881478e6>] entry_SYSCALL_64_fastpath+0xc4/0xc6 | |
61 | ||
62 | Link: https://lkml.org/lkml/2017/3/6/252 | |
63 | Signed-off-by: Andrey Ulanov <andreyu@google.com> | |
64 | Reported-by: Dmitry Vyukov <dvyukov@google.com> | |
65 | Fixes: 6209344 ("net: unix: fix inflight counting bug in garbage collector") | |
66 | Signed-off-by: David S. Miller <davem@davemloft.net> | |
67 | Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> | |
68 | --- | |
69 | net/unix/garbage.c | 17 +++++++++-------- | |
70 | 1 file changed, 9 insertions(+), 8 deletions(-) | |
71 | ||
72 | --- a/net/unix/garbage.c | |
73 | +++ b/net/unix/garbage.c | |
74 | @@ -146,6 +146,7 @@ void unix_notinflight(struct user_struct | |
75 | if (s) { | |
76 | struct unix_sock *u = unix_sk(s); | |
77 | ||
78 | + BUG_ON(!atomic_long_read(&u->inflight)); | |
79 | BUG_ON(list_empty(&u->link)); | |
80 | ||
81 | if (atomic_long_dec_and_test(&u->inflight)) | |
82 | @@ -341,6 +342,14 @@ void unix_gc(void) | |
83 | } | |
84 | list_del(&cursor); | |
85 | ||
86 | + /* Now gc_candidates contains only garbage. Restore original | |
87 | + * inflight counters for these as well, and remove the skbuffs | |
88 | + * which are creating the cycle(s). | |
89 | + */ | |
90 | + skb_queue_head_init(&hitlist); | |
91 | + list_for_each_entry(u, &gc_candidates, link) | |
92 | + scan_children(&u->sk, inc_inflight, &hitlist); | |
93 | + | |
94 | /* not_cycle_list contains those sockets which do not make up a | |
95 | * cycle. Restore these to the inflight list. | |
96 | */ | |
97 | @@ -350,14 +359,6 @@ void unix_gc(void) | |
98 | list_move_tail(&u->link, &gc_inflight_list); | |
99 | } | |
100 | ||
101 | - /* Now gc_candidates contains only garbage. Restore original | |
102 | - * inflight counters for these as well, and remove the skbuffs | |
103 | - * which are creating the cycle(s). | |
104 | - */ | |
105 | - skb_queue_head_init(&hitlist); | |
106 | - list_for_each_entry(u, &gc_candidates, link) | |
107 | - scan_children(&u->sk, inc_inflight, &hitlist); | |
108 | - | |
109 | spin_unlock(&unix_gc_lock); | |
110 | ||
111 | /* Here we are. Hitlist is filled. Die. */ |