netdev - [PATCH v1 net-next 2/2] af_unix: Don't hold client's sk_peer_lock in copy

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: <20240128103732.18185-3-kuniyu@amazon.com>
Date: Sun, 28 Jan 2024 02:37:32 -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: Kent Overstreet <kent.overstreet@...ux.dev>, Kuniyuki Iwashima
	<kuniyu@...zon.com>, Kuniyuki Iwashima <kuni1840@...il.com>,
	<netdev@...r.kernel.org>
Subject: [PATCH v1 net-next 2/2] af_unix: Don't hold client's sk_peer_lock in copy_peercred().

When (AF_UNIX, SOCK_STREAM) socket connect()s to a listening socket,
the listener's sk_peer_pid/sk_peer_cred are copied to the client in
copy_peercred().

Then, two sk_peer_locks are held there; one is client's and another
is listener's.  However, we need not hold the client's lock.

The only place where the client's sk_peer_pid/sk_peer_cred are set
is copy_peercred(), and once they are set, they never change.

Let's not hold client's sk_peer_lock in copy_peercred().

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

diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index f07374d31f7c..c5c292393be8 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -706,26 +706,10 @@ static void update_peercred(struct sock *sk)
 
 static void copy_peercred(struct sock *sk, struct sock *peersk)
 {
-	const struct cred *old_cred;
-	struct pid *old_pid;
-
-	if (sk < peersk) {
-		spin_lock(&sk->sk_peer_lock);
-		spin_lock_nested(&peersk->sk_peer_lock, SINGLE_DEPTH_NESTING);
-	} else {
-		spin_lock(&peersk->sk_peer_lock);
-		spin_lock_nested(&sk->sk_peer_lock, SINGLE_DEPTH_NESTING);
-	}
-	old_pid = sk->sk_peer_pid;
-	old_cred = sk->sk_peer_cred;
-	sk->sk_peer_pid  = get_pid(peersk->sk_peer_pid);
+	spin_lock(&peersk->sk_peer_lock);
+	sk->sk_peer_pid = get_pid(peersk->sk_peer_pid);
 	sk->sk_peer_cred = get_cred(peersk->sk_peer_cred);
-
-	spin_unlock(&sk->sk_peer_lock);
 	spin_unlock(&peersk->sk_peer_lock);
-
-	put_pid(old_pid);
-	put_cred(old_cred);
 }
 
 static int unix_listen(struct socket *sock, int backlog)
-- 
2.30.2