netdev - [PATCH v1 net-next 14/16] af_unix: Remove scm_fp_dup() in unix_attach

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]

Date: Fri, 2 Feb 2024 19:00:56 -0800
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 14/16] af_unix: Remove scm_fp_dup() in unix_attach_fds().

When we passed fds, we used to bump each file's recount twice before
linking the socket to a global list, gc_inflight_list.  Otherwise, the
inflight socket could have been garbage-collected before queuing skb.

Previously, we linked the inflight socket to the list in advance and
later queued the skb holding the socket.  So, there was a small race
window like this:

  CPU 1              CPU 2             CPU 3
  -----              -----             -----

  /* Here A's refcount is 1, and inflight count is 0 */

  bump A's refcount to 2
  bump A's inflight count to 1
  link A to list
                                       close(A) decreases
                                        A's refcount to 1
                     GC starts and
                      mark A as candidate
                      as refcount == inflight count

However, we no longer publish an inflight socket to GC before queueing
skb, so we can just move scm->fp to skb without cloning.

Signed-off-by: Kuniyuki Iwashima <kuniyu@...zon.com>
---
 net/unix/af_unix.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 0b70f7b5b5a8..9022a3a5dccc 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -1807,13 +1807,8 @@ static int unix_attach_fds(struct scm_cookie *scm, struct sk_buff *skb)
 	if (too_many_unix_fds(current))
 		return -ETOOMANYREFS;
 
-	/* Need to duplicate file references for the sake of garbage
-	 * collection.  Otherwise a socket in the fps might become a
-	 * candidate for GC while the skb is not yet queued.
-	 */
-	UNIXCB(skb).fp = scm_fp_dup(scm->fp);
-	if (!UNIXCB(skb).fp)
-		return -ENOMEM;
+	UNIXCB(skb).fp = scm->fp;
+	scm->fp = NULL;
 
 	if (unix_alloc_edges(UNIXCB(skb).fp))
 		return -ENOMEM;
-- 
2.30.2