On Mon, Feb 13, 2023 at 12:58:35PM -0800, Kuniyuki Iwashima wrote:
From: Greg KH gregkh@linuxfoundation.org Date: Mon, 13 Feb 2023 18:48:48 +0100
On Mon, Feb 13, 2023 at 08:44:55AM -0800, Kuniyuki Iwashima wrote:
From: Willy Tarreau w@1wt.eu Date: Mon, 13 Feb 2023 08:52:34 +0100
Hi Greg,
On Mon, Feb 13, 2023 at 08:25:34AM +0100, Greg KH wrote:
On Mon, Feb 13, 2023 at 05:27:03AM +0100, Willy Tarreau wrote:
Hi,
[CCed netdev]
On Sun, Feb 12, 2023 at 10:38:40PM -0500, Winter wrote: > Hi all, > > I'm facing the same issue as > https://lore.kernel.org/stable/CAFsF8vL4CGFzWMb38_XviiEgxoKX0GYup=JiUFXUOmag..., > but on 5.15. I've bisected it across releases to 5.15.88, and can reproduce > on 5.15.93. > > However, I cannot seem to find the identified problematic commit in the 5.15 > branch, so I'm unsure if this is a different issue or not. > > There's a few ways to reproduce this issue, but the one I've been using is > running libuv's (https://github.com/libuv/libuv) tests, specifically tests > 271 and 277.
>From the linked patch:
https://lore.kernel.org/stable/20221228144337.512799851@linuxfoundation.org/
But that commit only ended up in 6.0.y, not 5.15, so how is this an issue in 5.15.y?
Hmmm I plead -ENOCOFFEE on my side, I hadn't notice the "can't find the problematic commit", you're right indeed.
However if the issue happened in 5.15.88, the only part touching the network listening area is this one which may introduce an EINVAL on one listening path, but that seems unrelated to me given that it's only for ULP that libuv doesn't seem to be using:
dadd0dcaa67d ("net/ulp: prevent ULP without clone op from entering the LISTEN status")
This commit accidentally backports a part of 7a7160edf1bf ("net: Return errno in sk->sk_prot->get_port().") and removed err = -EADDRINUSE in inet_csk_listen_start(). Then, listen() will return 0 even if ->get_port() actually fails and returns 1.
I can send a small revert or a whole backport, but which is preferable ? The original patch is not for stable, but it will make future backports easy.
A whole revert is probably best, if it's not needed. But if it is, a fix up would be fine to get as well.
dadd0dcaa67d is needed to fix potential double-free, so could you queue this fixup for 5.15. ?
---8<---
From ad319ace8b5c1dd5105b7263b7ccfd0ba0926551 Mon Sep 17 00:00:00 2001
From: Kuniyuki Iwashima kuniyu@amazon.com Date: Mon, 13 Feb 2023 20:45:48 +0000 Subject: [PATCH] tcp: Fix listen() regression in 5.15.88.
When we backport dadd0dcaa67d ("net/ulp: prevent ULP without clone op from entering the LISTEN status"), we have accidentally backported a part of 7a7160edf1bf ("net: Return errno in sk->sk_prot->get_port().") and removed err = -EADDRINUSE in inet_csk_listen_start().
Thus, listen() no longer returns -EADDRINUSE even if ->get_port() failed as reported in [0].
We set -EADDRINUSE to err just before ->get_port() to fix the regression.
Reported-by: Winter winter@winter.cafe Signed-off-by: Kuniyuki Iwashima kuniyu@amazon.com
net/ipv4/inet_connection_sock.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c index a86140ff093c..29ec42c1f5d0 100644 --- a/net/ipv4/inet_connection_sock.c +++ b/net/ipv4/inet_connection_sock.c @@ -1070,6 +1070,7 @@ int inet_csk_listen_start(struct sock *sk, int backlog) * It is OK, because this socket enters to hash table only * after validation is complete. */
- err = -EADDRINUSE; inet_sk_state_store(sk, TCP_LISTEN); if (!sk->sk_prot->get_port(sk, inet->inet_num)) { inet->inet_sport = htons(inet->inet_num);
-- 2.38.1 ---8<---
Thanks, now queued up.
greg k-h