Re: [PATCH net-next v2] net: add SO_BYPASS_RFS to exclude a socket from RFS and aRFS

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] net: race in sk_setsockopt() deleting RFS flow before setting
  sk_bypass_rfs
It also noticed 1 related pre-existing issue this patch does not
introduce:
- [Medium] net: file descriptor leak in sk_getsockopt() [1]
--

commit 0bb55ec9420355430d450dec603d8adf895ff281
Author: Vitaly Zuevsky <vitaly.zuevsky@gcore.com>

net: add SO_BYPASS_RFS to exclude a socket from RFS and aRFS

This commit adds the SO_BYPASS_RFS boolean socket option and a skc_bypass_rfs
bit in struct sock_common to exclude a socket from RFS and aRFS flow recording
and deletion.

> diff --git a/net/core/sock.c b/net/core/sock.c
> index e2401d735c5a7..00808217aebf1 100644
> --- a/net/core/sock.c
> +++ b/net/core/sock.c
[ ... ]
> @@ -1597,6 +1598,14 @@ int sk_setsockopt(struct sock *sk, int level, int optname,
>  		reuseport_update_incoming_cpu(sk, val);
>  		break;
>  
> +#ifdef CONFIG_RPS
> +	case SO_BYPASS_RFS:
> +		if (valbool)
> +			sock_rps_delete_flow(sk);
> +		sk->sk_bypass_rfs = valbool;
> +		break;
> +#endif
> +
>  	case SO_CNX_ADVICE:
>  		if (val == 1)
>  			dst_negative_advice(sk);

[Severity: Medium]
Can a concurrent receive or send re-record the flow between
sock_rps_delete_flow() and setting sk->sk_bypass_rfs?

Callers such as inet_recvmsg() invoke sock_rps_record_flow() without
holding the socket lock:

CPU0                              CPU1
sk_setsockopt(SO_BYPASS_RFS, 1)
  sock_rps_delete_flow(sk)
    _sock_rps_delete_flow(sk)
                                  inet_recvmsg()
                                    sock_rps_record_flow(sk)
                                      /* sk->sk_bypass_rfs is still 0 */
                                      _sock_rps_record_flow(sk)  <-- re-records flow
  sk->sk_bypass_rfs = 1;

Once sk->sk_bypass_rfs is set, sock_rps_delete_flow() returns early, so
the re-recorded entry stays in rps_sock_flow_table while SO_BYPASS_RFS is
enabled and is not cleared when the socket closes.  Setting
sk->sk_bypass_rfs before calling _sock_rps_delete_flow() would avoid
leaving a stale entry if a concurrent reader races with setsockopt().

> @@ -2118,6 +2127,12 @@ int sk_getsockopt(struct sock *sk, int level, int optname,
>  		v.val = READ_ONCE(sk->sk_incoming_cpu);
>  		break;
>  
> +#ifdef CONFIG_RPS
> +	case SO_BYPASS_RFS:
> +		v.val = sk->sk_bypass_rfs;
> +		break;
> +#endif
> +
>  	case SO_MEMINFO:
>  	{
>  		u32 meminfo[SK_MEMINFO_VARS];

[Severity: Medium]
This problem wasn't introduced by this patch, but can sk_getsockopt() leak
a file descriptor via sk_getsockopt_peerpidfd() [1] when optlen is
smaller than sizeof(int)?

net/core/sock.c:sk_getsockopt_peerpidfd() {
	...
	if (len > sizeof(pidfd))
		len = sizeof(pidfd);
	...
	pidfd = pidfd_prepare(peer_pid, flags, &pidfd_file);
	...
	if (copy_to_sockptr(optval, &pidfd, len) ||
	    copy_to_sockptr(optlen, &len, sizeof(int))) {
		put_unused_fd(pidfd);
		fput(pidfd_file);

		return -EFAULT;
	}

	fd_install(pidfd, pidfd_file);
	return 0;
}

len is capped at sizeof(pidfd) without checking that it is at least
sizeof(pidfd).  If a caller passes a smaller optlen, copy_to_sockptr()
copies a truncated value into optval and fd_install() installs the new
pidfd into the process's file descriptor table without the caller
receiving the full descriptor number needed to close it.

[1] https://sashiko.dev/#/bug/linux-49f253f1-047b-482f-9138-26dfe35242e3

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261109014547.81555-1-vitaly.zuevsky@gcore.com?part=1