[PATCH mptcp-net] selftests: mptcp: fix an UAF in mptcp_connect.c

[PATCH mptcp-net] selftests: mptcp: fix an UAF in mptcp_connect.c

[Gang Yan]

From: Gang Yan <yangang@kylinos.cn>

At the end of 'sock_connect_mptcp()', it calls 'freeaddrinfo(addr)',
the 'peer' pointer (which points into 'addr') remains. Later, the main
loop uses this peer pointer for reconnection attempts. If the memory has
been freed and reused, the address data could be overwritten, resulting
in an invalid remote address.

This patch removes the '**peer' out-parameter entirely and adds a
sock_reconnect() helper that resolves the address and connects in a
self-contained scope, so no pointer to freed memory escapes.
Also prints the reconnect destination address on stderr as suggested by
Paolo.

Assisted-by: Codex:GLM5.2
Fixes: 05be5e273c84 ("selftests: mptcp: add disconnect tests")
Signed-off-by: Gang Yan <yangang@kylinos.cn>
---
 .../selftests/net/mptcp/mptcp_connect.c       | 25 ++++++++++++++-----
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.c b/tools/testing/selftests/net/mptcp/mptcp_connect.c
index ea4cb6c1bd5e..9c083038c400 100644
--- a/tools/testing/selftests/net/mptcp/mptcp_connect.c
+++ b/tools/testing/selftests/net/mptcp/mptcp_connect.c
@@ -368,7 +368,6 @@ static int sock_listen_mptcp(const char * const listenaddr,
 
 static int sock_connect_mptcp(const char * const remoteaddr,
 			      const char * const port, int proto,
-			      struct addrinfo **peer,
 			      int infd, struct wstate *winfo)
 {
 	struct addrinfo hints = {
@@ -404,13 +403,11 @@ static int sock_connect_mptcp(const char * const remoteaddr,
 			if (syn_copied >= 0) {
 				winfo->off = syn_copied;
 				winfo->len -= syn_copied;
-				*peer = a;
 				break; /* success */
 			}
 			perror("sendto()");
 		} else {
 			if (connect(sock, a->ai_addr, a->ai_addrlen) == 0) {
-				*peer = a;
 				break; /* success */
 			}
 			perror("connect()");
@@ -427,6 +424,22 @@ static int sock_connect_mptcp(const char * const remoteaddr,
 	return sock;
 }
 
+static int sock_reconnect(const char *host, const char *port, int fd)
+{
+	struct addrinfo hints = {
+		.ai_socktype = SOCK_STREAM,
+		.ai_family = pf,
+	};
+	struct addrinfo *addr;
+	int ret;
+
+	xgetaddrinfo(host, port, &hints, &addr);
+	ret = connect(fd, addr->ai_addr, addr->ai_addrlen);
+	freeaddrinfo(addr);
+
+	return ret;
+}
+
 static size_t do_rnd_write(const int fd, char *buf, const size_t len)
 {
 	static bool first = true;
@@ -1367,7 +1380,6 @@ void xdisconnect(int fd)
 
 int main_loop(void)
 {
-	struct addrinfo *peer = NULL;
 	int fd = 0, ret, fd_in = 0;
 	struct wstate winfo;
 
@@ -1378,7 +1390,7 @@ int main_loop(void)
 	}
 
 	memset(&winfo, 0, sizeof(winfo));
-	fd = sock_connect_mptcp(cfg_host, cfg_port, cfg_sock_proto, &peer, fd_in, &winfo);
+	fd = sock_connect_mptcp(cfg_host, cfg_port, cfg_sock_proto, fd_in, &winfo);
 	if (fd < 0)
 		return 2;
 
@@ -1413,7 +1425,8 @@ int main_loop(void)
 		 * connect to be blocking
 		 */
 		set_nonblock(fd, false);
-		if (connect(fd, peer->ai_addr, peer->ai_addrlen))
+		fprintf(stderr, "reconnecting to %s:%s\n", cfg_host, cfg_port);
+		if (sock_reconnect(cfg_host, cfg_port, fd))
 			xerror("can't reconnect: %d", errno);
 		if (cfg_input)
 			close(fd_in);
-- 
2.43.0

Re: [PATCH mptcp-net] selftests: mptcp: fix an UAF in mptcp_connect.c

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] sock_reconnect() unconditionally connects to the first resolved address instead of iterating, introducing a logic regression for multi-IP hosts.
--

--- Patch [1]: [PATCH mptcp-net] selftests: mptcp: fix an UAF in mptcp_connect.c ---
commit 0dae1d93a2aa0c4ea91bddbe5e055fd748d90080
Author: Gang Yan <yangang@kylinos.cn>

selftests: mptcp: fix an UAF in mptcp_connect.c

This patch removes the peer out-parameter from sock_connect_mptcp() to prevent a
use-after-free issue when the original address memory is freed. It introduces a
sock_reconnect() helper to resolve the address and connect independently
during reconnection attempts.

> diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.c b/tools/testing/selftests/net/mptcp/mptcp_connect.c
> index cbe573c4ab3ad..05ab9887485c0 100644
> --- a/tools/testing/selftests/net/mptcp/mptcp_connect.c
> +++ b/tools/testing/selftests/net/mptcp/mptcp_connect.c

[ ... ]

> @@ -427,6 +424,22 @@ static int sock_connect_mptcp(const char * const remoteaddr,
>  	return sock;
>  }
>  
> +static int sock_reconnect(const char *host, const char *port, int fd)
> +{
> +	struct addrinfo hints = {
> +		.ai_socktype = SOCK_STREAM,
> +		.ai_family = pf,
> +	};
> +	struct addrinfo *addr;
> +	int ret;
> +
> +	xgetaddrinfo(host, port, &hints, &addr);
> +	ret = connect(fd, addr->ai_addr, addr->ai_addrlen);

[Severity: Medium]
Will this unconditionally connect to the first address returned by
xgetaddrinfo() without iterating or attempting fallbacks?

The original connection logic in sock_connect_mptcp() implicitly saved the
successful connection address by keeping a pointer to the successful node
in the addrinfo list. By resolving the host again and blindly connecting to
the first returned address without looping, could this cause the selftest
to spuriously fail if the host resolves to multiple IPs (like dual-stack
IPv4/IPv6 on localhost) and the first address is not the one previously used?

> +	freeaddrinfo(addr);
> +
> +	return ret;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260814050625.80231-1-gang.yan@linux.dev?part=1

Re: [PATCH mptcp-net] selftests: mptcp: fix an UAF in mptcp_connect.c

[MPTCP CI]

Hi Gang,

Thank you for your modifications, that's great!

Our CI did some validations and here is its report:

- KVM Validation: normal (except selftest_mptcp_join): Success! ✅
- KVM Validation: normal (only selftest_mptcp_join): Success! ✅
- KVM Validation: debug (except selftest_mptcp_join): Success! ✅
- KVM Validation: debug (only selftest_mptcp_join): Success! ✅
- KVM Validation: btf-normal (only bpftest_all): Success! ✅
- KVM Validation: btf-debug (only bpftest_all): Success! ✅
- Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/31773341323

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/3c5bc3507f5e
Patchwork: https://patchwork.kernel.org/project/mptcp/list/?series=1145826


If there are some issues, you can reproduce them using the same environment as
the one used by the CI thanks to a docker image, e.g.:

    $ cd [kernel source code]
    $ docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \
        --pull always mptcp/mptcp-upstream-virtme-docker:latest \
        auto-normal

For more details:

    https://github.com/multipath-tcp/mptcp-upstream-virtme-docker


Please note that despite all the efforts that have been already done to have a
stable tests suite when executed on a public CI like here, it is possible some
reported issues are not due to your modifications. Still, do not hesitate to
help us improve that ;-)

Cheers,
MPTCP GH Action bot
Bot operated by Matthieu Baerts (NGI0 Core)

Re: [PATCH mptcp-net] selftests: mptcp: fix an UAF in mptcp_connect.c

[gang.yan]

August 14, 2026 at 1:06 PM, "Gang Yan" <gang.yan@linux.dev mailto:gang.yan@linux.dev?to=%22Gang%20Yan%22%20%3Cgang.yan%40linux.dev%3E > wrote:


> 
> From: Gang Yan <yangang@kylinos.cn>
> 
> At the end of 'sock_connect_mptcp()', it calls 'freeaddrinfo(addr)',
> the 'peer' pointer (which points into 'addr') remains. Later, the main
> loop uses this peer pointer for reconnection attempts. If the memory has
> been freed and reused, the address data could be overwritten, resulting
> in an invalid remote address.
> 
> This patch removes the '**peer' out-parameter entirely and adds a
> sock_reconnect() helper that resolves the address and connects in a
> self-contained scope, so no pointer to freed memory escapes.
> Also prints the reconnect destination address on stderr as suggested by
> Paolo.
> 

Hi Matt,

Sorry for this, after reviewing sashiko's comment, I think this modification
below should be better:

diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.c b/tools/testing/selftests/net/mptcp/mptcp_connect.c
index ea4cb6c1bd5e..c81ec4400bef 100644
--- a/tools/testing/selftests/net/mptcp/mptcp_connect.c
+++ b/tools/testing/selftests/net/mptcp/mptcp_connect.c
@@ -421,7 +421,8 @@ static int sock_connect_mptcp(const char * const remoteaddr,
                sock = -1;
        }
 
-       freeaddrinfo(addr);
+       if (sock == -1)
+               freeaddrinfo(addr);
        if (sock != -1)
                SOCK_TEST_TCPULP(sock, proto);
        return sock;
@@ -1424,6 +1425,7 @@ int main_loop(void)
        }
 
 out:
+       freeaddrinfo(peer);
        if (cfg_input)
                close(fd_in);

If you think it's OK, I can send v2 immediately.

Thanks
Gang

Re: [PATCH mptcp-net] selftests: mptcp: fix an UAF in mptcp_connect.c

[Matthieu Baerts]

14 Aug 2026 08:42:38 gang.yan@linux.dev:

> August 14, 2026 at 1:06 PM, "Gang Yan" <gang.yan@linux.dev mailto:gang.yan@linux.dev?to=%22Gang%20Yan%22%20%3Cgang.yan%40linux.dev%3E > wrote:
>
>
>>
>> From: Gang Yan <yangang@kylinos.cn>
>>
>> At the end of 'sock_connect_mptcp()', it calls 'freeaddrinfo(addr)',
>> the 'peer' pointer (which points into 'addr') remains. Later, the main
>> loop uses this peer pointer for reconnection attempts. If the memory has
>> been freed and reused, the address data could be overwritten, resulting
>> in an invalid remote address.
>>
>> This patch removes the '**peer' out-parameter entirely and adds a
>> sock_reconnect() helper that resolves the address and connects in a
>> self-contained scope, so no pointer to freed memory escapes.
>> Also prints the reconnect destination address on stderr as suggested by
>> Paolo.
>>
>
> Hi Matt,
>
> Sorry for this, after reviewing sashiko's comment, I think this modification
> below should be better:
>
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.c b/tools/testing/selftests/net/mptcp/mptcp_connect.c
> index ea4cb6c1bd5e..c81ec4400bef 100644
> --- a/tools/testing/selftests/net/mptcp/mptcp_connect.c
> +++ b/tools/testing/selftests/net/mptcp/mptcp_connect.c
> @@ -421,7 +421,8 @@ static int sock_connect_mptcp(const char * const remoteaddr,
>                 sock = -1;
>         }
>
> -       freeaddrinfo(addr);
> +       if (sock == -1)
> +               freeaddrinfo(addr);
>         if (sock != -1)
>                 SOCK_TEST_TCPULP(sock, proto);
>         return sock;
> @@ -1424,6 +1425,7 @@ int main_loop(void)
>         }
>
> out:
> +       freeaddrinfo(peer);

This assumes peer == addr. It is certainly the case but it looks wrong,
and I guess sashiko will complain like it did here.

We could add something on the commit message to say that it is always an IP address that is given, but maybe we should also just
handle that correctly: either peer here is not a pointer, and the content
is copied, or you pass &addr to sock_connect_mptcp, and you do the free here.

WDYT?

Cheers,
Matt

Re: [PATCH mptcp-net] selftests: mptcp: fix an UAF in mptcp_connect.c

[gang.yan]

August 14, 2026 at 4:54 PM, "Matthieu Baerts" <matttbe@kernel.org mailto:matttbe@kernel.org?to=%22Matthieu%20Baerts%22%20%3Cmatttbe%40kernel.org%3E > wrote:



> > 
> This assumes peer == addr. It is certainly the case but it looks wrong,
> and I guess sashiko will complain like it did here.
> 
> We could add something on the commit message to say that it is always an IP address that is given, but maybe we should also just
> handle that correctly: either peer here is not a pointer, and the content
> is copied, 

I prefer this, because it is smaller, like:

'''
 static int sock_connect_mptcp(const char * const remoteaddr,
 			      const char * const port, int proto,
-			      struct addrinfo **peer,
+			      struct sockaddr_storage *peer, socklen_t *peer_len,
 			      int infd, struct wstate *winfo)
 {
 ...
-				*peer = a;
 				break;
...
-				*peer = a;
 				break;
...
+	if (sock != -1) {
+		memcpy(peer, a->ai_addr, a->ai_addrlen);
+		*peer_len = a->ai_addrlen;
+	}
 	freeaddrinfo(addr);
...

-	struct addrinfo *peer = NULL;
+	struct sockaddr_storage peer;
+	socklen_t peer_len;
...
-	fd = sock_connect_mptcp(..., &peer, fd_in, &winfo);
+	fd = sock_connect_mptcp(..., &peer, &peer_len, fd_in, &winfo);
...
-	if (connect(fd, peer->ai_addr, peer->ai_addrlen))
+	if (connect(fd, (struct sockaddr *)&peer, peer_len))
'''

> or you pass &addr to sock_connect_mptcp, and you do the free here.

This needs a refactor of sock_connect_mptcp – I used to do it that way. But I
worry that backporting could be tricky. If you're fine with it, I'm okay with this plan
as well.

WDYT?

Thanks
Gang

Re: [PATCH mptcp-net] selftests: mptcp: fix an UAF in mptcp_connect.c

[Matthieu Baerts]

14 Aug 2026 11:20:47 gang.yan@linux.dev:

> August 14, 2026 at 4:54 PM, "Matthieu Baerts" <matttbe@kernel.org mailto:matttbe@kernel.org?to=%22Matthieu%20Baerts%22%20%3Cmatttbe%40kernel.org%3E > wrote:
>
>
>
>>>
>> This assumes peer == addr. It is certainly the case but it looks wrong,
>> and I guess sashiko will complain like it did here.
>>
>> We could add something on the commit message to say that it is always an IP address that is given, but maybe we should also just
>> handle that correctly: either peer here is not a pointer, and the content
>> is copied,
>
> I prefer this, because it is smaller, like:
>
> '''
> static int sock_connect_mptcp(const char * const remoteaddr,
>                   const char * const port, int proto,
> -                 struct addrinfo **peer,
> +                 struct sockaddr_storage *peer, socklen_t *peer_len,
>                   int infd, struct wstate *winfo)
> {
> ...
> -               *peer = a;
>                 break;
> ...
> -               *peer = a;
>                 break;
> ...
> +   if (sock != -1) {
> +       memcpy(peer, a->ai_addr, a->ai_addrlen);
> +       *peer_len = a->ai_addrlen;
> +   }
>     freeaddrinfo(addr);
> ...
>
> -   struct addrinfo *peer = NULL;
> +   struct sockaddr_storage peer;
> +   socklen_t peer_len;
> ...
> -   fd = sock_connect_mptcp(..., &peer, fd_in, &winfo);
> +   fd = sock_connect_mptcp(..., &peer, &peer_len, fd_in, &winfo);
> ...
> -   if (connect(fd, peer->ai_addr, peer->ai_addrlen))
> +   if (connect(fd, (struct sockaddr *)&peer, peer_len))
> '''

It looks OK but there are some modifications a bit everywhere.

>> or you pass &addr to sock_connect_mptcp, and you do the free here.
>
> This needs a refactor of sock_connect_mptcp – I used to do it that way. But I
> worry that backporting could be tricky. If you're fine with it, I'm okay with this plan
> as well.

I didn't check in details, but it seems like it would be shorter: just need to pass &addr to sock_connect_mptcp(... ••addr, ...) and use *addr
instead of addr in two places + moving the declaration and free to the
parent function. No?

If I'm missing something, the copy is fine.

Cheers,
Matt

Re: [PATCH mptcp-net] selftests: mptcp: fix an UAF in mptcp_connect.c

[Paolo Abeni]

Hi,

On 8/14/26 8:42 AM, gang.yan@linux.dev wrote:
> Sorry for this, after reviewing sashiko's comment, I think this modification
> below should be better:
> 
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.c b/tools/testing/selftests/net/mptcp/mptcp_connect.c
> index ea4cb6c1bd5e..c81ec4400bef 100644
> --- a/tools/testing/selftests/net/mptcp/mptcp_connect.c
> +++ b/tools/testing/selftests/net/mptcp/mptcp_connect.c
> @@ -421,7 +421,8 @@ static int sock_connect_mptcp(const char * const remoteaddr,
>                 sock = -1;
>         }
>  
> -       freeaddrinfo(addr);
> +       if (sock == -1)
> +               freeaddrinfo(addr);
>         if (sock != -1)
>                 SOCK_TEST_TCPULP(sock, proto);
>         return sock;
> @@ -1424,6 +1425,7 @@ int main_loop(void)
>         }
>  
>  out:
> +       freeaddrinfo(peer);
>         if (cfg_input)
>                 close(fd_in);
> 
> If you think it's OK, I can send v2 immediately.
I'm sorry for being in late.

This is in a better direction but still will be not fully correct:
freeaddrinfo() must be called on the argument returned by getaddrinfo()
- in this case: `addr`.

Otherwise some addresses will not be freed.

Since sock_connect_mptcp() is invoked only once per program execution,
and not-freed memory will _not_ be leaked at process exit time, I think
the easier solution is to remove the:

	freeaddrinfo(addr);

statement, adding a comment alike:

	/* All the allocated memory is released at exit() time, this
	 * is executed only once and ownership of a single address has
	 * to be transferred to the caller. Keep it simple and avoid
	 * later freeaddrinfo() entirely.
	 */
	xgetaddrinfo(listenaddr, port, &hints, &addr);

/P

Re: [PATCH mptcp-net] selftests: mptcp: fix an UAF in mptcp_connect.c

[gang.yan]

August 14, 2026 at 6:50 PM, "Paolo Abeni" <pabeni@redhat.com mailto:pabeni@redhat.com?to=%22Paolo%20Abeni%22%20%3Cpabeni%40redhat.com%3E > wrote:


> 
> Hi,
> 
> On 8/14/26 8:42 AM, gang.yan@linux.dev wrote:
> 
> > 
> > Sorry for this, after reviewing sashiko's comment, I think this modification
> >  below should be better:
> >  
> >  diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.c b/tools/testing/selftests/net/mptcp/mptcp_connect.c
> >  index ea4cb6c1bd5e..c81ec4400bef 100644
> >  --- a/tools/testing/selftests/net/mptcp/mptcp_connect.c
> >  +++ b/tools/testing/selftests/net/mptcp/mptcp_connect.c
> >  @@ -421,7 +421,8 @@ static int sock_connect_mptcp(const char * const remoteaddr,
> >  sock = -1;
> >  }
> >  
> >  - freeaddrinfo(addr);
> >  + if (sock == -1)
> >  + freeaddrinfo(addr);
> >  if (sock != -1)
> >  SOCK_TEST_TCPULP(sock, proto);
> >  return sock;
> >  @@ -1424,6 +1425,7 @@ int main_loop(void)
> >  }
> >  
> >  out:
> >  + freeaddrinfo(peer);
> >  if (cfg_input)
> >  close(fd_in);
> >  
> >  If you think it's OK, I can send v2 immediately.
> > 
> I'm sorry for being in late.
> 

Hi Paolo,

So nice to get your reply.

> This is in a better direction but still will be not fully correct:
> freeaddrinfo() must be called on the argument returned by getaddrinfo()
> - in this case: `addr`.
> 
> Otherwise some addresses will not be freed.
> 
> Since sock_connect_mptcp() is invoked only once per program execution,
> and not-freed memory will _not_ be leaked at process exit time, I think
> the easier solution is to remove the:
> 
>  freeaddrinfo(addr);

Sounds great!

> 
> statement, adding a comment alike:
> 
>  /* All the allocated memory is released at exit() time, this
>  * is executed only once and ownership of a single address has
>  * to be transferred to the caller. Keep it simple and avoid
>  * later freeaddrinfo() entirely.
>  */

I've just send the v2 and using AI to simplify the comment —— hope you
don’t mind.

Thanks
Gang

>  xgetaddrinfo(listenaddr, port, &hints, &addr);
> 
> /P
>