MPTCP Linux Development
 help / color / mirror / Atom feed
* [PATCH mptcp-next v2] selftests: mptcp: diag: fix stack buffer overflow in get_subflow_info()
@ 2026-07-02  4:11 Jiangshan Yi
  2026-07-02  5:14 ` MPTCP CI
  2026-07-02  5:50 ` Geliang Tang
  0 siblings, 2 replies; 6+ messages in thread
From: Jiangshan Yi @ 2026-07-02  4:11 UTC (permalink / raw)
  To: geliang; +Cc: mptcp, yijiangshan

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);

Fixes: c7ac7452df70 ("selftests: mptcp: add helpers to get subflow_info")
Suggested-by: Geliang Tang <geliang@kernel.org>
Signed-off-by: Jiangshan Yi <yijiangshan@kylinos.cn>
---
v2:
 - add field width to sscanf() (fix >80 col warning, MPTCP CI)
 - fix subject prefix: mptcp_diag: -> diag: (Geliang Tang)

 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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH mptcp-next v2] selftests: mptcp: diag: fix stack buffer overflow in get_subflow_info()
  2026-07-02  4:11 [PATCH mptcp-next v2] selftests: mptcp: diag: fix stack buffer overflow in get_subflow_info() Jiangshan Yi
@ 2026-07-02  5:14 ` MPTCP CI
  2026-07-02  5:50 ` Geliang Tang
  1 sibling, 0 replies; 6+ messages in thread
From: MPTCP CI @ 2026-07-02  5:14 UTC (permalink / raw)
  To: Jiangshan Yi; +Cc: mptcp

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

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


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)

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH mptcp-next v2] selftests: mptcp: diag: fix stack buffer overflow in get_subflow_info()
  2026-07-02  4:11 [PATCH mptcp-next v2] selftests: mptcp: diag: fix stack buffer overflow in get_subflow_info() Jiangshan Yi
  2026-07-02  5:14 ` MPTCP CI
@ 2026-07-02  5:50 ` Geliang Tang
  2026-07-02  6:29   ` [PATCH mptcp-next v3] " Jiangshan Yi
  1 sibling, 1 reply; 6+ messages in thread
From: Geliang Tang @ 2026-07-02  5:50 UTC (permalink / raw)
  To: Jiangshan Yi; +Cc: mptcp

Hi Jiangshan,

On Thu, 2026-07-02 at 12:11 +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);

v2 looks good now.

    Reviewed-by: Geliang Tang <geliang@kernel.org>

> 
> Fixes: c7ac7452df70 ("selftests: mptcp: add helpers to get
> subflow_info")

I suggest to remove this Fixes tag when applying it. Since this is a
cleanup, not fix.

> Suggested-by: Geliang Tang <geliang@kernel.org>

Also remove this line, add my Reviewed-by tag is enough.

Thanks,
-Geliang

> Signed-off-by: Jiangshan Yi <yijiangshan@kylinos.cn>
> ---
> v2:
>  - add field width to sscanf() (fix >80 col warning, MPTCP CI)
>  - fix subject prefix: mptcp_diag: -> diag: (Geliang Tang)
> 
>  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!");
>  


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH mptcp-next v3] selftests: mptcp: diag: fix stack buffer overflow in get_subflow_info()
  2026-07-02  5:50 ` Geliang Tang
@ 2026-07-02  6:29   ` Jiangshan Yi
  2026-07-02  7:37     ` MPTCP CI
  2026-07-03  1:09     ` Geliang Tang
  0 siblings, 2 replies; 6+ messages in thread
From: Jiangshan Yi @ 2026-07-02  6:29 UTC (permalink / raw)
  To: geliang; +Cc: mptcp, yijiangshan

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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH mptcp-next v3] selftests: mptcp: diag: fix stack buffer overflow in get_subflow_info()
  2026-07-02  6:29   ` [PATCH mptcp-next v3] " Jiangshan Yi
@ 2026-07-02  7:37     ` MPTCP CI
  2026-07-03  1:09     ` Geliang Tang
  1 sibling, 0 replies; 6+ messages in thread
From: MPTCP CI @ 2026-07-02  7:37 UTC (permalink / raw)
  To: Jiangshan Yi; +Cc: mptcp

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)

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH mptcp-next v3] selftests: mptcp: diag: fix stack buffer overflow in get_subflow_info()
  2026-07-02  6:29   ` [PATCH mptcp-next v3] " Jiangshan Yi
  2026-07-02  7:37     ` MPTCP CI
@ 2026-07-03  1:09     ` Geliang Tang
  1 sibling, 0 replies; 6+ messages in thread
From: Geliang Tang @ 2026-07-03  1:09 UTC (permalink / raw)
  To: Jiangshan Yi; +Cc: mptcp

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!");
>  

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-07-03  1:09 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-02  4:11 [PATCH mptcp-next v2] selftests: mptcp: diag: fix stack buffer overflow in get_subflow_info() Jiangshan Yi
2026-07-02  5:14 ` MPTCP CI
2026-07-02  5:50 ` Geliang Tang
2026-07-02  6:29   ` [PATCH mptcp-next v3] " Jiangshan Yi
2026-07-02  7:37     ` MPTCP CI
2026-07-03  1:09     ` Geliang Tang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox