| From 12f59370cd443f198c89d9b1957351255d4b62e5 Mon Sep 17 00:00:00 2001 |
| From: Andrey Ulanov <andreyu@google.com> |
| Date: Tue, 14 Mar 2017 20:16:42 -0700 |
| Subject: [PATCH] net: unix: properly re-increment inflight counter of GC |
| discarded candidates |
| |
| commit 7df9c24625b9981779afb8fcdbe2bb4765e61147 upstream. |
| |
| Dmitry has reported that a BUG_ON() condition in unix_notinflight() |
| may be triggered by a simple code that forwards unix socket in an |
| SCM_RIGHTS message. |
| That is caused by incorrect unix socket GC implementation in unix_gc(). |
| |
| The GC first collects list of candidates, then (a) decrements their |
| "children's" inflight counter, (b) checks which inflight counters are |
| now 0, and then (c) increments all inflight counters back. |
| (a) and (c) are done by calling scan_children() with inc_inflight or |
| dec_inflight as the second argument. |
| |
| Commit 6209344f5a37 ("net: unix: fix inflight counting bug in garbage |
| collector") changed scan_children() such that it no longer considers |
| sockets that do not have UNIX_GC_CANDIDATE flag. It also added a block |
| of code that that unsets this flag _before_ invoking |
| scan_children(, dec_iflight, ). This may lead to incorrect inflight |
| counters for some sockets. |
| |
| This change fixes this bug by changing order of operations: |
| UNIX_GC_CANDIDATE is now unset only after all inflight counters are |
| restored to the original state. |
| |
| kernel BUG at net/unix/garbage.c:149! |
| RIP: 0010:[<ffffffff8717ebf4>] [<ffffffff8717ebf4>] |
| unix_notinflight+0x3b4/0x490 net/unix/garbage.c:149 |
| Call Trace: |
| [<ffffffff8716cfbf>] unix_detach_fds.isra.19+0xff/0x170 net/unix/af_unix.c:1487 |
| [<ffffffff8716f6a9>] unix_destruct_scm+0xf9/0x210 net/unix/af_unix.c:1496 |
| [<ffffffff86a90a01>] skb_release_head_state+0x101/0x200 net/core/skbuff.c:655 |
| [<ffffffff86a9808a>] skb_release_all+0x1a/0x60 net/core/skbuff.c:668 |
| [<ffffffff86a980ea>] __kfree_skb+0x1a/0x30 net/core/skbuff.c:684 |
| [<ffffffff86a98284>] kfree_skb+0x184/0x570 net/core/skbuff.c:705 |
| [<ffffffff871789d5>] unix_release_sock+0x5b5/0xbd0 net/unix/af_unix.c:559 |
| [<ffffffff87179039>] unix_release+0x49/0x90 net/unix/af_unix.c:836 |
| [<ffffffff86a694b2>] sock_release+0x92/0x1f0 net/socket.c:570 |
| [<ffffffff86a6962b>] sock_close+0x1b/0x20 net/socket.c:1017 |
| [<ffffffff81a76b8e>] __fput+0x34e/0x910 fs/file_table.c:208 |
| [<ffffffff81a771da>] ____fput+0x1a/0x20 fs/file_table.c:244 |
| [<ffffffff81483ab0>] task_work_run+0x1a0/0x280 kernel/task_work.c:116 |
| [< inline >] exit_task_work include/linux/task_work.h:21 |
| [<ffffffff8141287a>] do_exit+0x183a/0x2640 kernel/exit.c:828 |
| [<ffffffff8141383e>] do_group_exit+0x14e/0x420 kernel/exit.c:931 |
| [<ffffffff814429d3>] get_signal+0x663/0x1880 kernel/signal.c:2307 |
| [<ffffffff81239b45>] do_signal+0xc5/0x2190 arch/x86/kernel/signal.c:807 |
| [<ffffffff8100666a>] exit_to_usermode_loop+0x1ea/0x2d0 |
| arch/x86/entry/common.c:156 |
| [< inline >] prepare_exit_to_usermode arch/x86/entry/common.c:190 |
| [<ffffffff81009693>] syscall_return_slowpath+0x4d3/0x570 |
| arch/x86/entry/common.c:259 |
| [<ffffffff881478e6>] entry_SYSCALL_64_fastpath+0xc4/0xc6 |
| |
| Link: https://lkml.org/lkml/2017/3/6/252 |
| Signed-off-by: Andrey Ulanov <andreyu@google.com> |
| Reported-by: Dmitry Vyukov <dvyukov@google.com> |
| Fixes: 6209344 ("net: unix: fix inflight counting bug in garbage collector") |
| Signed-off-by: David S. Miller <davem@davemloft.net> |
| Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com> |
| |
| diff --git a/net/unix/garbage.c b/net/unix/garbage.c |
| index 6a0d48525fcf..c36757e72844 100644 |
| --- a/net/unix/garbage.c |
| +++ b/net/unix/garbage.c |
| @@ -146,6 +146,7 @@ void unix_notinflight(struct user_struct *user, struct file *fp) |
| if (s) { |
| struct unix_sock *u = unix_sk(s); |
| |
| + BUG_ON(!atomic_long_read(&u->inflight)); |
| BUG_ON(list_empty(&u->link)); |
| |
| if (atomic_long_dec_and_test(&u->inflight)) |
| @@ -341,6 +342,14 @@ void unix_gc(void) |
| } |
| list_del(&cursor); |
| |
| + /* Now gc_candidates contains only garbage. Restore original |
| + * inflight counters for these as well, and remove the skbuffs |
| + * which are creating the cycle(s). |
| + */ |
| + skb_queue_head_init(&hitlist); |
| + list_for_each_entry(u, &gc_candidates, link) |
| + scan_children(&u->sk, inc_inflight, &hitlist); |
| + |
| /* not_cycle_list contains those sockets which do not make up a |
| * cycle. Restore these to the inflight list. |
| */ |
| @@ -350,14 +359,6 @@ void unix_gc(void) |
| list_move_tail(&u->link, &gc_inflight_list); |
| } |
| |
| - /* Now gc_candidates contains only garbage. Restore original |
| - * inflight counters for these as well, and remove the skbuffs |
| - * which are creating the cycle(s). |
| - */ |
| - skb_queue_head_init(&hitlist); |
| - list_for_each_entry(u, &gc_candidates, link) |
| - scan_children(&u->sk, inc_inflight, &hitlist); |
| - |
| spin_unlock(&unix_gc_lock); |
| |
| /* Here we are. Hitlist is filled. Die. */ |
| -- |
| 2.12.0 |
| |