Skip to content

Commit 5d210f3

Browse files
committed
pf: return PF_PASS/PF_DROP from pf_setup_pdesc()
We returned 'PF_DROP' instead of '-1' in one case, which would lead to us continuing the processing for an invalid packet. This also aligns us closer to OpenBSD, and reduces the odds of future similar mixups. MFC after: 3 days Sponsored by: Rubicon Communications, LLC ("Netgate")
1 parent dd2fc08 commit 5d210f3

File tree

1 file changed

+22
-22
lines changed

1 file changed

+22
-22
lines changed

sys/netpfil/pf/pf.c

Lines changed: 22 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -10446,28 +10446,28 @@ pf_setup_pdesc(sa_family_t af, int dir, struct pf_pdesc *pd, struct mbuf **m0,
1044610446
__func__);
1044710447
*action = PF_DROP;
1044810448
REASON_SET(reason, PFRES_SHORT);
10449-
return (-1);
10449+
return (PF_DROP);
1045010450
}
1045110451

1045210452
h = mtod(pd->m, struct ip *);
1045310453
if (pd->m->m_pkthdr.len < ntohs(h->ip_len)) {
1045410454
*action = PF_DROP;
1045510455
REASON_SET(reason, PFRES_SHORT);
10456-
return (-1);
10456+
return (PF_DROP);
1045710457
}
1045810458

1045910459
if (pf_normalize_ip(reason, pd) != PF_PASS) {
1046010460
/* We do IP header normalization and packet reassembly here */
1046110461
*m0 = pd->m;
1046210462
*action = PF_DROP;
10463-
return (-1);
10463+
return (PF_DROP);
1046410464
}
1046510465
*m0 = pd->m;
1046610466
h = mtod(pd->m, struct ip *);
1046710467

1046810468
if (pf_walk_header(pd, h, reason) != PF_PASS) {
1046910469
*action = PF_DROP;
10470-
return (-1);
10470+
return (PF_DROP);
1047110471
}
1047210472

1047310473
pd->src = (struct pf_addr *)&h->ip_src;
@@ -10497,15 +10497,15 @@ pf_setup_pdesc(sa_family_t af, int dir, struct pf_pdesc *pd, struct mbuf **m0,
1049710497
", pullup failed", __func__);
1049810498
*action = PF_DROP;
1049910499
REASON_SET(reason, PFRES_SHORT);
10500-
return (-1);
10500+
return (PF_DROP);
1050110501
}
1050210502

1050310503
h = mtod(pd->m, struct ip6_hdr *);
1050410504
if (pd->m->m_pkthdr.len <
1050510505
sizeof(struct ip6_hdr) + ntohs(h->ip6_plen)) {
1050610506
*action = PF_DROP;
1050710507
REASON_SET(reason, PFRES_SHORT);
10508-
return (-1);
10508+
return (PF_DROP);
1050910509
}
1051010510

1051110511
/*
@@ -10514,12 +10514,12 @@ pf_setup_pdesc(sa_family_t af, int dir, struct pf_pdesc *pd, struct mbuf **m0,
1051410514
*/
1051510515
if (htons(h->ip6_plen) == 0) {
1051610516
*action = PF_DROP;
10517-
return (-1);
10517+
return (PF_DROP);
1051810518
}
1051910519

1052010520
if (pf_walk_header6(pd, h, reason) != PF_PASS) {
1052110521
*action = PF_DROP;
10522-
return (-1);
10522+
return (PF_DROP);
1052310523
}
1052410524

1052510525
h = mtod(pd->m, struct ip6_hdr *);
@@ -10541,13 +10541,13 @@ pf_setup_pdesc(sa_family_t af, int dir, struct pf_pdesc *pd, struct mbuf **m0,
1054110541
PF_PASS) {
1054210542
*m0 = pd->m;
1054310543
*action = PF_DROP;
10544-
return (-1);
10544+
return (PF_DROP);
1054510545
}
1054610546
*m0 = pd->m;
1054710547
if (pd->m == NULL) {
1054810548
/* packet sits in reassembly queue, no error */
1054910549
*action = PF_PASS;
10550-
return (-1);
10550+
return (PF_DROP);
1055110551
}
1055210552

1055310553
/* Update pointers into the packet. */
@@ -10559,7 +10559,7 @@ pf_setup_pdesc(sa_family_t af, int dir, struct pf_pdesc *pd, struct mbuf **m0,
1055910559

1056010560
if (pf_walk_header6(pd, h, reason) != PF_PASS) {
1056110561
*action = PF_DROP;
10562-
return (-1);
10562+
return (PF_DROP);
1056310563
}
1056410564

1056510565
if (m_tag_find(pd->m, PACKET_TAG_PF_REASSEMBLED, NULL) != NULL) {
@@ -10589,7 +10589,7 @@ pf_setup_pdesc(sa_family_t af, int dir, struct pf_pdesc *pd, struct mbuf **m0,
1058910589
reason, af)) {
1059010590
*action = PF_DROP;
1059110591
REASON_SET(reason, PFRES_SHORT);
10592-
return (-1);
10592+
return (PF_DROP);
1059310593
}
1059410594
pd->hdrlen = sizeof(*th);
1059510595
pd->p_len = pd->tot_len - pd->off - (th->th_off << 2);
@@ -10605,15 +10605,15 @@ pf_setup_pdesc(sa_family_t af, int dir, struct pf_pdesc *pd, struct mbuf **m0,
1060510605
reason, af)) {
1060610606
*action = PF_DROP;
1060710607
REASON_SET(reason, PFRES_SHORT);
10608-
return (-1);
10608+
return (PF_DROP);
1060910609
}
1061010610
pd->hdrlen = sizeof(*uh);
1061110611
if (uh->uh_dport == 0 ||
1061210612
ntohs(uh->uh_ulen) > pd->m->m_pkthdr.len - pd->off ||
1061310613
ntohs(uh->uh_ulen) < sizeof(struct udphdr)) {
1061410614
*action = PF_DROP;
1061510615
REASON_SET(reason, PFRES_SHORT);
10616-
return (-1);
10616+
return (PF_DROP);
1061710617
}
1061810618
pd->sport = &uh->uh_sport;
1061910619
pd->dport = &uh->uh_dport;
@@ -10625,7 +10625,7 @@ pf_setup_pdesc(sa_family_t af, int dir, struct pf_pdesc *pd, struct mbuf **m0,
1062510625
reason, af)) {
1062610626
*action = PF_DROP;
1062710627
REASON_SET(reason, PFRES_SHORT);
10628-
return (-1);
10628+
return (PF_DROP);
1062910629
}
1063010630
pd->hdrlen = sizeof(pd->hdr.sctp);
1063110631
pd->p_len = pd->tot_len - pd->off;
@@ -10635,7 +10635,7 @@ pf_setup_pdesc(sa_family_t af, int dir, struct pf_pdesc *pd, struct mbuf **m0,
1063510635
if (pd->hdr.sctp.src_port == 0 || pd->hdr.sctp.dest_port == 0) {
1063610636
*action = PF_DROP;
1063710637
REASON_SET(reason, PFRES_SHORT);
10638-
return (-1);
10638+
return (PF_DROP);
1063910639
}
1064010640

1064110641
/*
@@ -10650,7 +10650,7 @@ pf_setup_pdesc(sa_family_t af, int dir, struct pf_pdesc *pd, struct mbuf **m0,
1065010650
if (pf_scan_sctp(pd) != PF_PASS) {
1065110651
*action = PF_DROP;
1065210652
REASON_SET(reason, PFRES_SHORT);
10653-
return (-1);
10653+
return (PF_DROP);
1065410654
}
1065510655
break;
1065610656
}
@@ -10659,7 +10659,7 @@ pf_setup_pdesc(sa_family_t af, int dir, struct pf_pdesc *pd, struct mbuf **m0,
1065910659
reason, af)) {
1066010660
*action = PF_DROP;
1066110661
REASON_SET(reason, PFRES_SHORT);
10662-
return (-1);
10662+
return (PF_DROP);
1066310663
}
1066410664
pd->pcksum = &pd->hdr.icmp.icmp_cksum;
1066510665
pd->hdrlen = ICMP_MINLEN;
@@ -10673,7 +10673,7 @@ pf_setup_pdesc(sa_family_t af, int dir, struct pf_pdesc *pd, struct mbuf **m0,
1067310673
reason, af)) {
1067410674
*action = PF_DROP;
1067510675
REASON_SET(reason, PFRES_SHORT);
10676-
return (-1);
10676+
return (PF_DROP);
1067710677
}
1067810678
/* ICMP headers we look further into to match state */
1067910679
switch (pd->hdr.icmp6.icmp6_type) {
@@ -10699,7 +10699,7 @@ pf_setup_pdesc(sa_family_t af, int dir, struct pf_pdesc *pd, struct mbuf **m0,
1069910699
reason, af)) {
1070010700
*action = PF_DROP;
1070110701
REASON_SET(reason, PFRES_SHORT);
10702-
return (-1);
10702+
return (PF_DROP);
1070310703
}
1070410704
pd->hdrlen = icmp_hlen;
1070510705
pd->pcksum = &pd->hdr.icmp6.icmp6_cksum;
@@ -10722,7 +10722,7 @@ pf_setup_pdesc(sa_family_t af, int dir, struct pf_pdesc *pd, struct mbuf **m0,
1072210722

1072310723
MPASS(pd->pcksum != NULL);
1072410724

10725-
return (0);
10725+
return (PF_PASS);
1072610726
}
1072710727

1072810728
static __inline void
@@ -10984,7 +10984,7 @@ pf_test(sa_family_t af, int dir, int pflags, struct ifnet *ifp, struct mbuf **m0
1098410984
PF_RULES_RLOCK();
1098510985

1098610986
if (pf_setup_pdesc(af, dir, &pd, m0, &action, &reason,
10987-
kif, default_actions) == -1) {
10987+
kif, default_actions) != PF_PASS) {
1098810988
if (action != PF_PASS)
1098910989
pd.act.log |= PF_LOG_FORCE;
1099010990
goto done;

0 commit comments

Comments
 (0)