crypto: af_alg - Use bh_lock_sock in sk_destruct
diff mbox series

Message ID
State Accepted
Commit 37f96694cf73ba116993a9d2d99ad6a75fa7fdb0
Headers show
  • crypto: af_alg - Use bh_lock_sock in sk_destruct
Related show

Commit Message

Herbert Xu Dec. 5, 2019, 5:45 a.m. UTC
On Wed, Dec 04, 2019 at 08:59:11PM -0800, Eric Dumazet wrote:
> crypto layer (hash_sock_destruct()) is called from rcu callback (this in BH context) but tries to grab a socket lock.
> A socket lock can schedule, which is illegal in BH context.

Fair enough.  Although I was rather intrigued as to how the RCU call
occured in the first place.  After some digging my theory is that
setsockopt on the crypto socket.

What are these filters even suppposed to do on an af_alg socket?

Anyhow, this is a bug that could have been triggered even without
this, but it would have been almost impossible to do it through
syzbot as you need to have an outstanding async skcipher/aead request
that is freed in BH context.

As af_alg_release_parent may be called from BH context (most notably
due to an async request that only completes after socket closure,
or as reported here because of an RCU-delayed sk_destruct call), we
must use bh_lock_sock instead of lock_sock.

Reported-by: Eric Dumazet <>
Fixes: c840ac6af3f8 ("crypto: af_alg - Disallow bind/setkey/...")
Cc: <>
Signed-off-by: Herbert Xu <>

diff mbox series

diff --git a/crypto/af_alg.c b/crypto/af_alg.c
index 0dceaabc6321..3d8e53010cda 100644
--- a/crypto/af_alg.c
+++ b/crypto/af_alg.c
@@ -134,11 +134,13 @@  void af_alg_release_parent(struct sock *sk)
 	sk = ask->parent;
 	ask = alg_sk(sk);
-	lock_sock(sk);
+	local_bh_disable();
+	bh_lock_sock(sk);
 	ask->nokey_refcnt -= nokey;
 	if (!last)
 		last = !--ask->refcnt;
-	release_sock(sk);
+	bh_unlock_sock(sk);
+	local_bh_enable();
 	if (last)