[bpf] bpf, sockmap: RCU splat with TLS redirect and strparser

Message ID 159303296342.360.5487181450879978407.stgit@john-XPS-13-9370
State New
Headers show
Series
  • [bpf] bpf, sockmap: RCU splat with TLS redirect and strparser
Related show

Commit Message

John Fastabend June 24, 2020, 9:09 p.m.
Redirect on non-TLS sockmap side has RCU lock held from sockmap code
path but when called from TLS this is no longer true. The RCU section
is needed because we use rcu dereference to fetch the psock of the
socket we are redirecting to.

To fix the below splat wrap the redirect usage in rcu_read_lock,
rcu_read_unlock.

[ 1095.937597] WARNING: suspicious RCU usage
[ 1095.940964] 5.7.0-rc7-02911-g463bac5f1ca79 #1 Tainted: G        W
[ 1095.944363] -----------------------------
[ 1095.947384] include/linux/skmsg.h:284 suspicious rcu_dereference_check() usage!
[ 1095.950866]
[ 1095.950866] other info that might help us debug this:
[ 1095.950866]
[ 1095.957146]
[ 1095.957146] rcu_scheduler_active = 2, debug_locks = 1
[ 1095.961482] 1 lock held by test_sockmap/15970:
[ 1095.964501]  #0: ffff9ea6b25de660 (sk_lock-AF_INET){+.+.}-{0:0}, at: tls_sw_recvmsg+0x13a/0x840 [tls]
[ 1095.968568]
[ 1095.968568] stack backtrace:
[ 1095.975001] CPU: 1 PID: 15970 Comm: test_sockmap Tainted: G        W         5.7.0-rc7-02911-g463bac5f1ca79 #1
[ 1095.977883] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.12.0-1 04/01/2014
[ 1095.980519] Call Trace:
[ 1095.982191]  dump_stack+0x8f/0xd0
[ 1095.984040]  sk_psock_skb_redirect+0xa6/0xf0
[ 1095.986073]  sk_psock_tls_strp_read+0x1d8/0x250
[ 1095.988095]  tls_sw_recvmsg+0x714/0x840 [tls]

Fixes: e91de6afa81c1 ("bpf: Fix running sk_skb program types with ktls")
Reported-by: Jakub Sitnicki <jakub@cloudflare.com>
Reported-by: kernel test robot <rong.a.chen@intel.com>
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
 net/core/skmsg.c |    3 +++
 1 file changed, 3 insertions(+)

Patch

diff --git a/net/core/skmsg.c b/net/core/skmsg.c
index 351afbf6bfba..070553fa3900 100644
--- a/net/core/skmsg.c
+++ b/net/core/skmsg.c
@@ -694,9 +694,11 @@  static void sk_psock_skb_redirect(struct sk_psock *psock, struct sk_buff *skb)
 		kfree_skb(skb);
 		return;
 	}
+	rcu_read_lock();
 	psock_other = sk_psock(sk_other);
 	if (!psock_other || sock_flag(sk_other, SOCK_DEAD) ||
 	    !sk_psock_test_state(psock_other, SK_PSOCK_TX_ENABLED)) {
+		rcu_read_unlock();
 		kfree_skb(skb);
 		return;
 	}
@@ -713,6 +715,7 @@  static void sk_psock_skb_redirect(struct sk_psock *psock, struct sk_buff *skb)
 	} else {
 		kfree_skb(skb);
 	}
+	rcu_read_unlock();
 }
 
 static void sk_psock_tls_verdict_apply(struct sk_psock *psock,