[PATCH mptcp-net] mptcp: fix divide-by-zero in __mptcp_push_pending close path

Shardul Bankar posted 1 patch 1 day, 20 hours ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/multipath-tcp/mptcp_net-next tags/patchew/20260523211800.2952905-1-shardul.b@mpiricsoftware.com
There is a newer version of this series
net/mptcp/protocol.c | 7 +++++++
1 file changed, 7 insertions(+)
[PATCH mptcp-net] mptcp: fix divide-by-zero in __mptcp_push_pending close path
Posted by Shardul Bankar 1 day, 20 hours ago
A divide-by-zero in tcp_tso_segs() is reachable via __mptcp_close_ssk ->
__mptcp_push_pending -> mptcp_push_release(ssk, &info) with info.mss_now
== 0, triggered by MPTCP_PM_CMD_FLUSH_ADDRS.  __mptcp_close_ssk() leaves
the ssk in a state where __tcp_can_send() is false (FIN_WAIT1/2, CLOSING,
LAST_ACK) and then unconditionally calls __mptcp_push_pending(sk, 0).
For such an ssk, mptcp_sendmsg_frag() returns -EAGAIN before reaching
its tcp_send_mss(), so info.mss_now stays 0 when the trailing
mptcp_push_release() feeds it into tcp_push() -> tcp_tso_autosize().

Commit 1094c6fe7280 ("mptcp: fix possible divide by zero") covered the
alloc-failure branch by moving tcp_send_mss() before allocation; it does
not cover this -EAGAIN-before-tcp_send_mss() case.

Initialise info->mss_now and ->size_goal at __subflow_push_pending()
entry, before any early-return path.  Idempotent with the assignment
inside mptcp_sendmsg_frag().

Found by an MPTCP protocol-flow harness extending BRF
(arXiv:2305.08782) via the MPTCP_PM_CMD_FLUSH_ADDRS kernel-PM admin
path; the oops is reached on the first qualifying run.

Fixes: 724cfd2ee8aa ("mptcp: allocate TX skbs in msk context")
Assisted-by: Claude:claude-opus-4-7
Signed-off-by: Shardul Bankar <shardul.b@mpiricsoftware.com>
---
 net/mptcp/protocol.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index a72a6ad6ee8b1..7977f60f98474 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -1627,6 +1627,13 @@ static int __subflow_push_pending(struct sock *sk, struct sock *ssk,
 	struct mptcp_data_frag *dfrag;
 	int len, copied = 0, err = 0;
 
+	/* Initialise mss_now/size_goal: mptcp_sendmsg_frag() may return
+	 * before reaching its own tcp_send_mss() (e.g. !__tcp_can_send()),
+	 * leaving __mptcp_push_pending()'s trailing mptcp_push_release()
+	 * with info.mss_now == 0 and tripping tcp_tso_segs() on the divide.
+	 */
+	info->mss_now = tcp_send_mss(ssk, &info->size_goal, info->flags);
+
 	while ((dfrag = mptcp_send_head(sk))) {
 		info->sent = dfrag->already_sent;
 		info->limit = dfrag->data_len;
-- 
2.34.1
Re: [PATCH mptcp-net] mptcp: fix divide-by-zero in __mptcp_push_pending close path
Posted by Paolo Abeni 1 day, 8 hours ago
On 5/23/26 11:18 PM, Shardul Bankar wrote:
> A divide-by-zero in tcp_tso_segs() is reachable via __mptcp_close_ssk ->
> __mptcp_push_pending -> mptcp_push_release(ssk, &info) with info.mss_now
> == 0, triggered by :.  __mptcp_close_ssk() leaves
> the ssk in a state where __tcp_can_send() is false (FIN_WAIT1/2, CLOSING,
> LAST_ACK) and then unconditionally calls __mptcp_push_pending(sk, 0).
> For such an ssk, mptcp_sendmsg_frag() returns -EAGAIN before reaching
> its tcp_send_mss(), so info.mss_now stays 0 when the trailing
> mptcp_push_release() feeds it into tcp_push() -> tcp_tso_autosize().
> 
> Commit 1094c6fe7280 ("mptcp: fix possible divide by zero") covered the
> alloc-failure branch by moving tcp_send_mss() before allocation; it does
> not cover this -EAGAIN-before-tcp_send_mss() case.
> 
> Initialise info->mss_now and ->size_goal at __subflow_push_pending()
> entry, before any early-return path.  Idempotent with the assignment
> inside mptcp_sendmsg_frag().
> 
> Found by an MPTCP protocol-flow harness extending BRF
> (arXiv:2305.08782) via the MPTCP_PM_CMD_FLUSH_ADDRS kernel-PM admin
> path; the oops is reached on the first qualifying run.
> 
> Fixes: 724cfd2ee8aa ("mptcp: allocate TX skbs in msk context")
> Assisted-by: Claude:claude-opus-4-7
> Signed-off-by: Shardul Bankar <shardul.b@mpiricsoftware.com>
> ---
>  net/mptcp/protocol.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index a72a6ad6ee8b1..7977f60f98474 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -1627,6 +1627,13 @@ static int __subflow_push_pending(struct sock *sk, struct sock *ssk,
>  	struct mptcp_data_frag *dfrag;
>  	int len, copied = 0, err = 0;
>  
> +	/* Initialise mss_now/size_goal: mptcp_sendmsg_frag() may return
> +	 * before reaching its own tcp_send_mss() (e.g. !__tcp_can_send()),
> +	 * leaving __mptcp_push_pending()'s trailing mptcp_push_release()
> +	 * with info.mss_now == 0 and tripping tcp_tso_segs() on the divide.
> +	 */
> +	info->mss_now = tcp_send_mss(ssk, &info->size_goal, info->flags);

Please don't add duplicated non trivial check in fast path. In fact the race 
outlined above is so thin that syzkaller has not been able to hit it so far.

Do you have a working reproducer? if so, could you please test vs the following?

---
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index a72a6ad6ee8b..8fe93b8459e2 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -1711,6 +1711,13 @@ void __mptcp_push_pending(struct sock *sk, unsigned int flags)
 					    (1 << ssk->sk_state) &
 					     (TCPF_FIN_WAIT1 | TCPF_FIN_WAIT2 | TCPF_CLOSE))
 						push_count--;
+
+
+					/* Prevent next mptcp_push_release()
+					 * from pushing data with 0 mss.
+					 */
+					release_sock(ssk);
+					ssk = NULL;
 					continue;
 				}
 				copied = true;
Re: [PATCH mptcp-net] mptcp: fix divide-by-zero in __mptcp_push_pending close path
Posted by Shardul Bankar 17 hours ago
Hi Paolo,

On Sun, 2026-05-24 at 10:56 +0200, Paolo Abeni wrote:
> On 5/23/26 11:18 PM, Shardul Bankar wrote:
> 
> 
> Please don't add duplicated non trivial check in fast path. In fact
> the race 
> outlined above is so thin that syzkaller has not been able to hit it
> so far.
> 

Fair point on the fast-path cost. Calling tcp_send_mss() at the entry
of __subflow_push_pending() pays a per-iteration cost in the hot send
path that only the rare close-path race needs.  Your alternative below
keeps the cost in the slow path where it belongs; I'll move to that
shape for v2.

> Do you have a working reproducer? if so, could you please test vs the
> following?
> 

Sorry, no reproducer yet. I'll update once I have one and have tested
your diff.  Meanwhile I'll send v2.

Thanks for the careful review.

Cheers,
Shardul
Re: [PATCH mptcp-net] mptcp: fix divide-by-zero in __mptcp_push_pending close path
Posted by MPTCP CI 1 day, 19 hours ago
Hi Shardul,

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

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


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)