Bug 295198 - ipfilter apparently doesn't validate packet lengths
Summary: ipfilter apparently doesn't validate packet lengths
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: 16.0-CURRENT
Hardware: Any Any
: --- Affects Only Me
Assignee: Cy Schubert
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2026-05-11 13:33 UTC by Mark Johnston
Modified: 2026-05-27 13:46 UTC (History)
1 user (show)

See Also:


Attachments
Validate length before checksum. (2.03 KB, patch)
2026-05-11 15:51 UTC, Cy Schubert
no flags Details | Diff
Validate length vefore checksum. (2.03 KB, patch)
2026-05-11 16:45 UTC, Cy Schubert
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Johnston freebsd_committer freebsd_triage 2026-05-11 13:33:36 UTC
I saw a crash while running the test suite:

panic: in6_cksum_partial_l2: mbuf len (51) < off(40)+len(512)                                                                                                                                                                                                                                                                 
cpuid = 8                                                                                                                                                                                                                                                                                                                     
time = 1778434309                                                                                                                                                                                                                                                                                                             
KDB: stack backtrace:                                                                                                                                                                                                                                                                                                         
db_trace_self_wrapper() at db_trace_self_wrapper+0xa5/frame 0xfffffe00f11d7c70                                                                                 
kdb_backtrace() at kdb_backtrace+0xc6/frame 0xfffffe00f11d7dd0                                                                                                                                                                                                                                                                
vpanic() at vpanic+0x214/frame 0xfffffe00f11d7f70                                                                                                                                                                                                                                                                             
panic() at panic+0xb5/frame 0xfffffe00f11d8030                                                                                                                                                                                                                                                                                
in6_cksum_partial_l2() at in6_cksum_partial_l2+0x37e/frame 0xfffffe00f11d8120                                                                                                                                                                                                                                                 
fr_cksum() at fr_cksum+0x155/frame 0xfffffe00f11d8180                                                                                                                                                                                                                                                                         
ipf_checkl4sum() at ipf_checkl4sum+0x144/frame 0xfffffe00f11d81c0                                                                                                                                                                                                                                                             
ipf_checkv6sum() at ipf_checkv6sum+0x81/frame 0xfffffe00f11d81f0                                                                                                                                                                                                                                                              
ipf_makefrip() at ipf_makefrip+0x2396/frame 0xfffffe00f11d8350                                                                                                 
ipf_check() at ipf_check+0x3c4/frame 0xfffffe00f11d85f0                                                                                                                                                                                                                                                                       
ipf_check_wrapper6() at ipf_check_wrapper6+0xd3/frame 0xfffffe00f11d8650                                                                                                                                                                                                                                                      
pfil_mbuf_in() at pfil_mbuf_in+0x7c/frame 0xfffffe00f11d8690                                                                                                                                                                                                                                                                  
ip6_input() at ip6_input+0xbdd/frame 0xfffffe00f11d88b0                                                                                                                                                                                                                                                                       
netisr_dispatch_src() at netisr_dispatch_src+0x1aa/frame 0xfffffe00f11d8990                                                                                                                                                                                                                                                   
ether_demux() at ether_demux+0x301/frame 0xfffffe00f11d8a50                                                                                                                                                                                                                                                                   
ether_nh_input() at ether_nh_input+0x6ec/frame 0xfffffe00f11d8b30                                                                                                                                                                                                                                                             
netisr_dispatch_src() at netisr_dispatch_src+0x1aa/frame 0xfffffe00f11d8c10                                                                                                                                                                                                                                                   
ether_input() at ether_input+0x1ac/frame 0xfffffe00f11d8cf0                                                                                                    
epair_tx_start_deferred() at epair_tx_start_deferred+0x140/frame 0xfffffe00f11d8d50                                                                                                                                                                                                                                           
taskqueue_run_locked() at taskqueue_run_locked+0x3c2/frame 0xfffffe00f11d8eb0                                                                                                                                                                                                                                                 
taskqueue_thread_loop() at taskqueue_thread_loop+0x138/frame 0xfffffe00f11d8ef0                                                                                                                                                                                                                                               
fork_exit() at fork_exit+0xa3/frame 0xfffffe00f11d8f30                                                                                                                                                                                                                                                                        
fork_trampoline() at fork_trampoline+0xe/frame 0xfffffe00f11d8f30                                                                                                                                                                                                                                                             
--- trap 0x5be18f56, rip = 0x10b55be18f96, rsp = 0x10b55be18fc6, rbp = 0x10b55be18ee6 ---

The basic problem here is:
(gdb) frame 25
#25 ipf_makefrip (hlen=hlen@entry=40, ip=ip@entry=0xfffffe00f40c456e, fin=fin@entry=0xfffffe00f11d8410) at /home/markj/sb/main/src/sys/netpfil/ipfilter/netinet/fil.c:2029
2029                    ipf_pr_ipv6hdr(fin);
(gdb) p fin->fin_plen 
$17 = 552
(gdb) p fin->fin_m->m_pkthdr.len
$18 = 51

That is, the packet's IPv6 header says the packet is longer than it actually is.  This causes an underflow in fr_cksum() which causes the panic.  Surprisingly I cannot find any code in ipfilter which actually checks for this condition.  Of course, ip6_input() does, but that happens after pfil hooks run.

So I presume we should add a check for this, presumably in ipf_makefrip(), after the protocol-specific packet length field is extracted.  But who knows what other validation is missing.
Comment 1 Mark Johnston freebsd_committer freebsd_triage 2026-05-11 13:34:07 UTC
Cy, could you please take a look at this?
Comment 2 Cy Schubert freebsd_committer freebsd_triage 2026-05-11 15:51:09 UTC
Created attachment 270604 [details]
Validate length before checksum.

Can you try this patch?

BTW, which test, specifically, is it panicking at? I'd like to test it here as well. I'm currently running the netinet6 tests here. No panic yet. (The netpfil tests passed.)
Comment 3 Mark Johnston freebsd_committer freebsd_triage 2026-05-11 15:57:11 UTC
(In reply to Cy Schubert from comment #2)
The panic isn't reproducible, and the tests run in parallel so I don't know which one triggered it.  It should be easy to write a scapy script which injects an invalid packet though?
Comment 4 Cy Schubert freebsd_committer freebsd_triage 2026-05-11 16:12:59 UTC
I'll write up a test and try to break my sandbox machine.
Comment 5 Cy Schubert freebsd_committer freebsd_triage 2026-05-11 16:45:20 UTC
Created attachment 270606 [details]
Validate length vefore checksum.

This patch fixes a typo.
Comment 6 Cy Schubert freebsd_committer freebsd_triage 2026-05-13 18:33:28 UTC
Comment on attachment 270606 [details]
Validate length vefore checksum.

Obsoleting as a review will be submitted.
Comment 7 Cy Schubert freebsd_committer freebsd_triage 2026-05-19 15:47:29 UTC
See https://reviews.freebsd.org/D57095 for the review.
Comment 8 commit-hook freebsd_committer freebsd_triage 2026-05-20 15:35:55 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=8dfb0805fc31cd78940429ab0560dae7e8ab6536

commit 8dfb0805fc31cd78940429ab0560dae7e8ab6536
Author:     Cy Schubert <cy@FreeBSD.org>
AuthorDate: 2026-05-11 15:44:52 +0000
Commit:     Cy Schubert <cy@FreeBSD.org>
CommitDate: 2026-05-20 15:32:37 +0000

    ipfilter: Validate length before checksum

    Validate the length of the packet listed in the mbuf is the same as
    the calculated packet length. If not reject the packet and bump the
    bad packet stat.

    PR:             295198
    MFC after:      1 week
    Differential Revision:  https://reviews.freebsd.org/D57095

 sys/netpfil/ipfilter/netinet/fil.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)
Comment 9 commit-hook freebsd_committer freebsd_triage 2026-05-27 13:42:05 UTC
A commit in branch stable/15 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=77dd10b2408eced1ac9eb63e27658491bf3ef701

commit 77dd10b2408eced1ac9eb63e27658491bf3ef701
Author:     Cy Schubert <cy@FreeBSD.org>
AuthorDate: 2026-05-11 15:44:52 +0000
Commit:     Cy Schubert <cy@FreeBSD.org>
CommitDate: 2026-05-27 13:41:25 +0000

    ipfilter: Validate length before checksum

    Validate the length of the packet listed in the mbuf is the same as
    the calculated packet length. If not reject the packet and bump the
    bad packet stat.

    PR:             295198
    Differential Revision:  https://reviews.freebsd.org/D57095

    (cherry picked from commit 8dfb0805fc31cd78940429ab0560dae7e8ab6536)

 sys/netpfil/ipfilter/netinet/fil.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)
Comment 10 commit-hook freebsd_committer freebsd_triage 2026-05-27 13:43:08 UTC
A commit in branch stable/14 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=f296b1fd3e9a00835988215118d8d8b996f3c02b

commit f296b1fd3e9a00835988215118d8d8b996f3c02b
Author:     Cy Schubert <cy@FreeBSD.org>
AuthorDate: 2026-05-11 15:44:52 +0000
Commit:     Cy Schubert <cy@FreeBSD.org>
CommitDate: 2026-05-27 13:42:01 +0000

    ipfilter: Validate length before checksum

    Validate the length of the packet listed in the mbuf is the same as
    the calculated packet length. If not reject the packet and bump the
    bad packet stat.

    PR:             295198
    Differential Revision:  https://reviews.freebsd.org/D57095

    (cherry picked from commit 8dfb0805fc31cd78940429ab0560dae7e8ab6536)

 sys/netpfil/ipfilter/netinet/fil.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)