Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 25 Sep 2024 12:34:28 GMT
From:      Kristof Provost <kp@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org
Subject:   git: 905db4aa8877 - main - pf: dedupe layer 4 protocol code in pf_setup_pdesc()
Message-ID:  <202409251234.48PCYSaM071683@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch main has been updated by kp:

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

commit 905db4aa88775865097714c170f4503da385747c
Author:     Kristof Provost <kp@FreeBSD.org>
AuthorDate: 2024-09-10 18:17:13 +0000
Commit:     Kristof Provost <kp@FreeBSD.org>
CommitDate: 2024-09-25 10:44:30 +0000

    pf: dedupe layer 4 protocol code in pf_setup_pdesc()
    
    In pf_setup_pdesc() the code for analysing TCP and UDP headers was
    the same for v4 and v6.  Deduplicate by moving the protocol switch
    after the address family switch.
    ok henning@ claudio@
    
    Obtained from:  OpenBSD, bluhm <bluhm@openbsd.org>, 72cf18cc6e
    Sponsored by:   Rubicon Communications, LLC ("Netgate")
    Differential Revision:  https://reviews.freebsd.org/D46647
---
 sys/netpfil/pf/pf.c | 255 ++++++++++++++++++++--------------------------------
 1 file changed, 97 insertions(+), 158 deletions(-)

diff --git a/sys/netpfil/pf/pf.c b/sys/netpfil/pf/pf.c
index e65d848d7cc9..215f2655d9d4 100644
--- a/sys/netpfil/pf/pf.c
+++ b/sys/netpfil/pf/pf.c
@@ -8590,78 +8590,6 @@ pf_setup_pdesc(sa_family_t af, int dir, struct pf_pdesc *pd, struct mbuf *m,
 		if (h->ip_off & htons(IP_MF | IP_OFFMASK))
 			return (0);
 
-		switch (h->ip_p) {
-		case IPPROTO_TCP: {
-			struct tcphdr *th = &pd->hdr.tcp;
-
-			if (!pf_pull_hdr(m, *off, th, sizeof(*th), action,
-				reason, AF_INET)) {
-				*action = PF_DROP;
-				REASON_SET(reason, PFRES_SHORT);
-				return (-1);
-			}
-			*hdrlen = sizeof(*th);
-			pd->p_len = pd->tot_len - *off - (th->th_off << 2);
-			pd->sport = &th->th_sport;
-			pd->dport = &th->th_dport;
-			break;
-		}
-		case IPPROTO_UDP: {
-			struct udphdr *uh = &pd->hdr.udp;
-
-			if (!pf_pull_hdr(m, *off, uh, sizeof(*uh), action,
-				reason, AF_INET)) {
-				*action = PF_DROP;
-				REASON_SET(reason, PFRES_SHORT);
-				return (-1);
-			}
-			*hdrlen = sizeof(*uh);
-			if (uh->uh_dport == 0 ||
-			    ntohs(uh->uh_ulen) > m->m_pkthdr.len - *off ||
-			    ntohs(uh->uh_ulen) < sizeof(struct udphdr)) {
-				*action = PF_DROP;
-				REASON_SET(reason, PFRES_SHORT);
-				return (-1);
-			}
-			pd->sport = &uh->uh_sport;
-			pd->dport = &uh->uh_dport;
-			break;
-		}
-		case IPPROTO_SCTP: {
-			if (!pf_pull_hdr(m, *off, &pd->hdr.sctp, sizeof(pd->hdr.sctp),
-			    action, reason, AF_INET)) {
-				*action = PF_DROP;
-				REASON_SET(reason, PFRES_SHORT);
-				return (-1);
-			}
-			*hdrlen = sizeof(pd->hdr.sctp);
-			pd->p_len = pd->tot_len - *off;
-
-			pd->sport = &pd->hdr.sctp.src_port;
-			pd->dport = &pd->hdr.sctp.dest_port;
-			if (pd->hdr.sctp.src_port == 0 || pd->hdr.sctp.dest_port == 0) {
-				*action = PF_DROP;
-				REASON_SET(reason, PFRES_SHORT);
-				return (-1);
-			}
-			if (pf_scan_sctp(m, *off, pd, kif) != PF_PASS) {
-				*action = PF_DROP;
-				REASON_SET(reason, PFRES_SHORT);
-				return (-1);
-			}
-			break;
-		}
-		case IPPROTO_ICMP: {
-			if (!pf_pull_hdr(m, *off, &pd->hdr.icmp, ICMP_MINLEN,
-				action, reason, AF_INET)) {
-				*action = PF_DROP;
-				REASON_SET(reason, PFRES_SHORT);
-				return (-1);
-			}
-			*hdrlen = ICMP_MINLEN;
-			break;
-		}
-		}
 		break;
 	}
 #endif
@@ -8750,103 +8678,114 @@ pf_setup_pdesc(sa_family_t af, int dir, struct pf_pdesc *pd, struct mbuf *m,
 			}
 		} while (!terminal);
 
-		switch (pd->proto) {
-		case IPPROTO_TCP: {
-			struct tcphdr *th = &pd->hdr.tcp;
+		break;
+	}
+#endif
+	default:
+		panic("pf_setup_pdesc called with illegal af %u", af);
+	}
 
-			if (!pf_pull_hdr(m, *off, th, sizeof(*th), action,
-				reason, AF_INET6)) {
-				*action = PF_DROP;
-				REASON_SET(reason, PFRES_SHORT);
-				return (-1);
-			}
-			*hdrlen = sizeof(*th);
-			pd->p_len = pd->tot_len - *off - (th->th_off << 2);
-			pd->sport = &th->th_sport;
-			pd->dport = &th->th_dport;
-			break;
+	switch (pd->proto) {
+	case IPPROTO_TCP: {
+		struct tcphdr *th = &pd->hdr.tcp;
+
+		if (!pf_pull_hdr(m, *off, th, sizeof(*th), action,
+			reason, af)) {
+			*action = PF_DROP;
+			REASON_SET(reason, PFRES_SHORT);
+			return (-1);
 		}
-		case IPPROTO_UDP: {
-			struct udphdr *uh = &pd->hdr.udp;
+		*hdrlen = sizeof(*th);
+		pd->p_len = pd->tot_len - *off - (th->th_off << 2);
+		pd->sport = &th->th_sport;
+		pd->dport = &th->th_dport;
+		break;
+	}
+	case IPPROTO_UDP: {
+		struct udphdr *uh = &pd->hdr.udp;
 
-			if (!pf_pull_hdr(m, *off, uh, sizeof(*uh), action,
-				reason, AF_INET6)) {
-				*action = PF_DROP;
-				REASON_SET(reason, PFRES_SHORT);
-				return (-1);
-			}
-			*hdrlen = sizeof(*uh);
-			if (uh->uh_dport == 0 ||
-			    ntohs(uh->uh_ulen) > m->m_pkthdr.len - *off ||
-			    ntohs(uh->uh_ulen) < sizeof(struct udphdr)) {
-				*action = PF_DROP;
-				REASON_SET(reason, PFRES_SHORT);
-				return (-1);
-			}
-			pd->sport = &uh->uh_sport;
-			pd->dport = &uh->uh_dport;
-			break;
+		if (!pf_pull_hdr(m, *off, uh, sizeof(*uh), action,
+			reason, af)) {
+			*action = PF_DROP;
+			REASON_SET(reason, PFRES_SHORT);
+			return (-1);
 		}
-		case IPPROTO_SCTP: {
-			if (!pf_pull_hdr(m, *off, &pd->hdr.sctp, sizeof(pd->hdr.sctp),
-			    action, reason, AF_INET6)) {
-				*action = PF_DROP;
-				REASON_SET(reason, PFRES_SHORT);
-				return (-1);
-			}
-			*hdrlen = sizeof(pd->hdr.sctp);
-			pd->p_len = pd->tot_len - *off;
-
-			pd->sport = &pd->hdr.sctp.src_port;
-			pd->dport = &pd->hdr.sctp.dest_port;
-			if (pd->hdr.sctp.src_port == 0 || pd->hdr.sctp.dest_port == 0) {
-				*action = PF_DROP;
-				REASON_SET(reason, PFRES_SHORT);
-				return (-1);
-			}
-			if (pf_scan_sctp(m, *off, pd, kif) != PF_PASS) {
-				*action = PF_DROP;
-				REASON_SET(reason, PFRES_SHORT);
-				return (-1);
-			}
-			break;
+		*hdrlen = sizeof(*uh);
+		if (uh->uh_dport == 0 ||
+		    ntohs(uh->uh_ulen) > m->m_pkthdr.len - *off ||
+		    ntohs(uh->uh_ulen) < sizeof(struct udphdr)) {
+			*action = PF_DROP;
+			REASON_SET(reason, PFRES_SHORT);
+			return (-1);
 		}
-		case IPPROTO_ICMPV6: {
-			size_t icmp_hlen = sizeof(struct icmp6_hdr);
+		pd->sport = &uh->uh_sport;
+		pd->dport = &uh->uh_dport;
+		break;
+	}
+	case IPPROTO_SCTP: {
+		if (!pf_pull_hdr(m, *off, &pd->hdr.sctp, sizeof(pd->hdr.sctp),
+		    action, reason, af)) {
+			*action = PF_DROP;
+			REASON_SET(reason, PFRES_SHORT);
+			return (-1);
+		}
+		*hdrlen = sizeof(pd->hdr.sctp);
+		pd->p_len = pd->tot_len - *off;
 
-			if (!pf_pull_hdr(m, *off, &pd->hdr.icmp6, icmp_hlen,
-				action, reason, AF_INET6)) {
-				*action = PF_DROP;
-				REASON_SET(reason, PFRES_SHORT);
-				return (-1);
-			}
-			/* ICMP headers we look further into to match state */
-			switch (pd->hdr.icmp6.icmp6_type) {
-			case MLD_LISTENER_QUERY:
-			case MLD_LISTENER_REPORT:
-				icmp_hlen = sizeof(struct mld_hdr);
-				break;
-			case ND_NEIGHBOR_SOLICIT:
-			case ND_NEIGHBOR_ADVERT:
-				icmp_hlen = sizeof(struct nd_neighbor_solicit);
-				break;
-			}
-			if (icmp_hlen > sizeof(struct icmp6_hdr) &&
-			    !pf_pull_hdr(m, *off, &pd->hdr.icmp6, icmp_hlen,
-				action, reason, AF_INET6)) {
-				*action = PF_DROP;
-				REASON_SET(reason, PFRES_SHORT);
-				return (-1);
-			}
-			*hdrlen = icmp_hlen;
+		pd->sport = &pd->hdr.sctp.src_port;
+		pd->dport = &pd->hdr.sctp.dest_port;
+		if (pd->hdr.sctp.src_port == 0 || pd->hdr.sctp.dest_port == 0) {
+			*action = PF_DROP;
+			REASON_SET(reason, PFRES_SHORT);
+			return (-1);
+		}
+		if (pf_scan_sctp(m, *off, pd, kif) != PF_PASS) {
+			*action = PF_DROP;
+			REASON_SET(reason, PFRES_SHORT);
+			return (-1);
+		}
+		break;
+	}
+	case IPPROTO_ICMP: {
+		if (!pf_pull_hdr(m, *off, &pd->hdr.icmp, ICMP_MINLEN,
+			action, reason, af)) {
+			*action = PF_DROP;
+			REASON_SET(reason, PFRES_SHORT);
+			return (-1);
+		}
+		*hdrlen = ICMP_MINLEN;
+		break;
+	}
+	case IPPROTO_ICMPV6: {
+		size_t icmp_hlen = sizeof(struct icmp6_hdr);
+
+		if (!pf_pull_hdr(m, *off, &pd->hdr.icmp6, icmp_hlen,
+			action, reason, af)) {
+			*action = PF_DROP;
+			REASON_SET(reason, PFRES_SHORT);
+			return (-1);
+		}
+		/* ICMP headers we look further into to match state */
+		switch (pd->hdr.icmp6.icmp6_type) {
+		case MLD_LISTENER_QUERY:
+		case MLD_LISTENER_REPORT:
+			icmp_hlen = sizeof(struct mld_hdr);
+			break;
+		case ND_NEIGHBOR_SOLICIT:
+		case ND_NEIGHBOR_ADVERT:
+			icmp_hlen = sizeof(struct nd_neighbor_solicit);
 			break;
 		}
+		if (icmp_hlen > sizeof(struct icmp6_hdr) &&
+		    !pf_pull_hdr(m, *off, &pd->hdr.icmp6, icmp_hlen,
+			action, reason, af)) {
+			*action = PF_DROP;
+			REASON_SET(reason, PFRES_SHORT);
+			return (-1);
 		}
+		*hdrlen = icmp_hlen;
 		break;
 	}
-#endif
-	default:
-		panic("pf_setup_pdesc called with illegal af %u", af);
 	}
 	return (0);
 }



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202409251234.48PCYSaM071683>