* [PATCH mptcp-next] tcp: Add mss zero checks to avoid divide errors
@ 2022-09-09 12:19 Geliang Tang
2022-09-09 14:12 ` tcp: Add mss zero checks to avoid divide errors: Tests Results MPTCP CI
2022-09-09 14:44 ` [PATCH mptcp-next] tcp: Add mss zero checks to avoid divide errors Paolo Abeni
0 siblings, 2 replies; 4+ messages in thread
From: Geliang Tang @ 2022-09-09 12:19 UTC (permalink / raw)
To: mptcp; +Cc: Geliang Tang
If mss_now is set to zero when invoking tcp_push(), this divide error will occur:
[ 120.369955] divide error: 0000 [#1] PREEMPT SMP NOPTI
[ 120.369969] CPU: 4 PID: 15485 Comm: test_progs Tainted: G OE 6.0.0-rc3-mptcp+ #251
[ 120.369976] Hardware name: LENOVO 20UASA0901/20UASA0901, BIOS N2WET27W (1.17 ) 03/29/2021
[ 120.369980] RIP: 0010:tcp_tso_segs+0x68/0x90
[ 120.369990] Code: 0f b6 8e c9 03 00 00 d3 e8 89 c1 8b 83 f4 01 00 00 83 f9 1f 77 09 89 c6 d3 ee 89 f1 48 01 ca 48 39 d0 89 ee 48 0f 47 c2 31 d2 <48> f7 f6 0f b7 93 06 02 00 00 5b 5d 39 c7 0f 47 c7 39 d0 0f 47 c2
[ 120.369998] RSP: 0018:ffffab29d1547c60 EFLAGS: 00010246
[ 120.370004] RAX: 000000000000febf RBX: ffff9b4a82f83480 RCX: 000000000000febf
[ 120.370009] RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000002
[ 120.370013] RBP: 0000000000000000 R08: 0000000000000a20 R09: 0000000000000000
[ 120.370017] R10: 0000000000000000 R11: ffffab29d1547ae0 R12: ffff9b4b02980000
[ 120.370022] R13: 0000000000000000 R14: 0000000000000000 R15: ffff9b4a867ec100
[ 120.370027] FS: 00007f73957ff700(0000) GS:ffff9b4ded700000(0000) knlGS:0000000000000000
[ 120.370032] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 120.370037] CR2: 00007f73957ffda0 CR3: 0000000104102005 CR4: 00000000003706e0
[ 120.370042] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 120.370046] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[ 120.370050] Call Trace:
[ 120.370056] <TASK>
[ 120.370062] tcp_write_xmit+0x80/0x1270
[ 120.370069] __tcp_push_pending_frames+0x32/0xf0
[ 120.370076] __mptcp_push_pending+0x401/0x430
[ 120.370083] mptcp_sendmsg+0x41d/0x4c0
[ 120.370089] sock_sendmsg+0x58/0x70
[ 120.370094] __sys_sendto+0x11e/0x150
[ 120.370100] ? mptcp_setsockopt+0x2a8/0x1170
[ 120.370109] ? aa_sk_perm+0x3e/0x1f0
[ 120.370116] ? fpregs_assert_state_consistent+0x22/0x50
[ 120.370129] __x64_sys_sendto+0x24/0x30
[ 120.370133] do_syscall_64+0x37/0x90
[ 120.370138] entry_SYSCALL_64_after_hwframe+0x63/0xcd
[ 120.370145] RIP: 0033:0x7f7395c15b66
[ 120.370150] Code: d5 53 49 89 f4 89 fb 48 83 ec 10 e8 c4 f7 ff ff 45 31 c9 89 c5 45 31 c0 45 89 f2 4c 89 ea 4c 89 e6 89 df b8 2c 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 36 89 ef 48 89 44 24 08 e8 f6 f7 ff ff 48 8b
[ 120.370157] RSP: 002b:00007f73957fe860 EFLAGS: 00000246 ORIG_RAX: 000000000000002c
[ 120.370163] RAX: ffffffffffffffda RBX: 000000000000000b RCX: 00007f7395c15b66
[ 120.370167] RDX: 00000000000005dc RSI: 00007f73957fe8c0 RDI: 000000000000000b
[ 120.370172] RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000000
[ 120.370176] R10: 0000000000000000 R11: 0000000000000246 R12: 00007f73957fe8c0
[ 120.370180] R13: 00000000000005dc R14: 0000000000000000 R15: 0000000000000000
[ 120.370187] </TASK>
[ 120.370190] Modules linked in: bpf_testmod(OE) uhid btusb btrtl btbcm btintel bluetooth nf_tables ecdh_generic ecc iptable_nat bpfilter i2c_designware_platform i2c_designware_core iwlmvm x86_pkg_temp_thermal iwlwifi intel_lpss_pci thinkpad_acpi intel_lpss mfd_core ledtrig_audio battery platform_profile intel_pmc_core intel_hid hid_multitouch hid_generic usbhid nvme nvme_core i2c_hid_acpi i2c_hid pinctrl_cannonlake sg dm_multipath scsi_dh_rdac scsi_dh_emc scsi_dh_alua efivarfs
[ 120.370238] ---[ end trace 0000000000000000 ]---
[ 120.378634] RIP: 0010:tcp_tso_segs+0x68/0x90
[ 120.378644] Code: 0f b6 8e c9 03 00 00 d3 e8 89 c1 8b 83 f4 01 00 00 83 f9 1f 77 09 89 c6 d3 ee 89 f1 48 01 ca 48 39 d0 89 ee 48 0f 47 c2 31 d2 <48> f7 f6 0f b7 93 06 02 00 00 5b 5d 39 c7 0f 47 c7 39 d0 0f 47 c2
[ 120.378649] RSP: 0018:ffffab29d1547c60 EFLAGS: 00010246
[ 120.378653] RAX: 000000000000febf RBX: ffff9b4a82f83480 RCX: 000000000000febf
[ 120.378656] RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000002
[ 120.378658] RBP: 0000000000000000 R08: 0000000000000a20 R09: 0000000000000000
[ 120.378661] R10: 0000000000000000 R11: ffffab29d1547ae0 R12: ffff9b4b02980000
[ 120.378664] R13: 0000000000000000 R14: 0000000000000000 R15: ffff9b4a867ec100
[ 120.378666] FS: 00007f73957ff700(0000) GS:ffff9b4ded700000(0000) knlGS:0000000000000000
[ 120.378670] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 120.378672] CR2: 00007f73957ffda0 CR3: 0000000104102005 CR4: 00000000003706e0
[ 120.378675] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 120.378677] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
This patch adds necessary zero checks for mss_now in
tcp_set_skb_tso_segs() and tcp_tso_autosize() to avoid this divide
error.
Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
net/ipv4/tcp_output.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 290019de766d..74db29eb6ba6 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -1441,7 +1441,7 @@ static void tcp_queue_skb(struct sock *sk, struct sk_buff *skb)
/* Initialize TSO segments for a packet. */
static void tcp_set_skb_tso_segs(struct sk_buff *skb, unsigned int mss_now)
{
- if (skb->len <= mss_now) {
+ if (!mss_now || skb->len <= mss_now) {
/* Avoid the costly divide in the normal
* non-TSO case.
*/
@@ -1971,6 +1971,9 @@ static u32 tcp_tso_autosize(const struct sock *sk, unsigned int mss_now,
unsigned long bytes;
u32 r;
+ if (!mss_now)
+ return min_tso_segs;
+
bytes = sk->sk_pacing_rate >> READ_ONCE(sk->sk_pacing_shift);
r = tcp_min_rtt(tcp_sk(sk)) >> READ_ONCE(sock_net(sk)->ipv4.sysctl_tcp_tso_rtt_log);
--
2.35.3
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: tcp: Add mss zero checks to avoid divide errors: Tests Results
2022-09-09 12:19 [PATCH mptcp-next] tcp: Add mss zero checks to avoid divide errors Geliang Tang
@ 2022-09-09 14:12 ` MPTCP CI
2022-09-09 14:44 ` [PATCH mptcp-next] tcp: Add mss zero checks to avoid divide errors Paolo Abeni
1 sibling, 0 replies; 4+ messages in thread
From: MPTCP CI @ 2022-09-09 14:12 UTC (permalink / raw)
To: Geliang Tang; +Cc: mptcp
Hi Geliang,
Thank you for your modifications, that's great!
Our CI did some validations and here is its report:
- KVM Validation: normal:
- Unstable: 1 failed test(s): selftest_mptcp_join 🔴:
- Task: https://cirrus-ci.com/task/5840132505862144
- Summary: https://api.cirrus-ci.com/v1/artifact/task/5840132505862144/summary/summary.txt
- KVM Validation: debug:
- Success! ✅:
- Task: https://cirrus-ci.com/task/5277182552440832
- Summary: https://api.cirrus-ci.com/v1/artifact/task/5277182552440832/summary/summary.txt
Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/20ce85e9e5a6
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-debug
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 (Tessares)
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH mptcp-next] tcp: Add mss zero checks to avoid divide errors
2022-09-09 12:19 [PATCH mptcp-next] tcp: Add mss zero checks to avoid divide errors Geliang Tang
2022-09-09 14:12 ` tcp: Add mss zero checks to avoid divide errors: Tests Results MPTCP CI
@ 2022-09-09 14:44 ` Paolo Abeni
2022-09-23 2:24 ` Geliang Tang
1 sibling, 1 reply; 4+ messages in thread
From: Paolo Abeni @ 2022-09-09 14:44 UTC (permalink / raw)
To: Geliang Tang, mptcp
On Fri, 2022-09-09 at 20:19 +0800, Geliang Tang wrote:
> If mss_now is set to zero when invoking tcp_push(), this divide error will occur:
We should not call at all tcp_push() in such situation. How did you
reproduce the issue?
Adding more check in TCP for the above situation is very likely not the
correct thing to do. Instead we should prevent MPTCP from reaching for
tcp_push() in the critical situation.
Thanks!
Paolo
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH mptcp-next] tcp: Add mss zero checks to avoid divide errors
2022-09-09 14:44 ` [PATCH mptcp-next] tcp: Add mss zero checks to avoid divide errors Paolo Abeni
@ 2022-09-23 2:24 ` Geliang Tang
0 siblings, 0 replies; 4+ messages in thread
From: Geliang Tang @ 2022-09-23 2:24 UTC (permalink / raw)
To: Paolo Abeni; +Cc: Geliang Tang, mptcp
Hi Paolo,
Sorry for the late reply.
Paolo Abeni <pabeni@redhat.com> 于2022年9月9日周五 22:44写道：
>
> On Fri, 2022-09-09 at 20:19 +0800, Geliang Tang wrote:
> > If mss_now is set to zero when invoking tcp_push(), this divide error will occur:
>
> We should not call at all tcp_push() in such situation. How did you
> reproduce the issue?
This does not happen on the export branch, but only when I add the BPF
redundant scheduler.
>
> Adding more check in TCP for the above situation is very likely not the
> correct thing to do. Instead we should prevent MPTCP from reaching for
> tcp_push() in the critical situation.
I agree. Let's just drop this patch. I'll add the new one to avoid
calling tcp_push() in such situation into the " BPF redundant scheduler"
series If we really need it.
Thanks!
-Geliang
>
> Thanks!
>
> Paolo
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-09-23 2:23 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-09 12:19 [PATCH mptcp-next] tcp: Add mss zero checks to avoid divide errors Geliang Tang
2022-09-09 14:12 ` tcp: Add mss zero checks to avoid divide errors: Tests Results MPTCP CI
2022-09-09 14:44 ` [PATCH mptcp-next] tcp: Add mss zero checks to avoid divide errors Paolo Abeni
2022-09-23 2:24 ` Geliang Tang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).