[PATCH] crypto: af_alg - Use bh_lock_sock in sk_destruct

[PATCH] crypto: af_alg - Use bh_lock_sock in sk_destruct

[Herbert Xu]

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
this is due to a SO_ATTACH_REUSEPORT_CBPF or SO_ATTACH_REUSEPORT_EBPF
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.

---8<---
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: syzbot+c2f1558d49e25cc36e5e@syzkaller.appspotmail.com
Reported-by: Eric Dumazet <eric.dumazet@gmail.com>
Fixes: c840ac6af3f8 ("crypto: af_alg - Disallow bind/setkey/...")
Cc: <stable@vger.kernel.org>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

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)
 		sock_put(sk);
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: BUG: sleeping function called from invalid context in lock_sock_nested

[syzbot]

syzbot has found a reproducer for the following crash on:

HEAD commit:    93ad0f96 net: wan: lapbether.c: Use built-in RCU list chec..
git tree:       net
console output: https://syzkaller.appspot.com/x/log.txt?x=1159eb8ee00000
kernel config:  https://syzkaller.appspot.com/x/.config?x=7e89bd00623fe71e
dashboard link: https://syzkaller.appspot.com/bug?extid=c2f1558d49e25cc36e5e
compiler:       gcc (GCC) 9.0.0 20181231 (experimental)
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=1070cad1e00000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=17de84a5e00000

IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+c2f1558d49e25cc36e5e@syzkaller.appspotmail.com

BUG: sleeping function called from invalid context at net/core/sock.c:2935
in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 3198, name:  
kworker/0:112
4 locks held by kworker/0:112/3198:
  #0: ffff8880aa426d28 ((wq_completion)events){+.+.}, at: __write_once_size  
include/linux/compiler.h:226 [inline]
  #0: ffff8880aa426d28 ((wq_completion)events){+.+.}, at: arch_atomic64_set  
arch/x86/include/asm/atomic64_64.h:34 [inline]
  #0: ffff8880aa426d28 ((wq_completion)events){+.+.}, at: atomic64_set  
include/asm-generic/atomic-instrumented.h:855 [inline]
  #0: ffff8880aa426d28 ((wq_completion)events){+.+.}, at: atomic_long_set  
include/asm-generic/atomic-long.h:40 [inline]
  #0: ffff8880aa426d28 ((wq_completion)events){+.+.}, at: set_work_data  
kernel/workqueue.c:615 [inline]
  #0: ffff8880aa426d28 ((wq_completion)events){+.+.}, at:  
set_work_pool_and_clear_pending kernel/workqueue.c:642 [inline]
  #0: ffff8880aa426d28 ((wq_completion)events){+.+.}, at:  
process_one_work+0x88b/0x1740 kernel/workqueue.c:2235
  #1: ffffc9000951fdc0 ((work_completion)(&map->work)){+.+.}, at:  
process_one_work+0x8c1/0x1740 kernel/workqueue.c:2239
  #2: ffffffff899a3f00 (rcu_read_lock){....}, at: sock_hash_free+0x0/0x540  
net/core/sock_map.c:317
  #3: ffffc90002478d20 (&htab->buckets[i].lock){+...}, at:  
sock_hash_free+0x131/0x540 net/core/sock_map.c:865
Preemption disabled at:
[<ffffffff86341331>] sock_hash_free+0x131/0x540 net/core/sock_map.c:865
CPU: 0 PID: 3198 Comm: kworker/0:112 Not tainted 5.5.0-rc5-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS  
Google 01/01/2011
Workqueue: events bpf_map_free_deferred
Call Trace:
  __dump_stack lib/dump_stack.c:77 [inline]
  dump_stack+0x197/0x210 lib/dump_stack.c:118
  ___might_sleep.cold+0x1fb/0x23e kernel/sched/core.c:6800
  __might_sleep+0x95/0x190 kernel/sched/core.c:6753
  lock_sock_nested+0x39/0x120 net/core/sock.c:2935
  lock_sock include/net/sock.h:1531 [inline]
  sock_hash_free+0x29f/0x540 net/core/sock_map.c:868
  bpf_map_free_deferred+0xb3/0x100 kernel/bpf/syscall.c:327
  process_one_work+0x9af/0x1740 kernel/workqueue.c:2264
  worker_thread+0x98/0xe40 kernel/workqueue.c:2410
  kthread+0x361/0x430 kernel/kthread.c:255
  ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352
BUG: scheduling while atomic: kworker/0:112/3198/0x00000202
4 locks held by kworker/0:112/3198:
  #0: ffff8880aa426d28 ((wq_completion)events){+.+.}, at: __write_once_size  
include/linux/compiler.h:226 [inline]
  #0: ffff8880aa426d28 ((wq_completion)events){+.+.}, at: arch_atomic64_set  
arch/x86/include/asm/atomic64_64.h:34 [inline]
  #0: ffff8880aa426d28 ((wq_completion)events){+.+.}, at: atomic64_set  
include/asm-generic/atomic-instrumented.h:855 [inline]
  #0: ffff8880aa426d28 ((wq_completion)events){+.+.}, at: atomic_long_set  
include/asm-generic/atomic-long.h:40 [inline]
  #0: ffff8880aa426d28 ((wq_completion)events){+.+.}, at: set_work_data  
kernel/workqueue.c:615 [inline]
  #0: ffff8880aa426d28 ((wq_completion)events){+.+.}, at:  
set_work_pool_and_clear_pending kernel/workqueue.c:642 [inline]
  #0: ffff8880aa426d28 ((wq_completion)events){+.+.}, at:  
process_one_work+0x88b/0x1740 kernel/workqueue.c:2235
  #1: ffffc9000951fdc0 ((work_completion)(&map->work)){+.+.}, at:  
process_one_work+0x8c1/0x1740 kernel/workqueue.c:2239
  #2: ffffffff899a3f00 (rcu_read_lock){....}, at: sock_hash_free+0x0/0x540  
net/core/sock_map.c:317
  #3: ffffc90002478d20 (&htab->buckets[i].lock){+...}, at:  
sock_hash_free+0x131/0x540 net/core/sock_map.c:865
Modules linked in:
Preemption disabled at:
[<ffffffff86341331>] sock_hash_free+0x131/0x540 net/core/sock_map.c:865