Skip to content

Commit d61ce93

Browse files
committed
af_unix: Fix garbage collector racing against connect()
jira LE-2015 cve CVE-2024-26923 Rebuild_History Non-Buildable kernel-5.14.0-427.42.1.el9_4 commit-author Michal Luczaj <mhal@rbox.co> commit 47d8ac0 Empty-Commit: Cherry-Pick Conflicts during history rebuild. Will be included in final tarball splat. Ref for failed cherry-pick at: ciq/ciq_backports/kernel-5.14.0-427.42.1.el9_4/47d8ac01.failed Garbage collector does not take into account the risk of embryo getting enqueued during the garbage collection. If such embryo has a peer that carries SCM_RIGHTS, two consecutive passes of scan_children() may see a different set of children. Leading to an incorrectly elevated inflight count, and then a dangling pointer within the gc_inflight_list. sockets are AF_UNIX/SOCK_STREAM S is an unconnected socket L is a listening in-flight socket bound to addr, not in fdtable V's fd will be passed via sendmsg(), gets inflight count bumped connect(S, addr) sendmsg(S, [V]); close(V) __unix_gc() ---------------- ------------------------- ----------- NS = unix_create1() skb1 = sock_wmalloc(NS) L = unix_find_other(addr) unix_state_lock(L) unix_peer(S) = NS // V count=1 inflight=0 NS = unix_peer(S) skb2 = sock_alloc() skb_queue_tail(NS, skb2[V]) // V became in-flight // V count=2 inflight=1 close(V) // V count=1 inflight=1 // GC candidate condition met for u in gc_inflight_list: if (total_refs == inflight_refs) add u to gc_candidates // gc_candidates={L, V} for u in gc_candidates: scan_children(u, dec_inflight) // embryo (skb1) was not // reachable from L yet, so V's // inflight remains unchanged __skb_queue_tail(L, skb1) unix_state_unlock(L) for u in gc_candidates: if (u.inflight) scan_children(u, inc_inflight_move_tail) // V count=1 inflight=2 (!) If there is a GC-candidate listening socket, lock/unlock its state. This makes GC wait until the end of any ongoing connect() to that socket. After flipping the lock, a possibly SCM-laden embryo is already enqueued. And if there is another embryo coming, it can not possibly carry SCM_RIGHTS. At this point, unix_inflight() can not happen because unix_gc_lock is already taken. Inflight graph remains unaffected. Fixes: 1fd05ba ("[AF_UNIX]: Rewrite garbage collector, fixes race.") Signed-off-by: Michal Luczaj <mhal@rbox.co> Reviewed-by: Kuniyuki Iwashima <kuniyu@amazon.com> Link: https://lore.kernel.org/r/20240409201047.1032217-1-mhal@rbox.co Signed-off-by: Paolo Abeni <pabeni@redhat.com> (cherry picked from commit 47d8ac0) Signed-off-by: Jonathan Maple <jmaple@ciq.com> # Conflicts: # net/unix/garbage.c
1 parent aeb8056 commit d61ce93

File tree

1 file changed

+122
-0
lines changed

1 file changed

+122
-0
lines changed
Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
af_unix: Fix garbage collector racing against connect()
2+
3+
jira LE-2015
4+
cve CVE-2024-26923
5+
Rebuild_History Non-Buildable kernel-5.14.0-427.42.1.el9_4
6+
commit-author Michal Luczaj <mhal@rbox.co>
7+
commit 47d8ac011fe1c9251070e1bd64cb10b48193ec51
8+
Empty-Commit: Cherry-Pick Conflicts during history rebuild.
9+
Will be included in final tarball splat. Ref for failed cherry-pick at:
10+
ciq/ciq_backports/kernel-5.14.0-427.42.1.el9_4/47d8ac01.failed
11+
12+
Garbage collector does not take into account the risk of embryo getting
13+
enqueued during the garbage collection. If such embryo has a peer that
14+
carries SCM_RIGHTS, two consecutive passes of scan_children() may see a
15+
different set of children. Leading to an incorrectly elevated inflight
16+
count, and then a dangling pointer within the gc_inflight_list.
17+
18+
sockets are AF_UNIX/SOCK_STREAM
19+
S is an unconnected socket
20+
L is a listening in-flight socket bound to addr, not in fdtable
21+
V's fd will be passed via sendmsg(), gets inflight count bumped
22+
23+
connect(S, addr) sendmsg(S, [V]); close(V) __unix_gc()
24+
---------------- ------------------------- -----------
25+
26+
NS = unix_create1()
27+
skb1 = sock_wmalloc(NS)
28+
L = unix_find_other(addr)
29+
unix_state_lock(L)
30+
unix_peer(S) = NS
31+
// V count=1 inflight=0
32+
33+
NS = unix_peer(S)
34+
skb2 = sock_alloc()
35+
skb_queue_tail(NS, skb2[V])
36+
37+
// V became in-flight
38+
// V count=2 inflight=1
39+
40+
close(V)
41+
42+
// V count=1 inflight=1
43+
// GC candidate condition met
44+
45+
for u in gc_inflight_list:
46+
if (total_refs == inflight_refs)
47+
add u to gc_candidates
48+
49+
// gc_candidates={L, V}
50+
51+
for u in gc_candidates:
52+
scan_children(u, dec_inflight)
53+
54+
// embryo (skb1) was not
55+
// reachable from L yet, so V's
56+
// inflight remains unchanged
57+
__skb_queue_tail(L, skb1)
58+
unix_state_unlock(L)
59+
for u in gc_candidates:
60+
if (u.inflight)
61+
scan_children(u, inc_inflight_move_tail)
62+
63+
// V count=1 inflight=2 (!)
64+
65+
If there is a GC-candidate listening socket, lock/unlock its state. This
66+
makes GC wait until the end of any ongoing connect() to that socket. After
67+
flipping the lock, a possibly SCM-laden embryo is already enqueued. And if
68+
there is another embryo coming, it can not possibly carry SCM_RIGHTS. At
69+
this point, unix_inflight() can not happen because unix_gc_lock is already
70+
taken. Inflight graph remains unaffected.
71+
72+
Fixes: 1fd05ba5a2f2 ("[AF_UNIX]: Rewrite garbage collector, fixes race.")
73+
Signed-off-by: Michal Luczaj <mhal@rbox.co>
74+
Reviewed-by: Kuniyuki Iwashima <kuniyu@amazon.com>
75+
Link: https://lore.kernel.org/r/20240409201047.1032217-1-mhal@rbox.co
76+
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
77+
(cherry picked from commit 47d8ac011fe1c9251070e1bd64cb10b48193ec51)
78+
Signed-off-by: Jonathan Maple <jmaple@ciq.com>
79+
80+
# Conflicts:
81+
# net/unix/garbage.c
82+
diff --cc net/unix/garbage.c
83+
index 2405f0f9af31,6433a414acf8..000000000000
84+
--- a/net/unix/garbage.c
85+
+++ b/net/unix/garbage.c
86+
@@@ -234,17 -274,26 +234,32 @@@ void unix_gc(void
87+
* receive queues. Other, non candidate sockets _can_ be
88+
* added to queue, so we must make sure only to touch
89+
* candidates.
90+
+ *
91+
+ * Embryos, though never candidates themselves, affect which
92+
+ * candidates are reachable by the garbage collector. Before
93+
+ * being added to a listener's queue, an embryo may already
94+
+ * receive data carrying SCM_RIGHTS, potentially making the
95+
+ * passed socket a candidate that is not yet reachable by the
96+
+ * collector. It becomes reachable once the embryo is
97+
+ * enqueued. Therefore, we must ensure that no SCM-laden
98+
+ * embryo appears in a (candidate) listener's queue between
99+
+ * consecutive scan_children() calls.
100+
*/
101+
list_for_each_entry_safe(u, next, &gc_inflight_list, link) {
102+
+ struct sock *sk = &u->sk;
103+
long total_refs;
104+
+ long inflight_refs;
105+
106+
++<<<<<<< HEAD
107+
+ total_refs = file_count(u->sk.sk_socket->file);
108+
+ inflight_refs = atomic_long_read(&u->inflight);
109+
++=======
110+
+ total_refs = file_count(sk->sk_socket->file);
111+
++>>>>>>> 47d8ac011fe1 (af_unix: Fix garbage collector racing against connect())
112+
113+
- WARN_ON_ONCE(!u->inflight);
114+
- WARN_ON_ONCE(total_refs < u->inflight);
115+
- if (total_refs == u->inflight) {
116+
+ BUG_ON(inflight_refs < 1);
117+
+ BUG_ON(total_refs < inflight_refs);
118+
+ if (total_refs == inflight_refs) {
119+
list_move_tail(&u->link, &gc_candidates);
120+
__set_bit(UNIX_GC_CANDIDATE, &u->gc_flags);
121+
__set_bit(UNIX_GC_MAYBE_CYCLE, &u->gc_flags);
122+
* Unmerged path net/unix/garbage.c

0 commit comments

Comments
 (0)