[PATCH] selftests: mptcp: mptcp_diag: fix stack buffer overflow in get_subflow_info()

Jiangshan Yi posted 1 patch 3 days, 18 hours ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/multipath-tcp/mptcp_net-next tags/patchew/20260701103809.4051377-1-yijiangshan@kylinos.cn
tools/testing/selftests/net/mptcp/mptcp_diag.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] selftests: mptcp: mptcp_diag: fix stack buffer overflow in get_subflow_info()
Posted by Jiangshan Yi 3 days, 18 hours ago
get_subflow_info() parses the subflow address string with:

	char saddr[64], daddr[64];

	ret = sscanf(subflow_addrs, "%[^:]:%d %[^:]:%d",
		     saddr, &sport, daddr, &dport);

The subflow_addrs buffer holds up to 1024 bytes and is taken directly
from the command line ("-c" argument). The "%[^:]" conversions have no
maximum field width, so if the address substring before the ':' exceeds
63 bytes, sscanf() writes past the end of the 64-byte saddr/daddr stack
buffers. This overflows the stack, corrupting adjacent stack data such
as the saved return address, and can crash the tool or lead to
out-of-bounds writes controlled by user-supplied input.

Bound both string conversions to the destination buffer size by adding
an explicit maximum field width of 63 (leaving room for the terminating
NUL), so at most 63 bytes are written into each 64-byte buffer:

	ret = sscanf(subflow_addrs, "%63[^:]:%d %63[^:]:%d",
		     saddr, &sport, daddr, &dport);

Signed-off-by: Jiangshan Yi <yijiangshan@kylinos.cn>
---
 tools/testing/selftests/net/mptcp/mptcp_diag.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_diag.c b/tools/testing/selftests/net/mptcp/mptcp_diag.c
index 5e222ba977e4..02ac93f794fe 100644
--- a/tools/testing/selftests/net/mptcp/mptcp_diag.c
+++ b/tools/testing/selftests/net/mptcp/mptcp_diag.c
@@ -377,7 +377,7 @@ static void get_subflow_info(char *subflow_addrs)
 	int ret;
 	int fd;
 
-	ret = sscanf(subflow_addrs, "%[^:]:%d %[^:]:%d", saddr, &sport, daddr, &dport);
+	ret = sscanf(subflow_addrs, "%63[^:]:%d %63[^:]:%d", saddr, &sport, daddr, &dport);
 	if (ret != 4)
 		die_perror("IP PORT Pairs has style problems!");
 
-- 
2.25.1
Re: [PATCH] selftests: mptcp: mptcp_diag: fix stack buffer overflow in get_subflow_info()
Posted by Geliang Tang 3 days, 1 hour ago
Hi Jiangshan,

On Wed, 2026-07-01 at 18:38 +0800, Jiangshan Yi wrote:
> get_subflow_info() parses the subflow address string with:
> 
> 	char saddr[64], daddr[64];
> 
> 	ret = sscanf(subflow_addrs, "%[^:]:%d %[^:]:%d",
> 		     saddr, &sport, daddr, &dport);
> 
> The subflow_addrs buffer holds up to 1024 bytes and is taken directly
> from the command line ("-c" argument). The "%[^:]" conversions have
> no
> maximum field width, so if the address substring before the ':'
> exceeds
> 63 bytes, sscanf() writes past the end of the 64-byte saddr/daddr
> stack
> buffers. This overflows the stack, corrupting adjacent stack data
> such
> as the saved return address, and can crash the tool or lead to
> out-of-bounds writes controlled by user-supplied input.
> 
> Bound both string conversions to the destination buffer size by
> adding
> an explicit maximum field width of 63 (leaving room for the
> terminating
> NUL), so at most 63 bytes are written into each 64-byte buffer:
> 
> 	ret = sscanf(subflow_addrs, "%63[^:]:%d %63[^:]:%d",
> 		     saddr, &sport, daddr, &dport);
> 
> Signed-off-by: Jiangshan Yi <yijiangshan@kylinos.cn>
> ---
>  tools/testing/selftests/net/mptcp/mptcp_diag.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_diag.c
> b/tools/testing/selftests/net/mptcp/mptcp_diag.c
> index 5e222ba977e4..02ac93f794fe 100644
> --- a/tools/testing/selftests/net/mptcp/mptcp_diag.c
> +++ b/tools/testing/selftests/net/mptcp/mptcp_diag.c
> @@ -377,7 +377,7 @@ static void get_subflow_info(char *subflow_addrs)
>  	int ret;
>  	int fd;
>  
> -	ret = sscanf(subflow_addrs, "%[^:]:%d %[^:]:%d", saddr,
> &sport, daddr, &dport);
> +	ret = sscanf(subflow_addrs, "%63[^:]:%d %63[^:]:%d", saddr,
> &sport, daddr, &dport);

Thanks for this patch. MPTCP CI complains:

WARNING: line length of 91 exceeds 80 columns
#44: FILE: tools/testing/selftests/net/mptcp/mptcp_diag.c:380:

Also, for the subject prefix, we usually use "selftests: mptcp: diag:"
instead of "selftests: mptcp: mptcp_diag:". Please consider updating it
if you spin a v2.

Thanks,
-Geliang

>  	if (ret != 4)
>  		die_perror("IP PORT Pairs has style problems!");
>  

Re: [PATCH] selftests: mptcp: mptcp_diag: fix stack buffer overflow in get_subflow_info()
Posted by Geliang Tang 3 days, 1 hour ago
On Thu, 2026-07-02 at 11:06 +0800, Geliang Tang wrote:
> Hi Jiangshan,
> 
> On Wed, 2026-07-01 at 18:38 +0800, Jiangshan Yi wrote:
> > get_subflow_info() parses the subflow address string with:
> > 
> > 	char saddr[64], daddr[64];
> > 
> > 	ret = sscanf(subflow_addrs, "%[^:]:%d %[^:]:%d",
> > 		     saddr, &sport, daddr, &dport);
> > 
> > The subflow_addrs buffer holds up to 1024 bytes and is taken
> > directly
> > from the command line ("-c" argument). The "%[^:]" conversions have
> > no
> > maximum field width, so if the address substring before the ':'
> > exceeds
> > 63 bytes, sscanf() writes past the end of the 64-byte saddr/daddr
> > stack
> > buffers. This overflows the stack, corrupting adjacent stack data
> > such
> > as the saved return address, and can crash the tool or lead to
> > out-of-bounds writes controlled by user-supplied input.
> > 
> > Bound both string conversions to the destination buffer size by
> > adding
> > an explicit maximum field width of 63 (leaving room for the
> > terminating
> > NUL), so at most 63 bytes are written into each 64-byte buffer:
> > 
> > 	ret = sscanf(subflow_addrs, "%63[^:]:%d %63[^:]:%d",
> > 		     saddr, &sport, daddr, &dport);
> > 
> > Signed-off-by: Jiangshan Yi <yijiangshan@kylinos.cn>
> > ---
> >  tools/testing/selftests/net/mptcp/mptcp_diag.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/tools/testing/selftests/net/mptcp/mptcp_diag.c
> > b/tools/testing/selftests/net/mptcp/mptcp_diag.c
> > index 5e222ba977e4..02ac93f794fe 100644
> > --- a/tools/testing/selftests/net/mptcp/mptcp_diag.c
> > +++ b/tools/testing/selftests/net/mptcp/mptcp_diag.c
> > @@ -377,7 +377,7 @@ static void get_subflow_info(char
> > *subflow_addrs)
> >  	int ret;
> >  	int fd;
> >  
> > -	ret = sscanf(subflow_addrs, "%[^:]:%d %[^:]:%d", saddr,
> > &sport, daddr, &dport);
> > +	ret = sscanf(subflow_addrs, "%63[^:]:%d %63[^:]:%d",
> > saddr,
> > &sport, daddr, &dport);
> 
> Thanks for this patch. MPTCP CI complains:
> 
> WARNING: line length of 91 exceeds 80 columns
> #44: FILE: tools/testing/selftests/net/mptcp/mptcp_diag.c:380:
> 
> Also, for the subject prefix, we usually use "selftests: mptcp:
> diag:"
> instead of "selftests: mptcp: mptcp_diag:". Please consider updating
> it
> if you spin a v2.

Also, when you send v2, please only CC the MPTCP mailing list
(mptcp@lists.linux.dev); there's no need to CC netdev or other mailing
lists.

Thanks,
-Geliang

> 
> Thanks,
> -Geliang
> 
> >  	if (ret != 4)
> >  		die_perror("IP PORT Pairs has style problems!");
> >  
> 
> 

Re: Re: [PATCH] selftests: mptcp: mptcp_diag: fix stack buffer overflow in get_subflow_info()
Posted by Jiangshan Yi 3 days, 1 hour ago
>> Thanks for this patch. MPTCP CI complains:
>> 
>> WARNING: line length of 91 exceeds 80 columns
>> #44: FILE: tools/testing/selftests/net/mptcp/mptcp_diag.c:380:
>> 
>> Also, for the subject prefix, we usually use "selftests: mptcp:
>> diag:"
>> instead of "selftests: mptcp: mptcp_diag:". Please consider updating
>> it
>> if you spin a v2.
>
>Also, when you send v2, please only CC the MPTCP mailing list
>(mptcp@lists.linux.dev); there's no need to CC netdev or other mailing
>lists.
>
>Thanks,
>-Geliang

Hi Geliang,

Thanks for your review!

You're right. I'll fix the line length issue and update the subject
prefix to "selftests: mptcp: diag:" as suggested, then send a v2.

Thanks,
Jiangshan
Re: [PATCH] selftests: mptcp: mptcp_diag: fix stack buffer overflow in get_subflow_info()
Posted by MPTCP CI 3 days, 17 hours ago
Hi Jiangshan,

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/28512899070

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


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)