Bug 268400
| Summary: | Page fault kernel panic with KTLS enabled | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | Base System | Reporter: | Daniel Ponte <amigan> | ||||||||||||
| Component: | kern | Assignee: | Mark Johnston <markj> | ||||||||||||
| Status: | Closed FIXED | ||||||||||||||
| Severity: | Affects Some People | CC: | amigan, kp, markj, net, viaprog, zlei | ||||||||||||
| Priority: | --- | Keywords: | crash | ||||||||||||
| Version: | 13.1-STABLE | ||||||||||||||
| Hardware: | amd64 | ||||||||||||||
| OS: | Any | ||||||||||||||
| Bug Depends on: | |||||||||||||||
| Bug Blocks: | 271607 | ||||||||||||||
| Attachments: |
|
||||||||||||||
|
Description
Daniel Ponte
2022-12-15 22:30:38 UTC
Created attachment 238823 [details]
backtrace
Attaching raw backtrace
Can you please share you network interfaces and firewall configuration `pf.conf` ? My interface configuration and pf policy are rather large and complex and it would take some time to sanitize them. Could I send directly to someone? That said, I can confirm this is still happening in 13.2-STABLE FreeBSD 13.2-STABLE #17 stable/13-n254582-c0e7e1848360: Thu Feb 16 15:19:52 EST 2023. Digging deeper, the panic appears to be caused by a pf rule with route-to/reply-to rerouting to an IPv6 gif(4) interface with MTU of only 1280. It is caused by KTLS trying to send a packet that gets rerouted by pf (likely by reply-to) that results in ICMPv6 type 2 (Packet Too Big). (In reply to Daniel Ponte from comment #3) It is best to have a minimal setup (pf rules / interfaces) so that it will be easy to reproduce and debug. If it is production usage, then you may want to send privately. From what you describe I suspect it may be a coner case, a combination of KTLS / pf route-to / reduced MTU. I'll CC @kp to see if he has any insights. You can send to me zlei@FreeBSD.org . Yes, the entire issue is the complexity of the setup, which is a production setup, and is bringing out a corner case. I do not need to use kTLS but I figured the project would be interested in fixing this panic, and I am happy to avail myself to the degree I am able. It looks like this happens while we're computing the checksum for the icmp6 header, and that we're dereferencing a NULL pointer. I'm relatively confident that we're processing an unmapped mbuf, and that's why we're panicing here. Mark taught in_cksum_skip() to handle those, but I think in6_cksum() doesn't handle them. So we either need to insert an mb_unmapped_to_ext() call in in6_cksum(), or we need to teach in6_cksum() to deal with unmapped mbufs. (In reply to Kristof Provost from comment #6) I think you're right. I thought there was a reason for not converting the in6_cksum() routines to use m_apply(), but I can't find it. Would this basically involve porting dfd5240189ca0 to netinet6, or am I missing the big picture? Created attachment 240668 [details] workaround (In reply to Daniel Ponte from comment #8) That's right. That would take a bit of time. In the meantime, if you're willing to test a patch, the attached one should work around the problem. (In reply to Mark Johnston from comment #9) Thank you. I have applied and am building now. (In reply to Daniel Ponte from comment #10) Have you had success with the patch applied? Yes and no. I have had no further crashes from this issue with the patch applied. However, it appears that KTLS is now totally broken for me (clients never receive the response from nginx). I don't think it is because of this patch, though. Actually, I slightly recant that statement. Doing more testing: * KTLS and IPv4 works fine * KTLS and IPv6 works fine unless the response is large. So, maybe this is related to this patch. It looks like hosts are receiving incorrect TCP checksums, both over IPv4 and IPv6. Rolling back this patch appears to resolve the incorrect checksum issues. (In reply to Daniel Ponte from comment #14) The patch should not have any effect on IPv4 packets. I know, which is why I am somewhat stumped, here.
15:49:07.223007 IP (tos 0x0, ttl 54, id 0, offset 0, flags [DF], proto TCP (6), length 40)
71.xxx.xxx.xxx.443 > 67.yyy.yyy.yyy.13889: Flags [R], cksum 0xbbe1 (correct), seq 1468723210, win 0, length 0
15:49:13.723854 IP (tos 0x0, ttl 54, id 0, offset 0, flags [DF], proto TCP (6), length 1500)
71.xxx.xxx.xxx.443 > 67.yyy.yyy.yyy.52767: Flags [.], cksum 0x06e1 (incorrect -> 0xe541), seq 6527:7975, ack 823, win 1027, options [nop,nop,TS val 739004335 ecr 3884435406], length 1448
15:49:29.282814 IP (tos 0x0, ttl 54, id 0, offset 0, flags [DF], proto TCP (6), length 52)
71.xxx.xxx.xxx.443 > 67.yyy.yyy.yyy.52767: Flags [R.], cksum 0x2746 (correct), seq 14417, ack 823, win 0, options [nop,nop,TS val 739019895 ecr 3884435406], length 0
15:49:29.282858 IP (tos 0x0, ttl 64, id 0, offset 0, flags [DF], proto TCP (6), length 52)
67.yyy.yyy.yyy.52767 > 71.xxx.xxx.xxx.443: Flags [.], cksum 0x7494 (incorrect -> 0x4bbf), seq 823, ack 6527, win 1026, options [nop,nop,TS val 3884512321 ecr 738940511], length 0
15:49:29.297044 IP (tos 0x0, ttl 54, id 0, offset 0, flags [DF], proto TCP (6), length 40)
71.xxx.xxx.xxx.443 > 67.yyy.yyy.yyy.52767: Flags [R], cksum 0xeabd (correct), seq 190739836, win 0, length 0
The 71.xxx host is the machine that had this patch applied. I have modified the patch to wrap the mb_unmapped_to_ext and goto in a conditional for a sysctl. KTLS appears to work again with no checksum problems, but I will also wait to see whether the machine crashes, in case my sysctl is broken.
Note that the tcpdump was being run on the 67 host, so NIC checksum offload would not apply to the incorrect received checksum (but certainly may to the incorrect sent one). I disabled all hardware offloads on the 67 machine as well (70 always had them disabled) and now I am seeing correct checksums everywhere. I will continue to monitor the situation with this patch and KTLS enabled with latest 13-STABLE. I am keeping my sysctl in place in case weird things happen again and I can simply toggle it to see what is going on. (In reply to Daniel Ponte from comment #19) Sorry for the delay. To be clear, the patch now appears to be holding up? I am working on a proper patch now, for inclusion into 14.0. (In reply to Mark Johnston from comment #20) Yes, the patch has been stable for some time now. Created attachment 242830 [details]
proposed patch
This is a proper solution to the problem. The patch is only lightly tested so far, but please give it a try in a test environment if possible.
Created attachment 242831 [details]
proposed patch
Uploaded the correct patch this time.
Created attachment 242832 [details]
proposed patch
Sigh. Third time's the charm. Sorry for the noise.
I am now using this patch and will keep an eye on the machine. (In reply to Daniel Ponte from comment #25) Thanks. The main thing to watch for would be incorrect v6 checksums in packets originating from the patched machine, or drops due to "incorrect" checksums in received packets. The review is here: https://reviews.freebsd.org/D40598 *** Bug 271550 has been marked as a duplicate of this bug. *** A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=6775ef4188b4d4c023e76ebd2b71fa8c2c7e7cd2 commit 6775ef4188b4d4c023e76ebd2b71fa8c2c7e7cd2 Author: Mark Johnston <markj@FreeBSD.org> AuthorDate: 2023-06-23 13:55:43 +0000 Commit: Mark Johnston <markj@FreeBSD.org> CommitDate: 2023-06-23 13:55:43 +0000 netinet6: Implement in6_cksum_partial() using m_apply() This ensures that in6_cksum_partial() can be applied to unmapped mbufs, which can happen at least when icmp6_reflect() quotes a packet. The basic idea is to restructure in6_cksum_partial() to operate on one mbuf at a time. If the buffer length is odd or unaligned, an extra residual byte may be returned, to be incorporated into the checksum when processing the next buffer. PR: 268400 Reviewed by: cy MFC after: 2 weeks Sponsored by: The FreeBSD Foundation Differential Revision: https://reviews.freebsd.org/D40598 sys/netinet6/in6.h | 6 +- sys/netinet6/in6_cksum.c | 300 +++++++++++++++++++++-------------------------- 2 files changed, 139 insertions(+), 167 deletions(-) A commit in branch stable/13 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=db6978e02401cc3c1ea6e965fffd2482b1dd6461 commit db6978e02401cc3c1ea6e965fffd2482b1dd6461 Author: Mark Johnston <markj@FreeBSD.org> AuthorDate: 2023-06-23 13:55:43 +0000 Commit: Mark Johnston <markj@FreeBSD.org> CommitDate: 2023-07-07 18:46:41 +0000 netinet6: Implement in6_cksum_partial() using m_apply() This ensures that in6_cksum_partial() can be applied to unmapped mbufs, which can happen at least when icmp6_reflect() quotes a packet. The basic idea is to restructure in6_cksum_partial() to operate on one mbuf at a time. If the buffer length is odd or unaligned, an extra residual byte may be returned, to be incorporated into the checksum when processing the next buffer. PR: 268400 Reviewed by: cy MFC after: 2 weeks Sponsored by: The FreeBSD Foundation Differential Revision: https://reviews.freebsd.org/D40598 (cherry picked from commit 6775ef4188b4d4c023e76ebd2b71fa8c2c7e7cd2) sys/netinet6/in6.h | 6 +- sys/netinet6/in6_cksum.c | 300 +++++++++++++++++++++-------------------------- 2 files changed, 139 insertions(+), 167 deletions(-) Thank you for the report. |
