[PATCH mptcp-next v3] selftests: mptcp: diag: fix stack buffer overflow in get_subflow_info()

Jiangshan Yi posted 1 patch 2 days, 22 hours ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/multipath-tcp/mptcp_net-next tags/patchew/20260702062909.4147641-1-yijiangshan@kylinos.cn
tools/testing/selftests/net/mptcp/mptcp_diag.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
[PATCH mptcp-next v3] selftests: mptcp: diag: fix stack buffer overflow in get_subflow_info()
Posted by Jiangshan Yi 2 days, 22 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);

Reviewed-by: Geliang Tang <geliang@kernel.org>
Signed-off-by: Jiangshan Yi <yijiangshan@kylinos.cn>
---
v3:
 - drop the Fixes tag: this is a cleanup/hardening, not a fix (Geliang)
 - drop the Suggested-by tag, keep Geliang's Reviewed-by (Geliang)
v2:
 - add field width to sscanf() (fix >80 col warning, MPTCP CI)
 - fix subject prefix: mptcp_diag: -> diag: (Geliang)
v1: 
 - initial submission

 tools/testing/selftests/net/mptcp/mptcp_diag.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_diag.c b/tools/testing/selftests/net/mptcp/mptcp_diag.c
index 5e222ba977e4..3b8d2c8a6216 100644
--- a/tools/testing/selftests/net/mptcp/mptcp_diag.c
+++ b/tools/testing/selftests/net/mptcp/mptcp_diag.c
@@ -377,7 +377,8 @@ 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 mptcp-next v3] selftests: mptcp: diag: fix stack buffer overflow in get_subflow_info()
Posted by Geliang Tang 2 days, 3 hours ago
Hi Jiangshan,

On Thu, 2026-07-02 at 14:29 +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);
> 
> Reviewed-by: Geliang Tang <geliang@kernel.org>
> Signed-off-by: Jiangshan Yi <yijiangshan@kylinos.cn>
> ---
> v3:
>  - drop the Fixes tag: this is a cleanup/hardening, not a fix
> (Geliang)
>  - drop the Suggested-by tag, keep Geliang's Reviewed-by (Geliang)

Actually, there's no need to send this v3, since it doesn't introduce
any code changes compared to v2; we could simply reply and continue the
discussion on the v2 thread.

Anyway, this patch LGTM. I've changed its status to "Queued".

Thanks,
-Geliang

> v2:
>  - add field width to sscanf() (fix >80 col warning, MPTCP CI)
>  - fix subject prefix: mptcp_diag: -> diag: (Geliang)
> v1: 
>  - initial submission
> 
>  tools/testing/selftests/net/mptcp/mptcp_diag.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_diag.c
> b/tools/testing/selftests/net/mptcp/mptcp_diag.c
> index 5e222ba977e4..3b8d2c8a6216 100644
> --- a/tools/testing/selftests/net/mptcp/mptcp_diag.c
> +++ b/tools/testing/selftests/net/mptcp/mptcp_diag.c
> @@ -377,7 +377,8 @@ 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!");
>  
Re: [PATCH mptcp-next v3] selftests: mptcp: diag: fix stack buffer overflow in get_subflow_info()
Posted by MPTCP CI 2 days, 21 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/28571296294

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


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)