git: bdd47177528b - main - pf: release rules lock before passing the packet to dummynet

The branch main has been updated by kp:

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

commit bdd47177528b5beacabb4837bfac0e9de92aae74
Author:     Kristof Provost <kp@FreeBSD.org>
AuthorDate: 2023-05-11 16:10:33 +0000
Commit:     Kristof Provost <kp@FreeBSD.org>
CommitDate: 2023-05-17 13:20:18 +0000

    pf: release rules lock before passing the packet to dummynet
    
    In the Ethernet rules we held the PF_RULES lock while we called
    ip_dn_io_ptr() (i.e. dummynet). That meant that we could end up back in
    pf while still holding the PF_RULES lock.
    That's not immediately fatal, because that lock is recursive, but still
    not ideal.
    
    There also appear to be scenarios where this can actually trigger
    deadlocks.
    
    We don't need to hold the PF_RULES lock, as long as we make a local copy
    of the data we need from the rule (in this case, the action and
    bridge_to target). It's safe to keep the struct ifnet pointer around,
    because we remain in NET_EPOCH.
    
    See also:       https://redmine.pfsense.org/issues/14373
    MFC after:      1 week
    Reviewed by:    mjg
    Sponsored by:   Rubicon Communications, LLC ("Netgate")
    Differential Revision:  https://reviews.freebsd.org/D40067
---
 sys/netpfil/pf/pf.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/sys/netpfil/pf/pf.c b/sys/netpfil/pf/pf.c
index 1424099b1e5a..06270d34da85 100644
--- a/sys/netpfil/pf/pf.c
+++ b/sys/netpfil/pf/pf.c
@@ -3875,10 +3875,8 @@ pf_match_eth_tag(struct mbuf *m, struct pf_keth_rule *r, int *tag, int mtag)
 }
 
 static void
-pf_bridge_to(struct pfi_kkif *kif, struct mbuf *m)
+pf_bridge_to(struct ifnet *ifp, struct mbuf *m)
 {
-	struct ifnet *ifp = kif->pfik_ifp;
-
 	/* If we don't have the interface drop the packet. */
 	if (ifp == NULL) {
 		m_freem(m);
@@ -3897,7 +3895,7 @@ pf_bridge_to(struct pfi_kkif *kif, struct mbuf *m)
 		return;
 	}
 
-	kif->pfik_ifp->if_transmit(kif->pfik_ifp, m);
+	ifp->if_transmit(ifp, m);
 }
 
 static int
@@ -3916,6 +3914,7 @@ pf_test_eth_rule(int dir, struct pfi_kkif *kif, struct mbuf **m0)
 	struct pf_mtag *mtag;
 	struct pf_keth_ruleq *rules;
 	struct pf_addr *src = NULL, *dst = NULL;
+	struct pfi_kkif *bridge_to;
 	sa_family_t af = 0;
 	uint16_t proto;
 	int asd = 0, match = 0;
@@ -4097,6 +4096,9 @@ pf_test_eth_rule(int dir, struct pfi_kkif *kif, struct mbuf **m0)
 		mtag->qid = r->qid;
 	}
 
+	action = r->action;
+	bridge_to = r->bridge_to;
+
 	/* Dummynet */
 	if (r->dnpipe) {
 		struct ip_fw_args dnflow;
@@ -4151,21 +4153,21 @@ pf_test_eth_rule(int dir, struct pfi_kkif *kif, struct mbuf **m0)
 			break;
 		}
 
+		PF_RULES_RUNLOCK();
+
 		mtag->flags |= PF_TAG_DUMMYNET;
 		ip_dn_io_ptr(m0, &dnflow);
 		if (*m0 != NULL)
 			mtag->flags &= ~PF_TAG_DUMMYNET;
+	} else {
+		PF_RULES_RUNLOCK();
 	}
 
-	action = r->action;
-
-	if (action == PF_PASS && r->bridge_to) {
-		pf_bridge_to(r->bridge_to, *m0);
+	if (action == PF_PASS && bridge_to) {
+		pf_bridge_to(bridge_to->pfik_ifp, *m0);
 		*m0 = NULL; /* We've eaten the packet. */
 	}
 
-	PF_RULES_RUNLOCK();
-
 	return (action);
 }