netdev - [PATCH v1 net-next 5/6] af_unix: Schedule GC based on graph state during sendmsg().

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20240503223150.6035-6-kuniyu@amazon.com>
Date: Fri, 3 May 2024 15:31:49 -0700
From: Kuniyuki Iwashima <kuniyu@...zon.com>
To: "David S. Miller" <davem@...emloft.net>, Eric Dumazet
	<edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni
	<pabeni@...hat.com>
CC: Kuniyuki Iwashima <kuniyu@...zon.com>, Kuniyuki Iwashima
	<kuni1840@...il.com>, <netdev@...r.kernel.org>
Subject: [PATCH v1 net-next 5/6] af_unix: Schedule GC based on graph state during sendmsg().

The conventional test to trigger GC was based on the number of inflight
sockets.

Now we have more reliable data indicating if the loop exists in the graph.

When the graph state is

  1. UNIX_GRAPH_NOT_CYCLIC, do not scheudle GC
  2. UNIX_GRAPH_MAYBE_CYCLIC, schedule GC if unix_tot_inflight > 16384
  3. UNIX_GRAPH_CYCLIC, schedule GC if unix_graph_circles > 1024

1024 might sound much smaller than 16384, but if the number of loops
is larger than 1024, there must be something wrong.

Signed-off-by: Kuniyuki Iwashima <kuniyu@...zon.com>
---
 net/unix/garbage.c | 44 ++++++++++++++++++++++++++++----------------
 1 file changed, 28 insertions(+), 16 deletions(-)

diff --git a/net/unix/garbage.c b/net/unix/garbage.c
index 85c0500764d4..48cea3cf4a42 100644
--- a/net/unix/garbage.c
+++ b/net/unix/garbage.c
@@ -128,7 +128,7 @@ static void unix_update_graph(struct unix_vertex *vertex)
 	if (!vertex)
 		return;
 
-	unix_graph_state = UNIX_GRAPH_MAYBE_CYCLIC;
+	WRITE_ONCE(unix_graph_state, UNIX_GRAPH_MAYBE_CYCLIC);
 }
 
 static LIST_HEAD(unix_unvisited_vertices);
@@ -533,7 +533,8 @@ static void unix_walk_scc(struct sk_buff_head *hitlist)
 	list_replace_init(&unix_visited_vertices, &unix_unvisited_vertices);
 	swap(unix_vertex_unvisited_index, unix_vertex_grouped_index);
 
-	unix_graph_state = unix_graph_circles ? UNIX_GRAPH_CYCLIC : UNIX_GRAPH_NOT_CYCLIC;
+	WRITE_ONCE(unix_graph_state,
+		   unix_graph_circles ? UNIX_GRAPH_CYCLIC : UNIX_GRAPH_NOT_CYCLIC);
 }
 
 static void unix_walk_scc_fast(struct sk_buff_head *hitlist)
@@ -555,7 +556,7 @@ static void unix_walk_scc_fast(struct sk_buff_head *hitlist)
 
 		if (scc_dead) {
 			unix_collect_skb(&scc, hitlist);
-			unix_graph_circles--;
+			WRITE_ONCE(unix_graph_circles, unix_graph_circles - 1);
 		}
 
 		list_del(&scc);
@@ -564,7 +565,7 @@ static void unix_walk_scc_fast(struct sk_buff_head *hitlist)
 	list_replace_init(&unix_visited_vertices, &unix_unvisited_vertices);
 
 	if (!unix_graph_circles)
-		unix_graph_state = UNIX_GRAPH_NOT_CYCLIC;
+		WRITE_ONCE(unix_graph_state, UNIX_GRAPH_NOT_CYCLIC);
 }
 
 static bool gc_in_progress;
@@ -608,27 +609,38 @@ void unix_gc(void)
 	queue_work(system_unbound_wq, &unix_gc_work);
 }
 
-#define UNIX_INFLIGHT_TRIGGER_GC 16000
+#define UNIX_INFLIGHT_SANE_CIRCLES (1 << 10)
+#define UNIX_INFLIGHT_SANE_SOCKETS (1 << 14)
 #define UNIX_INFLIGHT_SANE_USER (SCM_MAX_FD * 8)
 
 static void __unix_schedule_gc(struct scm_fp_list *fpl)
 {
-	/* If number of inflight sockets is insane,
-	 * force a garbage collect right now.
-	 *
-	 * Paired with the WRITE_ONCE() in unix_inflight(),
-	 * unix_notinflight(), and __unix_gc().
+	unsigned char graph_state = READ_ONCE(unix_graph_state);
+	bool wait = false;
+
+	if (graph_state == UNIX_GRAPH_NOT_CYCLIC)
+		return;
+
+	/* If the number of inflight sockets or cyclic references
+	 * is insane, schedule garbage collector if not running.
 	 */
-	if (READ_ONCE(unix_tot_inflight) > UNIX_INFLIGHT_TRIGGER_GC &&
-	    !READ_ONCE(gc_in_progress))
-		unix_gc();
+	if (graph_state == UNIX_GRAPH_CYCLIC) {
+		if (READ_ONCE(unix_graph_circles) < UNIX_INFLIGHT_SANE_CIRCLES)
+			return;
+	} else {
+		if (READ_ONCE(unix_tot_inflight) < UNIX_INFLIGHT_SANE_SOCKETS)
+			return;
+	}
 
 	/* Penalise users who want to send AF_UNIX sockets
 	 * but whose sockets have not been received yet.
 	 */
-	if (READ_ONCE(fpl->user->unix_inflight) < UNIX_INFLIGHT_SANE_USER)
-		return;
+	if (READ_ONCE(fpl->user->unix_inflight) > UNIX_INFLIGHT_SANE_USER)
+		wait = true;
+
+	if (!READ_ONCE(gc_in_progress))
+		unix_gc();
 
-	if (READ_ONCE(gc_in_progress))
+	if (wait)
 		flush_work(&unix_gc_work);
 }
-- 
2.30.2