Hello all, especially network hackers.
There are a few places in sys/net/pf_ioctl.c that look strange for me.
They are like so:
#ifndef INET
if (pp->af == AF_INET) {
error = EAFNOSUPPORT;
break;
}
#endif /* INET */
#ifndef INET6
if (pp->af == AF_INET6) {
error = EAFNOSUPPORT;
break;
}
#endif /* INET6 */
The problem is that newrule->af is not checked for anything except
AF_INET and AF_INET6. I.e. any address family will be accepted by ioctl.
It doesn't matter very much, because other routines will silently ignore
unknown address families. But why report EAFNOTSUPPORT at all then?
The sample patch is below (hope it will apply cleanly; I just extracted
it from the another work doing now).
--
Best wishes,
Vadim Zhukov
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?
Index: pf_ioctl.c
===================================================================
RCS file: /cvs/src/sys/net/pf_ioctl.c,v
retrieving revision 1.220
diff -u -p -r1.220 pf_ioctl.c
--- pf_ioctl.c 1 Sep 2009 13:42:00 -0000 1.220
+++ pf_ioctl.c 3 Oct 2009 16:58:58 -0000
@@ -1100,33 +1100,31 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
rule->cpid = p->p_pid;
rule->anchor = NULL;
rule->kif = NULL;
TAILQ_INIT(&rule->rdr.list);
TAILQ_INIT(&rule->nat.list);
/* initialize refcounting */
rule->states_cur = 0;
rule->src_nodes = 0;
rule->entries.tqe_prev = NULL;
+ if ((rule->af != AF_INET && rule->af != AF_INET6)
#ifndef INET
- if (rule->af == AF_INET) {
- pool_put(&pf_rule_pl, rule);
- error = EAFNOSUPPORT;
- break;
- }
+ || rule->af == AF_INET
#endif /* INET */
#ifndef INET6
- if (rule->af == AF_INET6) {
+ || rule->af == AF_INET6
+#endif /* INET6 */
+ ) {
pool_put(&pf_rule_pl, rule);
error = EAFNOSUPPORT;
break;
}
-#endif /* INET6 */
tail = TAILQ_LAST(ruleset->rules[rs_num].inactive.ptr,
pf_rulequeue);
if (tail)
rule->nr = tail->nr + 1;
else
rule->nr = 0;
if (rule->ifname[0]) {
rule->kif = pfi_kif_get(rule->ifname);
if (rule->kif == NULL) {
pool_put(&pf_rule_pl, rule);
@@ -1345,25 +1353,23 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
bcopy(&pcr->rule, newrule, sizeof(struct pf_rule));
newrule->cuid = p->p_cred->p_ruid;
newrule->cpid = p->p_pid;
TAILQ_INIT(&newrule->rdr.list);
TAILQ_INIT(&newrule->nat.list);
/* initialize refcounting */
newrule->states_cur = 0;
newrule->entries.tqe_prev = NULL;
+ if ((newrule->af != AF_INET && newrule->af != AF_INET6)
#ifndef INET
- if (newrule->af == AF_INET) {
- pool_put(&pf_rule_pl, newrule);
- error = EAFNOSUPPORT;
- break;
- }
+ || newrule->af == AF_INET
#endif /* INET */
#ifndef INET6
- if (newrule->af == AF_INET6) {
+ || newrule->af == AF_INET6
+#endif /* INET6 */
+ ) {
pool_put(&pf_rule_pl, newrule);
error = EAFNOSUPPORT;
break;
}
-#endif /* INET6 */
if (newrule->ifname[0]) {
newrule->kif = pfi_kif_get(newrule->ifname);
if (newrule->kif == NULL) {
@@ -1985,18 +2004,17 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
error = EBUSY;
break;
}
+ if ((pp->af != AF_INET && pp->af != AF_INET6)
#ifndef INET
- if (pp->af == AF_INET) {
- error = EAFNOSUPPORT;
- break;
- }
+ || pp->af == AF_INET
#endif /* INET */
#ifndef INET6
- if (pp->af == AF_INET6) {
+ || pp->af == AF_INET6
+#endif /* INET6 */
+ )
error = EAFNOSUPPORT;
break;
}
-#endif /* INET6 */
if (pp->addr.addr.type != PF_ADDR_ADDRMASK &&
pp->addr.addr.type != PF_ADDR_DYNIFTL &&
pp->addr.addr.type != PF_ADDR_TABLE) {
@@ -2121,20 +2139,18 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
break;
}
bcopy(&pca->addr, newpa, sizeof(struct pf_pooladdr));
+ if ((pca->af != AF_INET && pca->af != AF_INET6)
#ifndef INET
- if (pca->af == AF_INET) {
- pool_put(&pf_pooladdr_pl, newpa);
- error = EAFNOSUPPORT;
- break;
- }
+ || pca->af == AF_INET
#endif /* INET */
#ifndef INET6
- if (pca->af == AF_INET6) {
+ || pca->af == AF_INET6
+#endif /* INET6 */
+ ) {
pool_put(&pf_pooladdr_pl, newpa);
error = EAFNOSUPPORT;
break;
}
-#endif /* INET6 */
if (newpa->ifname[0]) {
newpa->kif = pfi_kif_get(newpa->ifname);
if (newpa->kif == NULL) {
