Hi,
Instead of passing an extra mbuf pointer to pf_route(), it should
just use pd->m. Then pf_test() can also operate on pd.m and set
the *m0 value in the caller just before it returns. Note that the
goto done after ip6_output() fixes a mbuf leak.
ok?
bluhm
Index: net/if_pfsync.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/net/if_pfsync.c,v
retrieving revision 1.236
diff -u -p -r1.236 if_pfsync.c
--- net/if_pfsync.c 27 Oct 2016 21:41:20 -0000 1.236
+++ net/if_pfsync.c 28 Oct 2016 11:53:20 -0000
@@ -1754,16 +1754,17 @@ pfsync_undefer(struct pfsync_deferral *p
}
switch (pd->pd_st->key[PF_SK_WIRE]->af) {
case AF_INET:
- pf_route(&pd->pd_m, &pdesc,
+ pf_route(&pdesc,
pd->pd_st->rule.ptr, pd->pd_st);
break;
#ifdef INET6
case AF_INET6:
- pf_route6(&pd->pd_m, &pdesc,
+ pf_route6(&pdesc,
pd->pd_st->rule.ptr, pd->pd_st);
break;
#endif /* INET6 */
}
+ pd->pd_m = pdesc.m;
} else {
switch (pd->pd_st->key[PF_SK_WIRE]->af) {
case AF_INET:
Index: net/pf.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/net/pf.c,v
retrieving revision 1.996
diff -u -p -r1.996 pf.c
--- net/pf.c 28 Oct 2016 07:54:19 -0000 1.996
+++ net/pf.c 28 Oct 2016 11:57:27 -0000
@@ -2196,6 +2196,7 @@ pf_translate_a(struct pf_pdesc *pd, stru
}
#if INET6
+/* pf_translate_af() may change pd->m, adjust local copies after calling */
int
pf_translate_af(struct pf_pdesc *pd)
{
@@ -5775,9 +5776,9 @@ pf_rtlabel_match(struct pf_addr *addr, s
return (ret);
}
+/* pf_route() may change pd->m, adjust local copies after calling */
void
-pf_route(struct mbuf **m, struct pf_pdesc *pd, struct pf_rule *r,
- struct pf_state *s)
+pf_route(struct pf_pdesc *pd, struct pf_rule *r, struct pf_state *s)
{
struct mbuf *m0, *m1;
struct sockaddr_in *dst, sin;
@@ -5789,22 +5790,19 @@ pf_route(struct mbuf **m, struct pf_pdes
int error = 0;
unsigned int rtableid;
- if (m == NULL || *m == NULL || r == NULL)
- panic("pf_route: invalid parameters");
-
- if ((*m)->m_pkthdr.pf.routed++ > 3) {
- m0 = *m;
- *m = NULL;
- goto bad;
+ if (pd->m->m_pkthdr.pf.routed++ > 3) {
+ m_freem(pd->m);
+ pd->m = NULL;
+ return;
}
if (r->rt == PF_DUPTO) {
- if ((m0 = m_dup_pkt(*m, max_linkhdr, M_NOWAIT)) == NULL)
+ if ((m0 = m_dup_pkt(pd->m, max_linkhdr, M_NOWAIT)) == NULL)
return;
} else {
if ((r->rt == PF_REPLYTO) == (r->direction == pd->dir))
return;
- m0 = *m;
+ m0 = pd->m;
}
if (m0->m_len < sizeof(struct ip)) {
@@ -5929,7 +5927,7 @@ pf_route(struct mbuf **m, struct pf_pdes
done:
if (r->rt != PF_DUPTO)
- *m = NULL;
+ pd->m = NULL;
if (!r->rt)
if_put(ifp);
rtfree(rt);
@@ -5941,9 +5939,9 @@ bad:
}
#ifdef INET6
+/* pf_route6() may change pd->m, adjust local copies after calling */
void
-pf_route6(struct mbuf **m, struct pf_pdesc *pd, struct pf_rule *r,
- struct pf_state *s)
+pf_route6(struct pf_pdesc *pd, struct pf_rule *r, struct pf_state *s)
{
struct mbuf *m0;
struct sockaddr_in6 *dst, sin6;
@@ -5955,22 +5953,19 @@ pf_route6(struct mbuf **m, struct pf_pde
struct m_tag *mtag;
unsigned int rtableid;
- if (m == NULL || *m == NULL || r == NULL)
- panic("pf_route6: invalid parameters");
-
- if ((*m)->m_pkthdr.pf.routed++ > 3) {
- m0 = *m;
- *m = NULL;
- goto bad;
+ if (pd->m->m_pkthdr.pf.routed++ > 3) {
+ m_freem(pd->m);
+ pd->m = NULL;
+ return;
}
if (r->rt == PF_DUPTO) {
- if ((m0 = m_dup_pkt(*m, max_linkhdr, M_NOWAIT)) == NULL)
+ if ((m0 = m_dup_pkt(pd->m, max_linkhdr, M_NOWAIT)) == NULL)
return;
} else {
if ((r->rt == PF_REPLYTO) == (r->direction == pd->dir))
return;
- m0 = *m;
+ m0 = pd->m;
}
if (m0->m_len < sizeof(struct ip6_hdr)) {
@@ -5990,7 +5985,7 @@ pf_route6(struct mbuf **m, struct pf_pde
if (!r->rt) {
m0->m_pkthdr.pf.flags |= PF_TAG_GENERATED;
ip6_output(m0, NULL, NULL, 0, NULL, NULL);
- return;
+ goto done;
}
if (s == NULL) {
@@ -6051,7 +6046,7 @@ pf_route6(struct mbuf **m, struct pf_pde
done:
if (r->rt != PF_DUPTO)
- *m = NULL;
+ pd->m = NULL;
return;
bad:
@@ -6659,7 +6654,7 @@ pf_test(sa_family_t af, int fwdir, struc
/* if packet has been reassembled, update packet description */
if (pf_status.reass && pd.virtual_proto == PF_VPROTO_FRAGMENT) {
- action = pf_setup_pdesc(&pd, &pdhdrs, af, dir, kif, *m0,
+ action = pf_setup_pdesc(&pd, &pdhdrs, af, dir, kif, pd.m,
&reason);
if (action != PF_PASS) {
#if NPFLOG > 0
@@ -6885,22 +6880,23 @@ done:
switch (action) {
case PF_SYNPROXY_DROP:
- m_freem(*m0);
+ m_freem(pd.m);
+ /* FALLTHROUGH */
case PF_DEFER:
- *m0 = NULL;
+ pd.m = NULL;
action = PF_PASS;
break;
case PF_DIVERT:
switch (pd.af) {
case AF_INET:
if (!divert_packet(pd.m, pd.dir, r->divert_packet.port))
- *m0 = NULL;
+ pd.m = NULL;
break;
#ifdef INET6
case AF_INET6:
if (!divert6_packet(pd.m, pd.dir,
r->divert_packet.port))
- *m0 = NULL;
+ pd.m = NULL;
break;
#endif /* INET6 */
}
@@ -6909,33 +6905,29 @@ done:
#ifdef INET6
case PF_AFRT:
if (pf_translate_af(&pd)) {
- if (!pd.m)
- *m0 = NULL;
action = PF_DROP;
break;
}
if (pd.naf == AF_INET)
- pf_route(&pd.m, &pd, r, s);
+ pf_route(&pd, r, s);
if (pd.naf == AF_INET6)
- pf_route6(&pd.m, &pd, r, s);
- *m0 = NULL;
+ pf_route6(&pd, r, s);
action = PF_PASS;
break;
#endif /* INET6 */
case PF_DROP:
- m_freem(*m0);
- *m0 = NULL;
+ m_freem(pd.m);
+ pd.m = NULL;
break;
default:
- /* pf_route can free the mbuf causing *m0 to become NULL */
if (r->rt) {
switch (pd.af) {
case AF_INET:
- pf_route(m0, &pd, r, s);
+ pf_route(&pd, r, s);
break;
#ifdef INET6
case AF_INET6:
- pf_route6(m0, &pd, r, s);
+ pf_route6(&pd, r, s);
break;
#endif /* INET6 */
}
@@ -6945,11 +6937,11 @@ done:
#ifdef INET6
/* if reassembled packet passed, create new fragments */
- if (pf_status.reass && action == PF_PASS && *m0 && fwdir == PF_FWD) {
+ if (pf_status.reass && action == PF_PASS && pd.m && fwdir == PF_FWD) {
struct m_tag *mtag;
- if ((mtag = m_tag_find(*m0, PACKET_TAG_PF_REASSEMBLED, NULL)))
- action = pf_refragment6(m0, mtag, NULL, NULL);
+ if ((mtag = m_tag_find(pd.m, PACKET_TAG_PF_REASSEMBLED, NULL)))
+ action = pf_refragment6(&pd.m, mtag, NULL, NULL);
}
#endif /* INET6 */
if (s && action != PF_DROP) {
@@ -6959,6 +6951,7 @@ done:
s->if_index_out = ifp->if_index;
}
+ *m0 = pd.m;
return (action);
}
Index: net/pfvar.h
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/net/pfvar.h,v
retrieving revision 1.443
diff -u -p -r1.443 pfvar.h
--- net/pfvar.h 27 Oct 2016 21:41:20 -0000 1.443
+++ net/pfvar.h 28 Oct 2016 11:48:51 -0000
@@ -1710,10 +1710,8 @@ int pf_state_key_attach(struct pf_state_
int pf_translate(struct pf_pdesc *, struct pf_addr *, u_int16_t,
struct pf_addr *, u_int16_t, u_int16_t, int);
int pf_translate_af(struct pf_pdesc *);
-void pf_route(struct mbuf **, struct pf_pdesc *, struct pf_rule *,
- struct pf_state *);
-void pf_route6(struct mbuf **, struct pf_pdesc *, struct pf_rule *,
- struct pf_state *);
+void pf_route(struct pf_pdesc *, struct pf_rule *, struct pf_state *);
+void pf_route6(struct pf_pdesc *, struct pf_rule *, struct pf_state *);
void pfr_initialize(void);
int pfr_match_addr(struct pfr_ktable *, struct pf_addr *, sa_family_t);