| Message ID | 20241230193430.3148259-1-edumazet@google.com (mailing list archive) |
|---|---|
| State | Deferred |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net] net: restrict SO_REUSEPORT to TCP, UDP and SCTP sockets | expand |
On Mon, 30 Dec 2024 19:34:30 +0000 Eric Dumazet wrote: > After blamed commit, crypto sockets could accidentally be destroyed > from RCU callback, as spotted by zyzbot [1]. > > Trying to acquire a mutex in RCU callback is not allowed. > > Restrict SO_REUSEPORT socket option to TCP, UDP and SCTP sockets. Looks like fcnal_test.sh and reuseport_addr_any.sh are failing after this patch, we need to adjust their respective binaries. I'll hide this patch from patchwork, even tho it's probably right..
On Tue, Dec 31, 2024 at 1:07 AM Jakub Kicinski <kuba@kernel.org> wrote: > > On Mon, 30 Dec 2024 19:34:30 +0000 Eric Dumazet wrote: > > After blamed commit, crypto sockets could accidentally be destroyed > > from RCU callback, as spotted by zyzbot [1]. > > > > Trying to acquire a mutex in RCU callback is not allowed. > > > > Restrict SO_REUSEPORT socket option to TCP, UDP and SCTP sockets. > > Looks like fcnal_test.sh and reuseport_addr_any.sh are failing > after this patch, we need to adjust their respective binaries. > I'll hide this patch from patchwork, even tho it's probably right.. It seems we should support raw sockets, they already use SOCK_RCU_FREE anyway. Although sk_reuseport_attach_bpf() has the following checks : if ((sk->sk_type != SOCK_STREAM && sk->sk_type != SOCK_DGRAM) || (sk->sk_protocol != IPPROTO_UDP && sk->sk_protocol != IPPROTO_TCP) || (sk->sk_family != AF_INET && sk->sk_family != AF_INET6)) { err = -ENOTSUPP; goto err_prog_put; } (BTW SCTP is not supported there, although SCTP got reuseport support in linux-6.0)
On Mon, 30 Dec 2024 19:34:30 +0000 Eric Dumazet <edumazet@google.com> wrote: > After blamed commit, crypto sockets could accidentally be destroyed > from RCU callback, as spotted by zyzbot [1]. > > Trying to acquire a mutex in RCU callback is not allowed. > > Restrict SO_REUSEPORT socket option to TCP, UDP and SCTP sockets. > ... > > Fixes: 8c7138b33e5c ("net: Unpublish sk from sk_reuseport_cb before call_rcu") > Reported-by: syzbot+b3e02953598f447d4d2a@syzkaller.appspotmail.com > Closes: https://lore.kernel.org/netdev/6772f2f4.050a0220.2f3838.04cb.GAE@google.com/T/#u > Signed-off-by: Eric Dumazet <edumazet@google.com> > Cc: Martin KaFai Lau <kafai@fb.com> > --- > include/net/sock.h | 7 +++++++ > net/core/sock.c | 6 +++++- > 2 files changed, 12 insertions(+), 1 deletion(-) > > diff --git a/include/net/sock.h b/include/net/sock.h > index 7464e9f9f47c..4010fd759e2a 100644 > --- a/include/net/sock.h > +++ b/include/net/sock.h > @@ -2730,6 +2730,13 @@ static inline bool sk_is_tcp(const struct sock *sk) > sk->sk_protocol == IPPROTO_TCP; > } > > +static inline bool sk_is_sctp(const struct sock *sk) > +{ > + return sk_is_inet(sk) && > + sk->sk_type == SOCK_STREAM && > + sk->sk_protocol == IPPROTO_SCTP; > +} Isn't SCTP SOCK_SEQPACKET ? In any case the sk_type test is redundant (for inet sockets). If support is needed for raw (ip) sockets is it enough to just check sk_is_inet() ? David > + > static inline bool sk_is_udp(const struct sock *sk) > { > return sk_is_inet(sk) && > diff --git a/net/core/sock.c b/net/core/sock.c > index 74729d20cd00..56e8517da8dc 100644 > --- a/net/core/sock.c > +++ b/net/core/sock.c > @@ -1295,7 +1295,11 @@ int sk_setsockopt(struct sock *sk, int level, int optname, > sk->sk_reuse = (valbool ? SK_CAN_REUSE : SK_NO_REUSE); > break; > case SO_REUSEPORT: > - sk->sk_reuseport = valbool; > + if (valbool && !sk_is_tcp(sk) && !sk_is_udp(sk) && > + !sk_is_sctp(sk)) > + ret = -EOPNOTSUPP; > + else > + sk->sk_reuseport = valbool; > break; > case SO_DONTROUTE: > sock_valbool_flag(sk, SOCK_LOCALROUTE, valbool);
diff --git a/include/net/sock.h b/include/net/sock.h index 7464e9f9f47c..4010fd759e2a 100644 --- a/include/net/sock.h +++ b/include/net/sock.h @@ -2730,6 +2730,13 @@ static inline bool sk_is_tcp(const struct sock *sk) sk->sk_protocol == IPPROTO_TCP; } +static inline bool sk_is_sctp(const struct sock *sk) +{ + return sk_is_inet(sk) && + sk->sk_type == SOCK_STREAM && + sk->sk_protocol == IPPROTO_SCTP; +} + static inline bool sk_is_udp(const struct sock *sk) { return sk_is_inet(sk) && diff --git a/net/core/sock.c b/net/core/sock.c index 74729d20cd00..56e8517da8dc 100644 --- a/net/core/sock.c +++ b/net/core/sock.c @@ -1295,7 +1295,11 @@ int sk_setsockopt(struct sock *sk, int level, int optname, sk->sk_reuse = (valbool ? SK_CAN_REUSE : SK_NO_REUSE); break; case SO_REUSEPORT: - sk->sk_reuseport = valbool; + if (valbool && !sk_is_tcp(sk) && !sk_is_udp(sk) && + !sk_is_sctp(sk)) + ret = -EOPNOTSUPP; + else + sk->sk_reuseport = valbool; break; case SO_DONTROUTE: sock_valbool_flag(sk, SOCK_LOCALROUTE, valbool);
After blamed commit, crypto sockets could accidentally be destroyed from RCU callback, as spotted by zyzbot [1]. Trying to acquire a mutex in RCU callback is not allowed. Restrict SO_REUSEPORT socket option to TCP, UDP and SCTP sockets. [1] BUG: sleeping function called from invalid context at kernel/locking/mutex.c:562 in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 24, name: ksoftirqd/1 preempt_count: 100, expected: 0 RCU nest depth: 0, expected: 0 1 lock held by ksoftirqd/1/24: #0: ffffffff8e937ba0 (rcu_callback){....}-{0:0}, at: rcu_lock_acquire include/linux/rcupdate.h:337 [inline] #0: ffffffff8e937ba0 (rcu_callback){....}-{0:0}, at: rcu_do_batch kernel/rcu/tree.c:2561 [inline] #0: ffffffff8e937ba0 (rcu_callback){....}-{0:0}, at: rcu_core+0xa37/0x17a0 kernel/rcu/tree.c:2823 Preemption disabled at: [<ffffffff8161c8c8>] softirq_handle_begin kernel/softirq.c:402 [inline] [<ffffffff8161c8c8>] handle_softirqs+0x128/0x9b0 kernel/softirq.c:537 CPU: 1 UID: 0 PID: 24 Comm: ksoftirqd/1 Not tainted 6.13.0-rc3-syzkaller-00174-ga024e377efed #0 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 09/13/2024 Call Trace: <TASK> __dump_stack lib/dump_stack.c:94 [inline] dump_stack_lvl+0x241/0x360 lib/dump_stack.c:120 __might_resched+0x5d4/0x780 kernel/sched/core.c:8758 __mutex_lock_common kernel/locking/mutex.c:562 [inline] __mutex_lock+0x131/0xee0 kernel/locking/mutex.c:735 crypto_put_default_null_skcipher+0x18/0x70 crypto/crypto_null.c:179 aead_release+0x3d/0x50 crypto/algif_aead.c:489 alg_do_release crypto/af_alg.c:118 [inline] alg_sock_destruct+0x86/0xc0 crypto/af_alg.c:502 __sk_destruct+0x58/0x5f0 net/core/sock.c:2260 rcu_do_batch kernel/rcu/tree.c:2567 [inline] rcu_core+0xaaa/0x17a0 kernel/rcu/tree.c:2823 handle_softirqs+0x2d4/0x9b0 kernel/softirq.c:561 run_ksoftirqd+0xca/0x130 kernel/softirq.c:950 smpboot_thread_fn+0x544/0xa30 kernel/smpboot.c:164 kthread+0x2f0/0x390 kernel/kthread.c:389 ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147 ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:244 </TASK> Fixes: 8c7138b33e5c ("net: Unpublish sk from sk_reuseport_cb before call_rcu") Reported-by: syzbot+b3e02953598f447d4d2a@syzkaller.appspotmail.com Closes: https://lore.kernel.org/netdev/6772f2f4.050a0220.2f3838.04cb.GAE@google.com/T/#u Signed-off-by: Eric Dumazet <edumazet@google.com> Cc: Martin KaFai Lau <kafai@fb.com> --- include/net/sock.h | 7 +++++++ net/core/sock.c | 6 +++++- 2 files changed, 12 insertions(+), 1 deletion(-)