[PATCH] ipv4/mptcp: fix divide error

[PATCH] ipv4/mptcp: fix divide error

[Jiang Biao]

From: Jiang Biao <benbjiang@tencent.com>

From: Jiang Biao <tcs_robot@tencent.com>

There is a fix divide error reported,
divide error: 0000 [#1] PREEMPT SMP KASAN
RIP: 0010:tcp_tso_autosize build/../net/ipv4/tcp_output.c:1975 [inline]
RIP: 0010:tcp_tso_segs+0x14f/0x250 build/../net/ipv4/tcp_output.c:1992
Code: 38 d0 7c 08 84 d2 0f 85 d6 00 00 00 8b 83 8c 03 00 00 48 8d bb 9e 03 00 00 48 89 f9 2d 41 01 00 00 4c 39 e8 49 0f 47 c5 31 d2 <41> f7 f4 48 ba 00 00 00 00 00 fc ff df 39 e8 0f 42 c5 48 c1 e9 03
RSP: 0018:ffffc9000205f558 EFLAGS: 00010246
RAX: 000000000000febf RBX: ffff88801ed48000 RCX: ffff88801ed4839e
RDX: 0000000000000000 RSI: ffff888102ad2340 RDI: ffff88801ed4839e
RBP: 0000000000000002 R08: ffffffff8796862a R09: 000000000000003f
R10: 0000000000000001 R11: 000000000000000a R12: 0000000000000000
R13: 000000000024705b R14: 000000000000000a R15: ffff88801ed48350
FS:  00007f923ce99700(0000) GS:ffff888023e00000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f4f25fa3000 CR3: 0000000088b09000 CR4: 0000000000752ef0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
PKRU: 55555554
Call Trace:
 tcp_write_xmit+0x135/0x5d50 build/../net/ipv4/tcp_output.c:2623
 __tcp_push_pending_frames+0xab/0x390 build/../net/ipv4/tcp_output.c:2874
 tcp_push+0x473/0x6f0 build/../net/ipv4/tcp.c:736
 mptcp_push_release.isra.32+0x17c/0x280 build/../net/mptcp/protocol.c:1437
 __mptcp_push_pending+0x451/0x530 build/../net/mptcp/protocol.c:1478
 mptcp_sendmsg+0x1759/0x1c00 build/../net/mptcp/protocol.c:1697
 inet_sendmsg+0xa1/0xd0 build/../net/ipv4/af_inet.c:821
 sock_sendmsg_nosec build/../net/socket.c:703 [inline]
 sock_sendmsg+0xc9/0x120 build/../net/socket.c:723
 ____sys_sendmsg+0x375/0x820 build/../net/socket.c:2392
 ___sys_sendmsg+0x10a/0x180 build/../net/socket.c:2446
 __sys_sendmmsg+0x193/0x470 build/../net/socket.c:2532
 __do_sys_sendmmsg build/../net/socket.c:2561 [inline]
 __se_sys_sendmmsg build/../net/socket.c:2558 [inline]
 __x64_sys_sendmmsg+0x99/0x100 build/../net/socket.c:2558
 do_syscall_x64 build/../arch/x86/entry/common.c:50 [inline]
 do_syscall_64+0x34/0xb0 build/../arch/x86/entry/common.c:80
 entry_SYSCALL_64_after_hwframe+0x44/0xae

It's introduced by non-initialized info->mss_now in __mptcp_push_pending.
Fix it by adding protection in mptcp_push_release.

Signed-off-by: Jiang Biao <tcs_robot@tencent.com>
---
 net/mptcp/protocol.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index a88924947815..bfb3cd85bf19 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -1433,8 +1433,10 @@ static struct sock *mptcp_subflow_get_send(struct mptcp_sock *msk)
 static void mptcp_push_release(struct sock *sk, struct sock *ssk,
 			       struct mptcp_sendmsg_info *info)
 {
+	int mss_now = info->mss_now ? info->mss_now : tcp_current_mss(ssk);
+
 	mptcp_set_timeout(sk, ssk);
-	tcp_push(ssk, 0, info->mss_now, tcp_sk(ssk)->nonagle, info->size_goal);
+	tcp_push(ssk, 0, mss_now, tcp_sk(ssk)->nonagle, info->size_goal);
 	release_sock(ssk);
 }

--
2.21.0

Re: [PATCH] ipv4/mptcp: fix divide error

[Matthieu Baerts]

Hi Jiang,

On 24/08/2021 09:19, Jiang Biao wrote:

(...)

> There is a fix divide error reported,
> divide error: 0000 [#1] PREEMPT SMP KASAN
> RIP: 0010:tcp_tso_autosize build/../net/ipv4/tcp_output.c:1975 [inline]
> RIP: 0010:tcp_tso_segs+0x14f/0x250 build/../net/ipv4/tcp_output.c:1992

Thank you for this patch and validating MPTCP on your side!

This issue is actively tracked on our Github project [1] and a patch is
already in our tree [2] but still under validation.
> It's introduced by non-initialized info->mss_now in __mptcp_push_pending.
> Fix it by adding protection in mptcp_push_release.

Indeed, you are right, info->mss_now can be set to 0 in some cases but
that's not normal.

Instead of adding a protection here, we preferred fixing the root cause,
see [2]. Do not hesitate to have a look at the other patch and comment
there if you don't agree with this version.
Except if [2] is difficult to backport, I think we don't need your extra
protection. WDYT?

Cheers,
Matt

[1] https://github.com/multipath-tcp/mptcp_net-next/issues/219
[2]
https://patchwork.kernel.org/project/mptcp/patch/286de8f451b32f60e75d3b8bcc4df515e186b930.1629481305.git.pabeni@redhat.com/
-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net

Re: [PATCH] ipv4/mptcp: fix divide error

[Jiang Biao]

Hi,

On Tue, 24 Aug 2021 at 15:36, Matthieu Baerts
<matthieu.baerts@tessares.net> wrote:
>
> Hi Jiang,
>
> On 24/08/2021 09:19, Jiang Biao wrote:
>
> (...)
>
> > There is a fix divide error reported,
> > divide error: 0000 [#1] PREEMPT SMP KASAN
> > RIP: 0010:tcp_tso_autosize build/../net/ipv4/tcp_output.c:1975 [inline]
> > RIP: 0010:tcp_tso_segs+0x14f/0x250 build/../net/ipv4/tcp_output.c:1992
>
> Thank you for this patch and validating MPTCP on your side!
>
> This issue is actively tracked on our Github project [1] and a patch is
> already in our tree [2] but still under validation.
> > It's introduced by non-initialized info->mss_now in __mptcp_push_pending.
> > Fix it by adding protection in mptcp_push_release.
>
> Indeed, you are right, info->mss_now can be set to 0 in some cases but
> that's not normal.
>
> Instead of adding a protection here, we preferred fixing the root cause,
> see [2]. Do not hesitate to have a look at the other patch and comment
> there if you don't agree with this version.
> Except if [2] is difficult to backport, I think we don't need your extra
> protection. WDYT?
>
Agreed, fixing the root cause is much better.
Thanks for the reply.

Regards,
Jiang

Re: [PATCH] ipv4/mptcp: fix divide error

[Mat Martineau]

On Tue, 24 Aug 2021, Jiang Biao wrote:

> Hi,
>
> On Tue, 24 Aug 2021 at 15:36, Matthieu Baerts
> <matthieu.baerts@tessares.net> wrote:
>>
>> Hi Jiang,
>>
>> On 24/08/2021 09:19, Jiang Biao wrote:
>>
>> (...)
>>
>>> There is a fix divide error reported,
>>> divide error: 0000 [#1] PREEMPT SMP KASAN
>>> RIP: 0010:tcp_tso_autosize build/../net/ipv4/tcp_output.c:1975 [inline]
>>> RIP: 0010:tcp_tso_segs+0x14f/0x250 build/../net/ipv4/tcp_output.c:1992
>>
>> Thank you for this patch and validating MPTCP on your side!
>>
>> This issue is actively tracked on our Github project [1] and a patch is
>> already in our tree [2] but still under validation.
>>> It's introduced by non-initialized info->mss_now in __mptcp_push_pending.
>>> Fix it by adding protection in mptcp_push_release.
>>
>> Indeed, you are right, info->mss_now can be set to 0 in some cases but
>> that's not normal.
>>
>> Instead of adding a protection here, we preferred fixing the root cause,
>> see [2]. Do not hesitate to have a look at the other patch and comment
>> there if you don't agree with this version.
>> Except if [2] is difficult to backport, I think we don't need your extra
>> protection. WDYT?
>>
> Agreed, fixing the root cause is much better.
> Thanks for the reply.
>

Hi Jiang -

Could you try cherry-picking this commit to see if it eliminates the error 
in your system?

https://github.com/multipath-tcp/mptcp_net-next/commit/9ef5aea5a794f4a369e26ed816e9c80cdc5a5f86


Thanks!

--
Mat Martineau
Intel

Re: [PATCH] ipv4/mptcp: fix divide error

[Jiang Biao]

On Thu, 26 Aug 2021 at 01:56, Mat Martineau
<mathew.j.martineau@linux.intel.com> wrote:
>
>
> On Tue, 24 Aug 2021, Jiang Biao wrote:
>
> > Hi,
> >
> > On Tue, 24 Aug 2021 at 15:36, Matthieu Baerts
> > <matthieu.baerts@tessares.net> wrote:
> >>
> >> Hi Jiang,
> >>
> >> On 24/08/2021 09:19, Jiang Biao wrote:
> >>
> >> (...)
> >>
> >>> There is a fix divide error reported,
> >>> divide error: 0000 [#1] PREEMPT SMP KASAN
> >>> RIP: 0010:tcp_tso_autosize build/../net/ipv4/tcp_output.c:1975 [inline]
> >>> RIP: 0010:tcp_tso_segs+0x14f/0x250 build/../net/ipv4/tcp_output.c:1992
> >>
> >> Thank you for this patch and validating MPTCP on your side!
> >>
> >> This issue is actively tracked on our Github project [1] and a patch is
> >> already in our tree [2] but still under validation.
> >>> It's introduced by non-initialized info->mss_now in __mptcp_push_pending.
> >>> Fix it by adding protection in mptcp_push_release.
> >>
> >> Indeed, you are right, info->mss_now can be set to 0 in some cases but
> >> that's not normal.
> >>
> >> Instead of adding a protection here, we preferred fixing the root cause,
> >> see [2]. Do not hesitate to have a look at the other patch and comment
> >> there if you don't agree with this version.
> >> Except if [2] is difficult to backport, I think we don't need your extra
> >> protection. WDYT?
> >>
> > Agreed, fixing the root cause is much better.
> > Thanks for the reply.
> >
>
> Hi Jiang -
>
> Could you try cherry-picking this commit to see if it eliminates the error
> in your system?
>
> https://github.com/multipath-tcp/mptcp_net-next/commit/9ef5aea5a794f4a369e26ed816e9c80cdc5a5f86
>
ok, I'll try and give you feedback.

Regards,
Jiang

Re: [PATCH] ipv4/mptcp: fix divide error

[Jiang Biao]

On Thu, 26 Aug 2021 at 01:56, Mat Martineau
<mathew.j.martineau@linux.intel.com> wrote:
>
>
> On Tue, 24 Aug 2021, Jiang Biao wrote:
>
> > Hi,
> >
> > On Tue, 24 Aug 2021 at 15:36, Matthieu Baerts
> > <matthieu.baerts@tessares.net> wrote:
> >>
> >> Hi Jiang,
> >>
> >> On 24/08/2021 09:19, Jiang Biao wrote:
> >>
> >> (...)
> >>
> >>> There is a fix divide error reported,
> >>> divide error: 0000 [#1] PREEMPT SMP KASAN
> >>> RIP: 0010:tcp_tso_autosize build/../net/ipv4/tcp_output.c:1975 [inline]
> >>> RIP: 0010:tcp_tso_segs+0x14f/0x250 build/../net/ipv4/tcp_output.c:1992
> >>
> >> Thank you for this patch and validating MPTCP on your side!
> >>
> >> This issue is actively tracked on our Github project [1] and a patch is
> >> already in our tree [2] but still under validation.
> >>> It's introduced by non-initialized info->mss_now in __mptcp_push_pending.
> >>> Fix it by adding protection in mptcp_push_release.
> >>
> >> Indeed, you are right, info->mss_now can be set to 0 in some cases but
> >> that's not normal.
> >>
> >> Instead of adding a protection here, we preferred fixing the root cause,
> >> see [2]. Do not hesitate to have a look at the other patch and comment
> >> there if you don't agree with this version.
> >> Except if [2] is difficult to backport, I think we don't need your extra
> >> protection. WDYT?
> >>
> > Agreed, fixing the root cause is much better.
> > Thanks for the reply.
> >
>
> Hi Jiang -
>
> Could you try cherry-picking this commit to see if it eliminates the error
> in your system?
>
> https://github.com/multipath-tcp/mptcp_net-next/commit/9ef5aea5a794f4a369e26ed816e9c80cdc5a5f86
>
Errors' gone with the patch.:)

Regards,
Jiang
>
> Thanks!
>
> --
> Mat Martineau
> Intel