git: cfdc4f6d0647 - main - pf: g/c unneeded af (address family) params to pf_change_ap

From: Kristof Provost <kp_at_FreeBSD.org>
Date: Wed, 16 Apr 2025 18:02:49 UTC
The branch main has been updated by kp:

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

commit cfdc4f6d06473bef750cf089ae79ec5be7447c43
Author:     Kristof Provost <kp@FreeBSD.org>
AuthorDate: 2025-04-14 15:28:03 +0000
Commit:     Kristof Provost <kp@FreeBSD.org>
CommitDate: 2025-04-16 14:23:47 +0000

    pf: g/c unneeded af (address family) params to pf_change_ap
    
    both af and naf (af-to case) are in the pf_pdesc
    some code shuffling to actually set these before calling pf_change_ap
    inspired by Richard Procter <richard.n.procter@gmail.com>'s mail on tech
    from Aug 17, but redone
    ok bluhm vgross
    
    Obtained from:  OpenBSD, henning <henning@openbsd.org>, 78ad05cbd1
    Sponsored by:   Rubicon Communications, LLC ("Netgate")
---
 sys/netpfil/pf/pf.c | 125 +++++++++++++++++++++++++---------------------------
 1 file changed, 61 insertions(+), 64 deletions(-)

diff --git a/sys/netpfil/pf/pf.c b/sys/netpfil/pf/pf.c
index 9c41bf80fec4..d4288ba34eb4 100644
--- a/sys/netpfil/pf/pf.c
+++ b/sys/netpfil/pf/pf.c
@@ -310,7 +310,7 @@ static int		 pf_check_threshold(struct pf_threshold *);
 
 static void		 pf_change_ap(struct pf_pdesc *, struct pf_addr *, u_int16_t *,
 			    u_int16_t *, u_int16_t *, struct pf_addr *,
-			    u_int16_t, u_int8_t, sa_family_t, sa_family_t);
+			    u_int16_t, u_int8_t);
 static int		 pf_modulate_sack(struct pf_pdesc *,
 			    struct tcphdr *, struct pf_state_peer *);
 int			 pf_icmp_mapping(struct pf_pdesc *, u_int8_t, int *,
@@ -634,11 +634,11 @@ pf_packet_rework_nat(struct pf_pdesc *pd, int off, struct pf_state_key *nk)
 		if (PF_ANEQ(pd->src, &nk->addr[pd->sidx], pd->af))
 			pf_change_ap(pd, pd->src, &th->th_sport, pd->ip_sum,
 			    &th->th_sum, &nk->addr[pd->sidx],
-			    nk->port[pd->sidx], 0, pd->af, pd->naf);
+			    nk->port[pd->sidx], 0);
 		if (PF_ANEQ(pd->dst, &nk->addr[pd->didx], pd->af))
 			pf_change_ap(pd, pd->dst, &th->th_dport, pd->ip_sum,
 			    &th->th_sum, &nk->addr[pd->didx],
-			    nk->port[pd->didx], 0, pd->af, pd->naf);
+			    nk->port[pd->didx], 0);
 		m_copyback(pd->m, off, sizeof(*th), (caddr_t)th);
 		break;
 	}
@@ -648,11 +648,11 @@ pf_packet_rework_nat(struct pf_pdesc *pd, int off, struct pf_state_key *nk)
 		if (PF_ANEQ(pd->src, &nk->addr[pd->sidx], pd->af))
 			pf_change_ap(pd, pd->src, &uh->uh_sport, pd->ip_sum,
 			    &uh->uh_sum, &nk->addr[pd->sidx],
-			    nk->port[pd->sidx], 1, pd->af, pd->naf);
+			    nk->port[pd->sidx], 1);
 		if (PF_ANEQ(pd->dst, &nk->addr[pd->didx], pd->af))
 			pf_change_ap(pd, pd->dst, &uh->uh_dport, pd->ip_sum,
 			    &uh->uh_sum, &nk->addr[pd->didx],
-			    nk->port[pd->didx], 1, pd->af, pd->naf);
+			    nk->port[pd->didx], 1);
 		m_copyback(pd->m, off, sizeof(*uh), (caddr_t)uh);
 		break;
 	}
@@ -663,12 +663,12 @@ pf_packet_rework_nat(struct pf_pdesc *pd, int off, struct pf_state_key *nk)
 		if (PF_ANEQ(pd->src, &nk->addr[pd->sidx], pd->af)) {
 			pf_change_ap(pd, pd->src, &sh->src_port, pd->ip_sum,
 			    &checksum, &nk->addr[pd->sidx],
-			    nk->port[pd->sidx], 1, pd->af, pd->naf);
+			    nk->port[pd->sidx], 1);
 		}
 		if (PF_ANEQ(pd->dst, &nk->addr[pd->didx], pd->af)) {
 			pf_change_ap(pd, pd->dst, &sh->dest_port, pd->ip_sum,
 			    &checksum, &nk->addr[pd->didx],
-			    nk->port[pd->didx], 1, pd->af, pd->naf);
+			    nk->port[pd->didx], 1);
 		}
 
 		break;
@@ -3262,15 +3262,14 @@ pf_proto_cksum_fixup(struct mbuf *m, u_int16_t cksum, u_int16_t old,
 
 static void
 pf_change_ap(struct pf_pdesc *pd, struct pf_addr *a, u_int16_t *p, u_int16_t *ic,
-        u_int16_t *pc, struct pf_addr *an, u_int16_t pn, u_int8_t u,
-        sa_family_t af, sa_family_t naf)
+        u_int16_t *pc, struct pf_addr *an, u_int16_t pn, u_int8_t u)
 {
 	struct pf_addr	ao;
 	u_int16_t	po;
 
-	PF_ACPY(&ao, a, af);
-	if (af == naf)
-		PF_ACPY(a, an, af);
+	PF_ACPY(&ao, a, pd->af);
+	if (pd->af == pd->naf)
+		PF_ACPY(a, an, pd->af);
 
 	if (pd->m->m_pkthdr.csum_flags & (CSUM_DELAY_DATA | CSUM_DELAY_DATA_IPV6))
 		*pc = ~*pc;
@@ -3280,10 +3279,10 @@ pf_change_ap(struct pf_pdesc *pd, struct pf_addr *a, u_int16_t *p, u_int16_t *ic
 	po = *p;
 	*p = pn;
 
-	switch (af) {
+	switch (pd->af) {
 #ifdef INET
 	case AF_INET:
-		switch (naf) {
+		switch (pd->naf) {
 		case AF_INET:
 			*ic = pf_cksum_fixup(pf_cksum_fixup(*ic,
 			    ao.addr16[0], an->addr16[0], 0),
@@ -3319,7 +3318,7 @@ pf_change_ap(struct pf_pdesc *pd, struct pf_addr *a, u_int16_t *p, u_int16_t *ic
 #endif /* INET */
 #ifdef INET6
 	case AF_INET6:
-		switch (naf) {
+		switch (pd->naf) {
 #ifdef INET
 		case AF_INET:
 			*pc = pf_cksum_fixup(pf_cksum_fixup(pf_cksum_fixup(
@@ -3357,7 +3356,7 @@ pf_change_ap(struct pf_pdesc *pd, struct pf_addr *a, u_int16_t *p, u_int16_t *ic
 		break;
 #endif /* INET6 */
 	default:
-		unhandled_af(af);
+		unhandled_af(pd->af);
 	}
 
 	if (pd->m->m_pkthdr.csum_flags & (CSUM_DELAY_DATA |
@@ -5600,7 +5599,7 @@ pf_test_rule(struct pf_krule **rm, struct pf_kstate **sm,
 			    nk->port[pd->sidx] != pd->nsport) {
 				pf_change_ap(pd, pd->src, &th->th_sport,
 				    pd->ip_sum, &th->th_sum, &nk->addr[pd->sidx],
-				    nk->port[pd->sidx], 0, pd->af, pd->naf);
+				    nk->port[pd->sidx], 0);
 				pd->sport = &th->th_sport;
 				pd->nsport = th->th_sport;
 				PF_ACPY(&pd->nsaddr, pd->src, pd->af);
@@ -5610,7 +5609,7 @@ pf_test_rule(struct pf_krule **rm, struct pf_kstate **sm,
 			    nk->port[pd->didx] != pd->ndport) {
 				pf_change_ap(pd, pd->dst, &th->th_dport,
 				    pd->ip_sum, &th->th_sum, &nk->addr[pd->didx],
-				    nk->port[pd->didx], 0, pd->af, pd->naf);
+				    nk->port[pd->didx], 0);
 				pd->dport = &th->th_dport;
 				pd->ndport = th->th_dport;
 				PF_ACPY(&pd->ndaddr, pd->dst, pd->af);
@@ -5626,7 +5625,7 @@ pf_test_rule(struct pf_krule **rm, struct pf_kstate **sm,
 				    &pd->hdr.udp.uh_sport,
 				    pd->ip_sum, &pd->hdr.udp.uh_sum,
 				    &nk->addr[pd->sidx],
-				    nk->port[pd->sidx], 1, pd->af, pd->naf);
+				    nk->port[pd->sidx], 1);
 				pd->sport = &pd->hdr.udp.uh_sport;
 				pd->nsport = pd->hdr.udp.uh_sport;
 				PF_ACPY(&pd->nsaddr, pd->src, pd->af);
@@ -5638,7 +5637,7 @@ pf_test_rule(struct pf_krule **rm, struct pf_kstate **sm,
 				    &pd->hdr.udp.uh_dport,
 				    pd->ip_sum, &pd->hdr.udp.uh_sum,
 				    &nk->addr[pd->didx],
-				    nk->port[pd->didx], 1, pd->af, pd->naf);
+				    nk->port[pd->didx], 1);
 				pd->dport = &pd->hdr.udp.uh_dport;
 				pd->ndport = pd->hdr.udp.uh_dport;
 				PF_ACPY(&pd->ndaddr, pd->dst, pd->af);
@@ -5653,7 +5652,7 @@ pf_test_rule(struct pf_krule **rm, struct pf_kstate **sm,
 				pf_change_ap(pd, pd->src,
 				    &pd->hdr.sctp.src_port, pd->ip_sum, &checksum,
 				    &nk->addr[pd->sidx],
-				    nk->port[pd->sidx], 1, pd->af, pd->naf);
+				    nk->port[pd->sidx], 1);
 				pd->sport = &pd->hdr.sctp.src_port;
 				pd->nsport = pd->hdr.sctp.src_port;
 				PF_ACPY(&pd->nsaddr, pd->src, pd->af);
@@ -5663,7 +5662,7 @@ pf_test_rule(struct pf_krule **rm, struct pf_kstate **sm,
 				pf_change_ap(pd, pd->dst,
 				    &pd->hdr.sctp.dest_port, pd->ip_sum, &checksum,
 				    &nk->addr[pd->didx],
-				    nk->port[pd->didx], 1, pd->af, pd->naf);
+				    nk->port[pd->didx], 1);
 				pd->dport = &pd->hdr.sctp.dest_port;
 				pd->ndport = pd->hdr.sctp.dest_port;
 				PF_ACPY(&pd->ndaddr, pd->dst, pd->af);
@@ -6333,12 +6332,12 @@ pf_translate(struct pf_pdesc *pd, struct pf_addr *saddr, u_int16_t sport,
 	case IPPROTO_TCP:
 		if (afto || *pd->sport != sport) {
 			pf_change_ap(pd, pd->src, pd->sport, pd->ip_sum, &pd->hdr.tcp.th_sum,
-			    saddr, sport, 0, pd->af, pd->naf);
+			    saddr, sport, 0);
 			rewrite = 1;
 		}
 		if (afto || *pd->dport != dport) {
 			pf_change_ap(pd, pd->dst, pd->dport, pd->ip_sum, &pd->hdr.tcp.th_sum,
-			    daddr, dport, 0, pd->af, pd->naf);
+			    daddr, dport, 0);
 			rewrite = 1;
 		}
 		break;
@@ -6346,12 +6345,12 @@ pf_translate(struct pf_pdesc *pd, struct pf_addr *saddr, u_int16_t sport,
 	case IPPROTO_UDP:
 		if (afto || *pd->sport != sport) {
 			pf_change_ap(pd, pd->src, pd->sport, pd->ip_sum, &pd->hdr.udp.uh_sum,
-			    saddr, sport, 1, pd->af, pd->naf);
+			    saddr, sport, 1);
 			rewrite = 1;
 		}
 		if (afto || *pd->dport != dport) {
 			pf_change_ap(pd, pd->dst, pd->dport, pd->ip_sum, &pd->hdr.udp.uh_sum,
-			    daddr, dport, 1, pd->af, pd->naf);
+			    daddr, dport, 1);
 			rewrite = 1;
 		}
 		break;
@@ -6360,12 +6359,12 @@ pf_translate(struct pf_pdesc *pd, struct pf_addr *saddr, u_int16_t sport,
 		uint16_t checksum = 0;
 		if (afto || *pd->sport != sport) {
 			pf_change_ap(pd, pd->src, pd->sport, pd->ip_sum, &checksum,
-			    saddr, sport, 1, pd->af, pd->naf);
+			    saddr, sport, 1);
 			rewrite = 1;
 		}
 		if (afto || *pd->dport != dport) {
 			pf_change_ap(pd, pd->dst, pd->dport, pd->ip_sum, &checksum,
-			    daddr, dport, 1, pd->af, pd->naf);
+			    daddr, dport, 1);
 			rewrite = 1;
 		}
 		break;
@@ -7105,26 +7104,24 @@ pf_test_state(struct pf_kstate **state, struct pf_pdesc *pd, u_short *reason)
 			didx = pd->didx;
 		}
 
+		if (afto) {
+			PF_ACPY(&pd->nsaddr, &nk->addr[sidx], nk->af);
+			PF_ACPY(&pd->ndaddr, &nk->addr[didx], nk->af);
+			pd->naf = nk->af;
+			action = PF_AFRT;
+		}
+
 		if (afto || PF_ANEQ(pd->src, &nk->addr[sidx], pd->af) ||
 		    nk->port[sidx] != pd->osport)
 			pf_change_ap(pd, pd->src, pd->sport, pd->ip_sum,
 			    pd->pcksum, &nk->addr[sidx],
-			    nk->port[sidx], pd->virtual_proto == IPPROTO_UDP,
-			    pd->af, nk->af);
+			    nk->port[sidx], pd->virtual_proto == IPPROTO_UDP);
 
 		if (afto || PF_ANEQ(pd->dst, &nk->addr[didx], pd->af) ||
 		    nk->port[didx] != pd->odport)
 			pf_change_ap(pd, pd->dst, pd->dport, pd->ip_sum,
 			    pd->pcksum, &nk->addr[didx],
-			    nk->port[didx], pd->virtual_proto == IPPROTO_UDP,
-			    pd->af, nk->af);
-
-		if (afto) {
-			PF_ACPY(&pd->nsaddr, &nk->addr[sidx], nk->af);
-			PF_ACPY(&pd->ndaddr, &nk->addr[didx], nk->af);
-			pd->naf = nk->af;
-			action = PF_AFRT;
-		}
+			    nk->port[didx], pd->virtual_proto == IPPROTO_UDP);
 
 		copyback = 1;
 	}
@@ -8022,18 +8019,6 @@ pf_test_state_icmp(struct pf_kstate **state, struct pf_pdesc *pd,
 					m_copyback(pd->m, pd->off,
 					    sizeof(struct icmp6_hdr),
 					    (c_caddr_t)&pd->hdr.icmp6);
-					if (pf_change_icmp_af(pd->m, ipoff2, pd,
-					    &pd2, &nk->addr[sidx],
-					    &nk->addr[didx], pd->af,
-					    nk->af))
-						return (PF_DROP);
-					pf_change_ap(pd, pd2.src, &th.th_sport,
-					    pd->ip_sum, &dummy_cksum, &nk->addr[pd2.sidx],
-					    nk->port[sidx], 1, pd->af, nk->af);
-					pf_change_ap(pd, pd2.dst, &th.th_dport,
-					    pd->ip_sum, &dummy_cksum, &nk->addr[pd2.didx],
-					    nk->port[didx], 1, pd->af, nk->af);
-					m_copyback(pd2.m, pd2.off, 8, (c_caddr_t)&th);
 					PF_ACPY(&pd->nsaddr, &nk->addr[pd2.sidx],
 					    nk->af);
 					PF_ACPY(&pd->ndaddr,
@@ -8053,6 +8038,18 @@ pf_test_state_icmp(struct pf_kstate **state, struct pf_pdesc *pd,
 						    pd->src->addr32[0];
 					}
 					pd->naf = nk->af;
+					if (pf_change_icmp_af(pd->m, ipoff2, pd,
+					    &pd2, &nk->addr[sidx],
+					    &nk->addr[didx], pd->af,
+					    nk->af))
+						return (PF_DROP);
+					pf_change_ap(pd, pd2.src, &th.th_sport,
+					    pd->ip_sum, &dummy_cksum, &nk->addr[pd2.sidx],
+					    nk->port[sidx], 1);
+					pf_change_ap(pd, pd2.dst, &th.th_dport,
+					    pd->ip_sum, &dummy_cksum, &nk->addr[pd2.didx],
+					    nk->port[didx], 1);
+					m_copyback(pd2.m, pd2.off, 8, (c_caddr_t)&th);
 					return (PF_AFRT);
 				}
 #endif /* INET && INET6 */
@@ -8155,19 +8152,6 @@ pf_test_state_icmp(struct pf_kstate **state, struct pf_pdesc *pd,
 					m_copyback(pd->m, pd->off,
 					    sizeof(struct icmp6_hdr),
 					    (c_caddr_t)&pd->hdr.icmp6);
-					if (pf_change_icmp_af(pd->m, ipoff2, pd,
-					    &pd2, &nk->addr[sidx],
-					    &nk->addr[didx], pd->af,
-					    nk->af))
-						return (PF_DROP);
-					pf_change_ap(pd, pd2.src, &uh.uh_sport,
-					    pd->ip_sum, &uh.uh_sum, &nk->addr[pd2.sidx],
-					    nk->port[sidx], 1, pd->af, nk->af);
-					pf_change_ap(pd, pd2.dst, &uh.uh_dport,
-					    pd->ip_sum, &uh.uh_sum, &nk->addr[pd2.didx],
-					    nk->port[didx], 1, pd->af, nk->af);
-					m_copyback(pd2.m, pd2.off, sizeof(uh),
-					    (c_caddr_t)&uh);
 					PF_ACPY(&pd->nsaddr,
 					    &nk->addr[pd2.sidx], nk->af);
 					PF_ACPY(&pd->ndaddr,
@@ -8187,6 +8171,19 @@ pf_test_state_icmp(struct pf_kstate **state, struct pf_pdesc *pd,
 						    pd->src->addr32[0];
 					}
 					pd->naf = nk->af;
+					if (pf_change_icmp_af(pd->m, ipoff2, pd,
+					    &pd2, &nk->addr[sidx],
+					    &nk->addr[didx], pd->af,
+					    nk->af))
+						return (PF_DROP);
+					pf_change_ap(pd, pd2.src, &uh.uh_sport,
+					    pd->ip_sum, &uh.uh_sum, &nk->addr[pd2.sidx],
+					    nk->port[sidx], 1);
+					pf_change_ap(pd, pd2.dst, &uh.uh_dport,
+					    pd->ip_sum, &uh.uh_sum, &nk->addr[pd2.didx],
+					    nk->port[didx], 1);
+					m_copyback(pd2.m, pd2.off, sizeof(uh),
+					    (c_caddr_t)&uh);
 					return (PF_AFRT);
 				}
 #endif /* INET && INET6 */