Date: Fri, 15 Nov 2013 12:16:41 GMT From: Kajetan Staszkiewicz <vegeta@tuxpowered.net> To: freebsd-gnats-submit@FreeBSD.org Subject: kern/183997: route-to rule passes traffic when no targets are specified Message-ID: <201311151216.rAFCGfaA072054@oldred.freebsd.org> Resent-Message-ID: <201311151220.rAFCK0xA051283@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
>Number: 183997
>Category: kern
>Synopsis: route-to rule passes traffic when no targets are specified
>Confidential: no
>Severity: non-critical
>Priority: low
>Responsible: freebsd-bugs
>State: open
>Quarter:
>Keywords:
>Date-Required:
>Class: sw-bug
>Submitter-Id: current-users
>Arrival-Date: Fri Nov 15 12:20:00 UTC 2013
>Closed-Date:
>Last-Modified:
>Originator: Kajetan Staszkiewicz
>Release: 9.1, 10.0-BETA
>Organization:
InnoGames GmbH
>Environment:
FreeBSD xxxxxxxx 9.1-RELEASE-p7 FreeBSD 9.1-RELEASE-p7 #93 r+cb64f52: Fri Oct 25 16:05:48 CEST 2013 root@xxxxxxx:/usr/obj/mnt/builder/freebsd.git/sys/IGLB3 amd64
>Description:
When rule's redirection pool is empty, a packet still gets forwarded but no redirection occurs. In case of using route-to the packet leaving the router will have original target MAC address and thus will be received by the rotuer itself again if public and internal interfaces are on the same network. Due to no TTL decrease in pf, this situation causes a network loop stealing bandwidth and CPU time.
Redirection pool can be emptied by misconfiguration or by some automatic healthchecking tool which decides that all targets are dead and there is nobody to forward the traffic to.
>How-To-Repeat:
Empty a table and have some traffic hit a route-to rule using this table.
>Fix:
Pass the status of pf_map_addr to pf_set_rt_ifp and then to pf_create_state. If the status shows that pf_map_addr has failed, abort creation of the state and try to delete the src_node it could have created.
The attached patch is for FreeBSD 10.0-BETA3 and was not yet tested under some bigger load, although the same approach works well for FreeBSD 9.1. I can provide the 9.1 patch too if requested.
Patch attached with submission follows:
# When rule's redirection pool is empty, a packet still gets forwarded but no
# redirection occurs. In case of using route-to the packet leaving the router
# will have original target MAC address and thus will be received by the
# rotuer itself again if public and internal interfaces are on the same
# network. Due to no TTL decrease in pf, this situation causes a network
# loop stealing bandwidth and CPU time.
#
# Pass the status of pf_map_addr to pf_set_rt_ifp and then to pf_create_state.
# If the status shows that pf_map_addr has failed, abort creation of the
# state and try to delete the src_node it could have created.
#
# kajetan.staszkiewicz@innogames.de
# Work sponsored by InnoGames GmbH
#
diff --git a/sys/net/pfvar.h b/sys/net/pfvar.h
index e5395e3..7a44f47 100644
--- a/sys/net/pfvar.h
+++ b/sys/net/pfvar.h
@@ -1214,6 +1214,7 @@ struct pf_pdesc {
#define PFRES_SRCLIMIT 13 /* Source node/conn limit */
#define PFRES_SYNPROXY 14 /* SYN proxy */
#define PFRES_MAX 15 /* total+1 */
+#define PFRES_MAPFAILED 16 /* pf_map_addr failed */
#define PFRES_NAMES { \
"match", \
diff --git a/sys/netpfil/pf/pf.c b/sys/netpfil/pf/pf.c
index 9da73c5..12d1e9a 100644
--- a/sys/netpfil/pf/pf.c
+++ b/sys/netpfil/pf/pf.c
@@ -267,8 +267,8 @@ static u_int16_t pf_get_mss(struct mbuf *, int, u_int16_t,
sa_family_t);
static u_int16_t pf_calc_mss(struct pf_addr *, sa_family_t,
int, u_int16_t);
-static void pf_set_rt_ifp(struct pf_state *,
- struct pf_addr *);
+static int pf_set_rt_ifp(struct pf_state *,
+ struct pf_addr *, sa_family_t af);
static int pf_check_proto_cksum(struct mbuf *, int, int,
u_int8_t, sa_family_t);
static void pf_print_state_parts(struct pf_state *,
@@ -2929,29 +2929,32 @@ pf_calc_mss(struct pf_addr *addr, sa_family_t af, int rtableid, u_int16_t offer)
return (mss);
}
-static void
-pf_set_rt_ifp(struct pf_state *s, struct pf_addr *saddr)
+static int
+pf_set_rt_ifp(struct pf_state *s, struct pf_addr *saddr, sa_family_t af)
{
struct pf_rule *r = s->rule.ptr;
struct pf_src_node *sn = NULL;
+ int map_status = 0;
s->rt_kif = NULL;
if (!r->rt || r->rt == PF_FASTROUTE)
- return;
- switch (s->key[PF_SK_WIRE]->af) {
+ return 0;
+ switch (af) {
#ifdef INET
case AF_INET:
- pf_map_addr(AF_INET, r, saddr, &s->rt_addr, NULL, &sn);
+ map_status = pf_map_addr(AF_INET, r, saddr, &s->rt_addr, NULL, &sn);
s->rt_kif = r->rpool.cur->kif;
break;
#endif /* INET */
#ifdef INET6
case AF_INET6:
- pf_map_addr(AF_INET6, r, saddr, &s->rt_addr, NULL, &sn);
+ map_status = pf_map_addr(AF_INET6, r, saddr, &s->rt_addr, NULL, &sn);
s->rt_kif = r->rpool.cur->kif;
break;
#endif /* INET6 */
}
+
+ return map_status;
}
static u_int32_t
@@ -3516,6 +3519,19 @@ pf_create_state(struct pf_rule *r, struct pf_rule *nr, struct pf_rule *a,
s->timeout = PFTM_OTHER_FIRST_PACKET;
}
+ /* Call pf_set_rt_ifp (and thus pf_map_addr). If pf_map_addr fails,
+ remove the state and drop the packet. It makes no sense forwarding
+ it if redirection mapping has failed. Do it before setting timeouts,
+ csfailed won't remove the src_node otherwise. */
+ if (pf_set_rt_ifp(s, pd->src, pd->af)) {
+ REASON_SET(&reason, PFRES_MAPFAILED);
+ pf_src_tree_remove_state(s);
+ STATE_DEC_COUNTERS(s);
+ uma_zfree(V_pf_state_z, s);
+ /* Try to remove (nat_)src_node. */
+ goto csfailed;
+ }
+
s->creation = time_uptime;
s->expire = time_uptime;
@@ -3589,7 +3605,6 @@ pf_create_state(struct pf_rule *r, struct pf_rule *nr, struct pf_rule *a,
} else
*sm = s;
- pf_set_rt_ifp(s, pd->src); /* needs s->state_key set */
if (tag > 0)
s->tag = tag;
if (pd->proto == IPPROTO_TCP && (th->th_flags & (TH_SYN|TH_ACK)) ==
>Release-Note:
>Audit-Trail:
>Unformatted:
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201311151216.rAFCGfaA072054>