[PATCH net v2 0/8] net: fixes for requests completing on a socket that no longer listens

Hyunwoo Kim posted 8 patches 1 month ago
include/net/tcp.h        |  2 ++
net/core/sock.c          |  3 +-
net/ipv4/tcp_input.c     | 64 +++++++++++++++++++++++-----------------
net/ipv4/tcp_ipv4.c      | 52 ++++++++++++++++++++++++++++++--
net/ipv4/tcp_minisocks.c |  9 +++++-
net/ipv6/af_inet6.c      |  4 +++
net/ipv6/ipv6_sockglue.c | 15 ++++++++++
net/ipv6/tcp_ipv6.c      | 44 +++++++++++++++++++++++++--
8 files changed, 160 insertions(+), 33 deletions(-)
[PATCH net v2 0/8] net: fixes for requests completing on a socket that no longer listens
Posted by Hyunwoo Kim 1 month ago
connect(AF_UNSPEC) and listen() move a socket back and forth between
listener and active session. IPV6_ADDRFORM on top of that turns an
AF_INET6 socket into an AF_INET one.

Two things follow. One is that what the socket had before the change is
left behind: requests still in the ehash, and parent fields a child
inherits. The other is that the socket is used while it is changing.
tcp_check_req() does not hold the listener lock, and tcp_v{4,6}_rcv()
reads sk_state twice without it on the listener path.

Patch 2 is neither. After a reuseport migration the listener that counted
a request and the listener the count is decremented on are not the same.
It goes with patch 3 because patch 3 needs it. Nothing ever resets that
count, so patch 3 on its own has a check that can be bypassed.

Patches 2, 6, 7 and 8 are new in v2.

Hyunwoo Kim (8):
  tcp: fix use-after-free of the listener's ipv6_pinfo after
    IPV6_ADDRFORM
  tcp: fix imbalanced icsk_accept_queue count in tcp_check_req()
  ipv6: fix request socket use-after-free after IPV6_ADDRFORM
  net: fix out-of-bounds write in sk_clone() racing with IPV6_ADDRFORM
  tcp: do not inherit out_of_order_queue from parent
  tcp: fix use-after-free in the lockless listener path
  net: clear sk_tsq_flags in sk_clone()
  tcp: do not inherit retransmit state from parent

 include/net/tcp.h        |  2 ++
 net/core/sock.c          |  3 +-
 net/ipv4/tcp_input.c     | 64 +++++++++++++++++++++++-----------------
 net/ipv4/tcp_ipv4.c      | 52 ++++++++++++++++++++++++++++++--
 net/ipv4/tcp_minisocks.c |  9 +++++-
 net/ipv6/af_inet6.c      |  4 +++
 net/ipv6/ipv6_sockglue.c | 15 ++++++++++
 net/ipv6/tcp_ipv6.c      | 44 +++++++++++++++++++++++++--
 8 files changed, 160 insertions(+), 33 deletions(-)

-- 
2.43.0
Re: [PATCH net v2 0/8] net: fixes for requests completing on a socket that no longer listens
Posted by David Laight 1 month ago
On Mon, 24 Aug 2026 12:32:44 +0900
Hyunwoo Kim <imv4bel@gmail.com> wrote:

> connect(AF_UNSPEC) and listen() move a socket back and forth between
> listener and active session. IPV6_ADDRFORM on top of that turns an
> AF_INET6 socket into an AF_INET one.

Is it even valid to call connect() after listen()?

David

> 
> Two things follow. One is that what the socket had before the change is
> left behind: requests still in the ehash, and parent fields a child
> inherits. The other is that the socket is used while it is changing.
> tcp_check_req() does not hold the listener lock, and tcp_v{4,6}_rcv()
> reads sk_state twice without it on the listener path.
> 
> Patch 2 is neither. After a reuseport migration the listener that counted
> a request and the listener the count is decremented on are not the same.
> It goes with patch 3 because patch 3 needs it. Nothing ever resets that
> count, so patch 3 on its own has a check that can be bypassed.
> 
> Patches 2, 6, 7 and 8 are new in v2.
> 
> Hyunwoo Kim (8):
>   tcp: fix use-after-free of the listener's ipv6_pinfo after
>     IPV6_ADDRFORM
>   tcp: fix imbalanced icsk_accept_queue count in tcp_check_req()
>   ipv6: fix request socket use-after-free after IPV6_ADDRFORM
>   net: fix out-of-bounds write in sk_clone() racing with IPV6_ADDRFORM
>   tcp: do not inherit out_of_order_queue from parent
>   tcp: fix use-after-free in the lockless listener path
>   net: clear sk_tsq_flags in sk_clone()
>   tcp: do not inherit retransmit state from parent
> 
>  include/net/tcp.h        |  2 ++
>  net/core/sock.c          |  3 +-
>  net/ipv4/tcp_input.c     | 64 +++++++++++++++++++++++-----------------
>  net/ipv4/tcp_ipv4.c      | 52 ++++++++++++++++++++++++++++++--
>  net/ipv4/tcp_minisocks.c |  9 +++++-
>  net/ipv6/af_inet6.c      |  4 +++
>  net/ipv6/ipv6_sockglue.c | 15 ++++++++++
>  net/ipv6/tcp_ipv6.c      | 44 +++++++++++++++++++++++++--
>  8 files changed, 160 insertions(+), 33 deletions(-)
>
Re: [PATCH net v2 0/8] net: fixes for requests completing on a socket that no longer listens
Posted by Hyunwoo Kim 1 month ago
On Mon, Aug 24, 2026 at 09:30:44AM +0100, David Laight wrote:
> On Mon, 24 Aug 2026 12:32:44 +0900
> Hyunwoo Kim <imv4bel@gmail.com> wrote:
> 
> > connect(AF_UNSPEC) and listen() move a socket back and forth between
> > listener and active session. IPV6_ADDRFORM on top of that turns an
> > AF_INET6 socket into an AF_INET one.
> 
> Is it even valid to call connect() after listen()?

More precisely, it is connect(AF_UNSPEC), which is a disconnect. This 
is a valid operation.


Best regards,
Hyunwoo Kim
Re: [PATCH net v2 0/8] net: fixes for requests completing on a socket that no longer listens
Posted by Jakub Kicinski 3 weeks, 6 days ago
On Mon, 24 Aug 2026 12:32:44 +0900 Hyunwoo Kim wrote:
> connect(AF_UNSPEC) and listen() move a socket back and forth between
> listener and active session. IPV6_ADDRFORM on top of that turns an
> AF_INET6 socket into an AF_INET one.
> 
> Two things follow. One is that what the socket had before the change is
> left behind: requests still in the ehash, and parent fields a child
> inherits. The other is that the socket is used while it is changing.
> tcp_check_req() does not hold the listener lock, and tcp_v{4,6}_rcv()
> reads sk_state twice without it on the listener path.
> 
> Patch 2 is neither. After a reuseport migration the listener that counted
> a request and the listener the count is decremented on are not the same.
> It goes with patch 3 because patch 3 needs it. Nothing ever resets that
> count, so patch 3 on its own has a check that can be bypassed.

No reviews? Are people getting tired of the disconnect bugs?

Hyunwoo Kim, can any of the fixes be simplified if we move more
cleanup / responsibility to the disconnect path? Patch 6 perhaps?
Re: [PATCH net v2 0/8] net: fixes for requests completing on a socket that no longer listens
Posted by Hyunwoo Kim 3 weeks, 6 days ago
On Mon, Aug 31, 2026 at 04:35:56PM -0700, Jakub Kicinski wrote:
> On Mon, 24 Aug 2026 12:32:44 +0900 Hyunwoo Kim wrote:
> > connect(AF_UNSPEC) and listen() move a socket back and forth between
> > listener and active session. IPV6_ADDRFORM on top of that turns an
> > AF_INET6 socket into an AF_INET one.
> > 
> > Two things follow. One is that what the socket had before the change is
> > left behind: requests still in the ehash, and parent fields a child
> > inherits. The other is that the socket is used while it is changing.
> > tcp_check_req() does not hold the listener lock, and tcp_v{4,6}_rcv()
> > reads sk_state twice without it on the listener path.
> > 
> > Patch 2 is neither. After a reuseport migration the listener that counted
> > a request and the listener the count is decremented on are not the same.
> > It goes with patch 3 because patch 3 needs it. Nothing ever resets that
> > count, so patch 3 on its own has a check that can be bypassed.
> 
> No reviews? Are people getting tired of the disconnect bugs?
> 
> Hyunwoo Kim, can any of the fixes be simplified if we move more
> cleanup / responsibility to the disconnect path? Patch 6 perhaps?

Fixing it like this [1] lets the disconnect path handle it, and also
closes the trigger path for patches 4, 5 and 8.

What do you think?

[1]: https://lore.kernel.org/all/apaAskNL-IvdcR7h@v4bel/


Best regards,
Hyuwnoo Kim