[PATCH net] net: restrict SO_REUSEPORT to TCP, UDP and SCTP sockets

[PATCH net] net: restrict SO_REUSEPORT to TCP, UDP and SCTP sockets

[Eric Dumazet]

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(-)

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);
-- 
2.47.1.613.gc27f4b7a9f-goog

Re: [PATCH net] net: restrict SO_REUSEPORT to TCP, UDP and SCTP sockets

[Jakub Kicinski]

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..
-- 
pw-bot: defer

Re: [PATCH net] net: restrict SO_REUSEPORT to TCP, UDP and SCTP sockets

[Eric Dumazet]

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)

Re: [PATCH net] net: restrict SO_REUSEPORT to TCP, UDP and SCTP sockets

[David Laight]

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);

Re: [PATCH net] net: restrict SO_REUSEPORT to TCP, UDP and SCTP sockets

[Martin KaFai Lau]

On 12/31/24 4:44 AM, Eric Dumazet wrote:
> 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)) {

I think this should be mostly aligned with what is supported in the bpf 
reuseport_array and sock_map. This can be changed if other sock supported is 
added to the bpf map.

> err = -ENOTSUPP;
> goto err_prog_put;
> }