Bug 266442
| Summary: | kernel page fault on packet with broken lengths if ipfilter is loaded | ||
|---|---|---|---|
| Product: | Base System | Reporter: | Robert Morris <rtm> |
| Component: | kern | Assignee: | Cy Schubert <cy> |
| Status: | Closed FIXED | ||
| Severity: | Affects Some People | CC: | cy, emaste, net, zlei |
| Priority: | --- | ||
| Version: | CURRENT | ||
| Hardware: | Any | ||
| OS: | Any | ||
| Attachments: | |||
Confirmed. Same page fault occurs without ipfilter loaded. Created attachment 239852 [details]
Notify pfil of dropped mangled packet
Can you try this patch, please. Though it fixes the problem the final patch may return a different return code or may increment a counter. For now it avoids the panic.
Created attachment 239853 [details]
Updated patch with DTrace probe
This patch adds a DTrace SDT.
Created attachment 239857 [details]
Also add to the fr_bad packet count.
In addition to a DTrace probe, add to the bad packet count. These are the proposed two commits.
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=79f7745c098a766d34a4e072cdd1a06e6d0829d5 commit 79f7745c098a766d34a4e072cdd1a06e6d0829d5 Author: Cy Schubert <cy@FreeBSD.org> AuthorDate: 2023-02-02 00:49:08 +0000 Commit: Cy Schubert <cy@FreeBSD.org> CommitDate: 2023-02-02 17:41:22 +0000 ipfilter: Fix use after free on packet with broken lengths Under the scenario with a packet with length of 67 bytes, a header length using the default of 20 bytes and a TCP data offset (th_off) of 48 will cause m_pullup() to fail to make sure bytes are arragned contiguously. m_pullup() will free the mbuf chain and return a null. ipfilter stores the resultant mbuf address (or the resulting NULL) in its fr_info_t structure. Unfortuntely the eroneous packet is not flagged for drop. This results in a kernel page fault at line 410 of sys/netinet/ip_fastfwd.c as it tries to use a now previously freed, by m_pullup(), mbuf. PR: 266442 Reported by: Robert Morris <rtm@lcs.mit.edu> MFC after: 1 week sys/netpfil/ipfilter/netinet/fil.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) (In reply to Cy Schubert from comment #3) The latest kernel source (as of Feb 3) makes the reported crash go away when I try it. It was indeed an IP Filter bug. It was not telling pfil to drop the packet when m_pullup() had freed the mbuf chain while failing. The bug likely exists in NetBSD too. I've sent them an email. This ticket will be closed after the patch has been MFCed to stable. A commit in branch stable/13 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=1da7a8a066150bf132b3e1a48fad009212a0010a commit 1da7a8a066150bf132b3e1a48fad009212a0010a Author: Cy Schubert <cy@FreeBSD.org> AuthorDate: 2023-02-02 00:49:08 +0000 Commit: Cy Schubert <cy@FreeBSD.org> CommitDate: 2023-02-09 21:19:41 +0000 ipfilter: Fix use after free on packet with broken lengths Under the scenario with a packet with length of 67 bytes, a header length using the default of 20 bytes and a TCP data offset (th_off) of 48 will cause m_pullup() to fail to make sure bytes are arragned contiguously. m_pullup() will free the mbuf chain and return a null. ipfilter stores the resultant mbuf address (or the resulting NULL) in its fr_info_t structure. Unfortuntely the eroneous packet is not flagged for drop. This results in a kernel page fault at line 410 of sys/netinet/ip_fastfwd.c as it tries to use a now previously freed, by m_pullup(), mbuf. PR: 266442 Reported by: Robert Morris <rtm@lcs.mit.edu> (cherry picked from commit 79f7745c098a766d34a4e072cdd1a06e6d0829d5) sys/netpfil/ipfilter/netinet/fil.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) A commit in branch stable/12 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=351f2f68852ac3e1d0ef745dc024dd745b07a34f commit 351f2f68852ac3e1d0ef745dc024dd745b07a34f Author: Cy Schubert <cy@FreeBSD.org> AuthorDate: 2023-02-02 00:49:08 +0000 Commit: Cy Schubert <cy@FreeBSD.org> CommitDate: 2023-02-09 21:20:51 +0000 ipfilter: Fix use after free on packet with broken lengths Under the scenario with a packet with length of 67 bytes, a header length using the default of 20 bytes and a TCP data offset (th_off) of 48 will cause m_pullup() to fail to make sure bytes are arragned contiguously. m_pullup() will free the mbuf chain and return a null. ipfilter stores the resultant mbuf address (or the resulting NULL) in its fr_info_t structure. Unfortuntely the eroneous packet is not flagged for drop. This results in a kernel page fault at line 410 of sys/netinet/ip_fastfwd.c as it tries to use a now previously freed, by m_pullup(), mbuf. PR: 266442 Reported by: Robert Morris <rtm@lcs.mit.edu> (cherry picked from commit 79f7745c098a766d34a4e072cdd1a06e6d0829d5) sys/netpfil/ipfilter/netinet/fil.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) |
Created attachment 236590 [details] Inject a packet that causes a kernel page fault if ipfilter is loaded. If ipfilter is loaded, and a packet arrives with IP hlen = 20 bytes (the default), IP packet len = 67 bytes, and TCP th_off = 48 bytes, ipf_pullup() will call m_pullup(len=68), which fails and causes ipf_pullup() to free the mbuf and zero out the mbuf pointer in *fin->fin_mp. But the information that the packet was discarded is lost because ipf_pr_ipv4hdr() does not return an error to ipf_makefrip(). So the calling code thinks everything is OK, ip_tryforward() sees PFIL_PASS and uses m = *fin->fin_mp, and it crashes. Error indications should probably be made to flow up from ipf_pullup() through ipf_pr_ipv4hdr() to ipf_makefrip(), so that callers know not to try to use the freed mbuf. I've attached a demo: # cc -o pf7a pf7a.c # ./pf7a ... panic: Fatal page fault at 0xffffffc000488f14: 0x0000000000001d panic() at panic+0x2a page_fault_handler() at page_fault_handler+0x1d6 do_trap_supervisor() at do_trap_supervisor+0x76 cpu_exception_handler_supervisor() at cpu_exception_handler_supervisor+0x70 --- exception 13, tval = 0x1d ip_tryforward() at ip_tryforward+0x278 ip_input() at ip_input+0x356 netisr_dispatch_src() at netisr_dispatch_src+0xca netisr_dispatch() at netisr_dispatch+0x10 tunwrite_l3() at tunwrite_l3+0x182 tunwrite() at tunwrite+0x128 devfs_write_f() at devfs_write_f+0xa6 fo_write() at fo_write+0xa dofilewrite() at dofilewrite+0x66 kern_writev() at kern_writev+0x40 sys_write() at sys_write+0x54 syscallenter() at syscallenter+0xec ecall_handler() at ecall_handler+0x18 do_trap_user() at do_trap_user+0xea cpu_exception_handler_user() at cpu_exception_handler_user+0x72 Here's the call chain at the point where m_pullup() fails: ipf_pullup() at ipf_pullup+0x182 ipf_pr_pullup() at ipf_pr_pullup+0x5c ipf_pr_tcpcommon() at ipf_pr_tcpcommon+0x28e ipf_pr_tcp() at ipf_pr_tcp+0x46 ipf_pr_ipv4hdr() at ipf_pr_ipv4hdr+0x220 ipf_makefrip() at ipf_makefrip+0x60 ipf_check() at ipf_check+0x142 ipf_check_wrapper() at ipf_check_wrapper+0x88 pfil_mbuf_in() at pfil_mbuf_in+0x58 ip_tryforward() at ip_tryforward+0x1c0 ip_input() at ip_input+0x356