Created attachment 230608 [details] Divert socket test code On https://forums.freebsd.org/threads/pf-divert-to-loop-problem.81508 the poster describes how the "divert-to" rule creates packet loops on FreeBSD 12.2, and I also independently reproduced this bug on 13.0 and 14-CURRENT too. It can be reproduced with this pf rule: pass out on em0 divert-to 0.0.0.0 port 2000 while running the attached C code, which binds a divert socket to 0.0.0.0:2000 and reads packets and writes them back unchanged. Adding some logging to the pf kernel module, I noticed that the PF_PACKET_LOOPED flag never gets set in the pf_test() function. Checking for conditions that set it which aren't being met, I think I found out why. The following one line change fixes the issue for me: ---snip--- diff --git a/sys/netpfil/pf/pf.c b/sys/netpfil/pf/pf.c index 1686def4626..bd71d338517 100644 --- a/sys/netpfil/pf/pf.c +++ b/sys/netpfil/pf/pf.c @@ -6496,7 +6496,7 @@ pf_test(int dir, int pflags, struct ifnet *ifp, struct mbuf **m0, struct inpcb * if (__predict_false(ip_divert_ptr != NULL) && ((ipfwtag = m_tag_locate(m, MTAG_IPFW_RULE, 0, NULL)) != NULL)) { struct ipfw_rule_ref *rr = (struct ipfw_rule_ref *)(ipfwtag+1); - if (rr->info & IPFW_IS_DIVERT && rr->rulenum == 0) { + if (rr->info & IPFW_IS_DIVERT /*&& rr->rulenum == 0*/) { if (pd.pf_mtag == NULL && ((pd.pf_mtag = pf_get_mtag(m)) == NULL)) { action = PF_DROP; ---snip--- Why does that work? It appears that the "rulenum" field is only written to in this one place: ((struct ipfw_rule_ref *)(ipfwtag+1))->rulenum = dir; and if "dir" is what I think it is, then as per /usr/include/netpfil/pf/pf.h: enum { PF_INOUT, PF_IN, PF_OUT }; the 0 in "rr->rulenum == 0" would be PF_INOUT, which packets never are. However checking for values 1 and 2 instead, didn't seem to fix the issue either. Only deleting the entire rr->rulenum check seems to fix it.
Keyword: patch or patch-ready – in lieu of summary line prefix: [patch] * bulk change for the keyword * summary lines may be edited manually (not in bulk). Keyword descriptions and search interface: <https://bugs.freebsd.org/bugzilla/describekeywords.cgi>
Created attachment 243379 [details] Test code to test pf divert-to
(In reply to Alfa from comment #2) Hi, i have the same infinity loop problem , i have tried PF Divert rules given below on between FreeBSD 11.0 to 14.0 CURRENT versions. There is same problem with all versions.It seems to me no work has been done to fix pf divert. By the way i am currently using both IPFW and PF at the same time, i use IPFW for DIVERT but i am trying to move on FreeBSD 14.0 to work with only PF . But this DIVERT is not working on FreeBSD 14.0-CURRENT pf. So i couldn't give up IPFW's DIVERT. I have atteched a code above the attachment and i have tried all available codes on the internet. LAN =igb1 pass in quick on igb1 proto udp from any to port { 53 } divert-to 127.0.0.1 port 3355 # I have found this rule (pass out quick on igb1 inet proto udp from any to port 53 flags S/SA keep state divert-reply) from google but i got this error: /etc/pf.conf:83: divert-reply has no meaning in FreeBSD pf(4) pfctl: Syntax error in config file: pf rules not loaded FreeBSD 14.0-CURRENT pf.conf(5) man page divert-to <host> port <port> Used to redirect packets to a local socket bound to host and port. The packets will not be modified, so getsockname(2) on the socket will return the original destination address of the packet. divert-reply Used to receive replies for sockets that are bound to addresses which are not local to the machine. See setsockopt(2) for informa- tion on how to bind these sockets.
(In reply to Damjan Jovanovic from comment #0) It's fixed in CURRENT, see https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=272770. Please, give it a try and let us know if the issue persists.
https://download.freebsd.org/snapshots/ISO-IMAGES/16.0/FreeBSD-16.0-CURRENT-amd64-20251110-dbb34d496708-281771-disc1.iso.xz # uname -v FreeBSD 16.0-CURRENT main-n281796-e1c6f4cb9bd2 GENERIC Running the above supplied divert-sockets.c test program and the following rule: pass in on em0 proto tcp from any to port 7 divert-to 0.0.0.0 port 2000 And the IPv4 echo service enabled in inetd.conf, telnet'ing to port 7 panics the kernel in pf_test at: https://github.com/freebsd/freebsd-src/blob/e1c6f4cb9bd29358c2b2fe249af9a2f9626b0670/sys/netpfil/pf/pf.c#L11080 r = s->rule; With NULL in s. (the following else handles s == NULL). I have a vmcore file, and can provide more details if needed.
The panic occurs on the initial (bare) SYN: (kgdb) print pd $2 = { lookup = { done = 0, uid = 0, gid = 0 }, tot_len = 60, hdr = { tcp = { th_sport = 3251, th_dport = 1792, th_seq = 4265330243, th_ack = 0, th_x2 = 0 '\000', th_off = 10 '\n', th_flags = 2 '\002', th_win = 61690, th_sum = 54814, th_urp = 0 }, .....
Created attachment 265405 [details] Diff to fix NULL pointer crash
It's great that you had time to track it down to the root cause. From a glance, it seems to be unrelated to the original question here. If you have the same vision then I think you better open a separate report.
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=66f2f1c83247f05a3a599d7e88c7e7efbedd16b5 commit 66f2f1c83247f05a3a599d7e88c7e7efbedd16b5 Author: Kristof Provost <kp@FreeBSD.org> AuthorDate: 2025-11-15 13:44:54 +0000 Commit: Kristof Provost <kp@FreeBSD.org> CommitDate: 2025-11-15 21:38:21 +0000 pf: handle divert packets In a divert setup pf_test_state() may return PF_PASS, but not set the state pointer. We didn't handle that, and as a result crashed immediately afterwards trying to dereference that NULL state pointer. Add a test case to provoke the problem. PR: 260867 MFC after: 2 weeks Submitted by: Phil Budne <phil.budne@gmail.com> Sponsored by: Rubicon Communications, LLC ("Netgate") sys/netpfil/pf/pf.c | 20 ++++++++++-------- tests/sys/netpfil/pf/divert-to.sh | 43 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 8 deletions(-)
Since the original issue was open, and the panic was reproduced by the supplied test program, I posted on the existing issue, rather than create another (I figured the original issue could be retitled), but if standard operating procedure is to open new issues and ask questions later, I'll try and do that in the future! Two notes: My understanding of the intended interactions with the handling of initial SYN packets (ie; SYN cookies) and pfil states is slim to none.. My patch suppresses the panic, but I cannot vouch for its correctness, nor guess whether other protocols (SCTP?) might have similar issues! The work was done for hire, porting changes for product based on pfSense 2.5.2. I've yet to hear if my client would like the patch (if accepted) to be credited to (sponsored by?) them.
(In reply to Phil Budne from comment #10) No worries at all. From my perspective, the intention was to help you bring more attention to your patch, because commenting on old bug reports might receive fewer views. Regarding the concern whether it is correct, I think you have got a successful code review as long as it is already committed by Kristof. You might want to examine the actual commit landed to see how the idea was extended to cover another place in the code, and how it was covered with an automated test (if you are interested in testing).
A commit in branch stable/15 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=7d8effcf65fe598417924e0b314570b893b626c0 commit 7d8effcf65fe598417924e0b314570b893b626c0 Author: Kristof Provost <kp@FreeBSD.org> AuthorDate: 2025-11-15 13:44:54 +0000 Commit: Kristof Provost <kp@FreeBSD.org> CommitDate: 2025-11-30 10:27:27 +0000 pf: handle divert packets In a divert setup pf_test_state() may return PF_PASS, but not set the state pointer. We didn't handle that, and as a result crashed immediately afterwards trying to dereference that NULL state pointer. Add a test case to provoke the problem. PR: 260867 MFC after: 2 weeks Submitted by: Phil Budne <phil.budne@gmail.com> Sponsored by: Rubicon Communications, LLC ("Netgate") (cherry picked from commit 66f2f1c83247f05a3a599d7e88c7e7efbedd16b5) sys/netpfil/pf/pf.c | 20 ++++++++++-------- tests/sys/netpfil/pf/divert-to.sh | 43 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 8 deletions(-)
A commit in branch stable/14 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=a009793a5e5fad0b42cb0a158dcbccd5eff40d9f commit a009793a5e5fad0b42cb0a158dcbccd5eff40d9f Author: Kristof Provost <kp@FreeBSD.org> AuthorDate: 2025-11-15 13:44:54 +0000 Commit: Kristof Provost <kp@FreeBSD.org> CommitDate: 2025-11-30 10:26:28 +0000 pf: handle divert packets In a divert setup pf_test_state() may return PF_PASS, but not set the state pointer. We didn't handle that, and as a result crashed immediately afterwards trying to dereference that NULL state pointer. Add a test case to provoke the problem. PR: 260867 MFC after: 2 weeks Submitted by: Phil Budne <phil.budne@gmail.com> Sponsored by: Rubicon Communications, LLC ("Netgate") (cherry picked from commit 66f2f1c83247f05a3a599d7e88c7e7efbedd16b5) sys/netpfil/pf/pf.c | 20 +++++++++++------- tests/sys/netpfil/pf/divert-to.sh | 44 +++++++++++++++++++++++++++++++++++++-- 2 files changed, 54 insertions(+), 10 deletions(-)
A commit in branch stable/13 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=81385f622037a5b78fd4f8046163367fa607d37a commit 81385f622037a5b78fd4f8046163367fa607d37a Author: Kristof Provost <kp@FreeBSD.org> AuthorDate: 2025-11-15 13:44:54 +0000 Commit: Kristof Provost <kp@FreeBSD.org> CommitDate: 2025-11-29 20:02:00 +0000 pf: handle divert packets In a divert setup pf_test_state() may return PF_PASS, but not set the state pointer. We didn't handle that, and as a result crashed immediately afterwards trying to dereference that NULL state pointer. Add a test case to provoke the problem. PR: 260867 MFC after: 2 weeks Submitted by: Phil Budne <phil.budne@gmail.com> Sponsored by: Rubicon Communications, LLC ("Netgate") (cherry picked from commit 66f2f1c83247f05a3a599d7e88c7e7efbedd16b5) sys/netpfil/pf/pf.c | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-)