mirror of
https://github.com/torvalds/linux.git
synced 2025-12-01 07:26:02 +07:00
af_unix: Initialise scc_index in unix_add_edge().
Quang Le reported that the AF_UNIX GC could garbage-collect a
receive queue of an alive in-flight socket, with a nice repro.
The repro consists of three stages.
1)
1-a. Create a single cyclic reference with many sockets
1-b. close() all sockets
1-c. Trigger GC
2)
2-a. Pass sk-A to an embryo sk-B
2-b. Pass sk-X to sk-X
2-c. Trigger GC
3)
3-a. accept() the embryo sk-B
3-b. Pass sk-B to sk-C
3-c. close() the in-flight sk-A
3-d. Trigger GC
As of 2-c, sk-A and sk-X are linked to unix_unvisited_vertices,
and unix_walk_scc() groups them into two different SCCs:
unix_sk(sk-A)->vertex->scc_index = 2 (UNIX_VERTEX_INDEX_START)
unix_sk(sk-X)->vertex->scc_index = 3
Once GC completes, unix_graph_grouped is set to true.
Also, unix_graph_maybe_cyclic is set to true due to sk-X's
cyclic self-reference, which makes close() trigger GC.
At 3-b, unix_add_edge() allocates unix_sk(sk-B)->vertex and
links it to unix_unvisited_vertices.
unix_update_graph() is called at 3-a. and 3-b., but neither
unix_graph_grouped nor unix_graph_maybe_cyclic is changed
because both sk-B's listener and sk-C are not in-flight.
3-c decrements sk-A's file refcnt to 1.
Since unix_graph_grouped is true at 3-d, unix_walk_scc_fast()
is finally called and iterates 3 sockets sk-A, sk-B, and sk-X:
sk-A -> sk-B (-> sk-C)
sk-X -> sk-X
This is totally fine. All of them are not yet close()d and
should be grouped into different SCCs.
However, unix_vertex_dead() misjudges that sk-A and sk-B are
in the same SCC and sk-A is dead.
unix_sk(sk-A)->scc_index == unix_sk(sk-B)->scc_index <-- Wrong!
&&
sk-A's file refcnt == unix_sk(sk-A)->vertex->out_degree
^-- 1 in-flight count for sk-B
-> sk-A is dead !?
The problem is that unix_add_edge() does not initialise scc_index.
Stage 1) is used for heap spraying, making a newly allocated
vertex have vertex->scc_index == 2 (UNIX_VERTEX_INDEX_START)
set by unix_walk_scc() at 1-c.
Let's track the max SCC index from the previous unix_walk_scc()
call and assign the max + 1 to a new vertex's scc_index.
This way, we can continue to avoid Tarjan's algorithm while
preventing misjudgments.
Fixes: ad081928a8 ("af_unix: Avoid Tarjan's algorithm if unnecessary.")
Reported-by: Quang Le <quanglex97@gmail.com>
Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
Link: https://patch.msgid.link/20251109025233.3659187-1-kuniyu@google.com
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
This commit is contained in:
committed by
Paolo Abeni
parent
8c0726e861
commit
60e6489f8e
@@ -145,6 +145,7 @@ enum unix_vertex_index {
|
|||||||
};
|
};
|
||||||
|
|
||||||
static unsigned long unix_vertex_unvisited_index = UNIX_VERTEX_INDEX_MARK1;
|
static unsigned long unix_vertex_unvisited_index = UNIX_VERTEX_INDEX_MARK1;
|
||||||
|
static unsigned long unix_vertex_max_scc_index = UNIX_VERTEX_INDEX_START;
|
||||||
|
|
||||||
static void unix_add_edge(struct scm_fp_list *fpl, struct unix_edge *edge)
|
static void unix_add_edge(struct scm_fp_list *fpl, struct unix_edge *edge)
|
||||||
{
|
{
|
||||||
@@ -153,6 +154,7 @@ static void unix_add_edge(struct scm_fp_list *fpl, struct unix_edge *edge)
|
|||||||
if (!vertex) {
|
if (!vertex) {
|
||||||
vertex = list_first_entry(&fpl->vertices, typeof(*vertex), entry);
|
vertex = list_first_entry(&fpl->vertices, typeof(*vertex), entry);
|
||||||
vertex->index = unix_vertex_unvisited_index;
|
vertex->index = unix_vertex_unvisited_index;
|
||||||
|
vertex->scc_index = ++unix_vertex_max_scc_index;
|
||||||
vertex->out_degree = 0;
|
vertex->out_degree = 0;
|
||||||
INIT_LIST_HEAD(&vertex->edges);
|
INIT_LIST_HEAD(&vertex->edges);
|
||||||
INIT_LIST_HEAD(&vertex->scc_entry);
|
INIT_LIST_HEAD(&vertex->scc_entry);
|
||||||
@@ -489,10 +491,15 @@ prev_vertex:
|
|||||||
scc_dead = unix_vertex_dead(v);
|
scc_dead = unix_vertex_dead(v);
|
||||||
}
|
}
|
||||||
|
|
||||||
if (scc_dead)
|
if (scc_dead) {
|
||||||
unix_collect_skb(&scc, hitlist);
|
unix_collect_skb(&scc, hitlist);
|
||||||
else if (!unix_graph_maybe_cyclic)
|
} else {
|
||||||
unix_graph_maybe_cyclic = unix_scc_cyclic(&scc);
|
if (unix_vertex_max_scc_index < vertex->scc_index)
|
||||||
|
unix_vertex_max_scc_index = vertex->scc_index;
|
||||||
|
|
||||||
|
if (!unix_graph_maybe_cyclic)
|
||||||
|
unix_graph_maybe_cyclic = unix_scc_cyclic(&scc);
|
||||||
|
}
|
||||||
|
|
||||||
list_del(&scc);
|
list_del(&scc);
|
||||||
}
|
}
|
||||||
@@ -507,6 +514,7 @@ static void unix_walk_scc(struct sk_buff_head *hitlist)
|
|||||||
unsigned long last_index = UNIX_VERTEX_INDEX_START;
|
unsigned long last_index = UNIX_VERTEX_INDEX_START;
|
||||||
|
|
||||||
unix_graph_maybe_cyclic = false;
|
unix_graph_maybe_cyclic = false;
|
||||||
|
unix_vertex_max_scc_index = UNIX_VERTEX_INDEX_START;
|
||||||
|
|
||||||
/* Visit every vertex exactly once.
|
/* Visit every vertex exactly once.
|
||||||
* __unix_walk_scc() moves visited vertices to unix_visited_vertices.
|
* __unix_walk_scc() moves visited vertices to unix_visited_vertices.
|
||||||
|
|||||||
Reference in New Issue
Block a user