Fix several issues in the mptcp connect test's main_loop function.
- Fix a bug where the wrong file descriptor was being checked for errors - Fix the input file descriptor lifecycle in the reconnection loop to prevent use of invalid fd - Add proper resource cleanup in error paths
Cong Liu (3): selftests: mptcp: Fix incorrect file descriptor check in main_loop selftests: mptcp: Fix input fd lifecycle in reconnection loop selftests: mptcp: Clean up resources properly in main_loop
.../selftests/net/mptcp/mptcp_connect.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-)
base-commit: 2b88851f583d3c4e40bcd40cfe1965241ec229dd
Fix a bug where the code was checking the wrong file descriptor when opening the input file. The code was checking 'fd' instead of 'fd_in', which could lead to incorrect error handling.
Signed-off-by: Cong Liu liucong2@kylinos.cn --- tools/testing/selftests/net/mptcp/mptcp_connect.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.c b/tools/testing/selftests/net/mptcp/mptcp_connect.c index 4209b9569039..31f4c5618569 100644 --- a/tools/testing/selftests/net/mptcp/mptcp_connect.c +++ b/tools/testing/selftests/net/mptcp/mptcp_connect.c @@ -1249,7 +1249,7 @@ int main_loop(void)
if (cfg_input && cfg_sockopt_types.mptfo) { fd_in = open(cfg_input, O_RDONLY); - if (fd < 0) + if (fd_in < 0) xerror("can't open %s:%d", cfg_input, errno); }
@@ -1272,7 +1272,7 @@ int main_loop(void)
if (cfg_input && !cfg_sockopt_types.mptfo) { fd_in = open(cfg_input, O_RDONLY); - if (fd < 0) + if (fd_in < 0) xerror("can't open %s:%d", cfg_input, errno); }
When both cfg_input and cfg_sockopt_types.mptfo are set, the input file descriptor (fd_in) is opened before the reconnection loop but closed within the loop. However, when mptfo is enabled, the descriptor is not reopened in the loop, causing subsequent iterations to use an invalid file descriptor.
Move the file open operation into the loop to ensure fd_in is always valid when needed, regardless of mptfo setting.
Signed-off-by: Cong Liu liucong2@kylinos.cn --- tools/testing/selftests/net/mptcp/mptcp_connect.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.c b/tools/testing/selftests/net/mptcp/mptcp_connect.c index 31f4c5618569..4d4ea4627daa 100644 --- a/tools/testing/selftests/net/mptcp/mptcp_connect.c +++ b/tools/testing/selftests/net/mptcp/mptcp_connect.c @@ -1247,12 +1247,6 @@ int main_loop(void) struct addrinfo *peer; struct wstate winfo;
- if (cfg_input && cfg_sockopt_types.mptfo) { - fd_in = open(cfg_input, O_RDONLY); - if (fd_in < 0) - xerror("can't open %s:%d", cfg_input, errno); - } - memset(&winfo, 0, sizeof(winfo)); fd = sock_connect_mptcp(cfg_host, cfg_port, cfg_sock_proto, &peer, fd_in, &winfo); if (fd < 0) @@ -1270,7 +1264,7 @@ int main_loop(void) if (cfg_cmsg_types.cmsg_enabled) apply_cmsg_types(fd, &cfg_cmsg_types);
- if (cfg_input && !cfg_sockopt_types.mptfo) { + if (cfg_input) { fd_in = open(cfg_input, O_RDONLY); if (fd_in < 0) xerror("can't open %s:%d", cfg_input, errno);
Add proper cleanup of resources (file descriptors and address info) in error paths to prevent resource leaks.
Signed-off-by: Cong Liu liucong2@kylinos.cn --- tools/testing/selftests/net/mptcp/mptcp_connect.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.c b/tools/testing/selftests/net/mptcp/mptcp_connect.c index 4d4ea4627daa..e82fde0411b2 100644 --- a/tools/testing/selftests/net/mptcp/mptcp_connect.c +++ b/tools/testing/selftests/net/mptcp/mptcp_connect.c @@ -1271,8 +1271,13 @@ int main_loop(void) }
ret = copyfd_io(fd_in, fd, 1, 0, &winfo); - if (ret) + if (ret) { + close(fd); + if (cfg_input) + close(fd_in); + freeaddrinfo(peer); return ret; + }
if (cfg_truncate > 0) { xdisconnect(fd, peer->ai_addrlen); @@ -1291,6 +1296,7 @@ int main_loop(void) goto again; } else { close(fd); + freeaddrinfo(peer); }
return 0;
Hi Cong Liu,
On 13/01/2025 09:52, Cong Liu wrote:
Fix several issues in the mptcp connect test's main_loop function.
- Fix a bug where the wrong file descriptor was being checked for errors
- Fix the input file descriptor lifecycle in the reconnection loop to prevent use of invalid fd
- Add proper resource cleanup in error paths
Thank you for these fixes!
Please note that when sending patches to the Netdev mailing list, it is asked to follow some specific rules, e.g.
- designate your patch to a tree - [PATCH net] or [PATCH net-next] - for fixes the Fixes: tag is required, regardless of the tree
https://docs.kernel.org/process/maintainer-netdev.html
If I'm not mistaken, it looks like you have here fixes for net, and the Fixes tag is missing.
Do you mind sending a v2 with this being fixed please? Because these fixes are not urgent, do you mind sending them only to the MPTCP ML for the moment, and not to the other ones (and no other people in CC).
Cong Liu (3): selftests: mptcp: Fix incorrect file descriptor check in main_loop selftests: mptcp: Fix input fd lifecycle in reconnection loop selftests: mptcp: Clean up resources properly in main_loop
.../selftests/net/mptcp/mptcp_connect.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-)
base-commit: 2b88851f583d3c4e40bcd40cfe1965241ec229dd
Note that this base-commit doesn't seem to exist. Because of that, our MPTCP CI was not able to automatically apply this commit.
Talking about our CI, it looks like that 'mptcp_connect' now crashes in some cases with:
free(): double free detected in tcache 2
Do you mind checking this please?
https://github.com/multipath-tcp/mptcp_net-next/actions/runs/12751035845
pw-bot: cr
Cheers, Matt
linux-kselftest-mirror@lists.linaro.org
-
Cong Liu -
Matthieu Baerts