Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.
Critical: 0 · High: 1 · Medium: 0 · Low: 0
If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).
- [High] Incomplete fix.
--
udp: fix auto-selected port colliding with an existing SO_REUSEPORT socket
This commit changes udp_lib_lport_inuse() so that, during a port scan
(bitmap != NULL), compatible SO_REUSEPORT sockets are marked in the bitmap
instead of being skipped. The reuseport exemption now applies only when a
specific port is requested. This matches TCP commit 0643ee4fd1b7.
> diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
> index b090bd1f59e86..a7ade3171becc 100644
> --- a/net/ipv4/udp.c
> +++ b/net/ipv4/udp.c
> @@ -147,11 +147,10 @@ static int udp_lib_lport_inuse(struct net *net, __u16
> num,
> (!sk2->sk_bound_dev_if || !sk->sk_bound_dev_if ||
> sk2->sk_bound_dev_if == sk->sk_bound_dev_if) &&
> inet_rcv_saddr_equal(sk, sk2, true)) {
> - if (sk2->sk_reuseport && sk->sk_reuseport &&
> + if (!bitmap && sk2->sk_reuseport && sk->sk_reuseport &&
[Severity: High]
Doesn't this leave the same collision in place when both sockets also have
SO_REUSEADDR set?
The new !bitmap check sits inside the outer filter in
udp_lib_lport_inuse(), and that filter is also evaluated in scan mode:
net/ipv4/udp.c:udp_lib_lport_inuse() {
...
(bitmap || udp_sk(sk2)->udp_port_hash == num) &&
(!sk2->sk_reuse || !sk->sk_reuse) &&
...
}
If sk and sk2 both have sk_reuse set, the outer condition is false. In that
case neither the new reuseport branch nor the __set_bit() runs, so sk2's
port is never marked in the bitmap.
udp_lib_get_port() then treats the port as free:
if (low <= snum && snum <= high &&
!test_bit(snum >> udptable->log, bitmap) &&
!inet_is_local_reserved_port(net, snum))
goto found;
At found:, since sk->sk_reuseport is set, udp_reuseport_add_sock() finds
sk2 (same port hash, same bound dev, sk2->sk_reuseport, same uid) and does:
return reuseport_add_sock(sk, sk2,
inet_rcv_saddr_any(sk));
The unrelated socket then joins the existing reuseport group and gets part
of its datagrams. That is the symptom described in the commit message.
IPv6 goes through the same path via udp_v6_get_port()->udp_lib_get_port().
Implicit autobind from connect()/sendmsg() on an unbound socket calls
udp_lib_get_port(sk, 0, ...) and reaches it as well.
The commit message says:
Note that this still collides when both sockets also have SO_REUSEADDR
set: udp_lib_lport_inuse() skips such a pair before it reaches the
reuseport check, so the port is never marked either. That is not a
common setup and is left as is here.
Is that really uncommon? Server code and libraries often set SO_REUSEADDR
together with SO_REUSEPORT.
The message also says UDP now does the same as TCP. TCP's scan calls
inet_bind_conflict() with relax == false, and that treats a reuse pair as a
conflict:
net/ipv4/inet_connection_sock.c:inet_bind_conflict() {
...
if (sk->sk_reuse && sk2->sk_reuse &&
sk2->sk_state != TCP_LISTEN) {
if (!relax || (!reuseport_ok && sk->sk_reuseport &&
...
return true;
...
}
So TCP never hands out an occupied port during a scan because of
SO_REUSEADDR, but the UDP scan still does.
Could the outer term be changed to something like:
(bitmap || !sk2->sk_reuse || !sk->sk_reuse) &&
so that every compatible occupied port is marked during the scan? The
SO_REUSEADDR and SO_REUSEPORT exemptions would still apply on the
explicit-port path.
Separately, sockets that have only SO_REUSEADDR set can also be given the
same auto-selected port today. That behaviour predates this patch, but the
same change would fix it too.
> !rcu_access_pointer(sk->sk_reuseport_cb) &&
> uid_eq(uid, sk_uid(sk2))) {
> - if (!bitmap)
> - return 0;
> + return 0;
> } else {
> if (!bitmap)
> return 1;
--
Sashiko AI review ·
https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260923060547.118748-1-jiayuan.chen%40linux.dev
