On Mon, May 08, 2017 at 02:56:55PM +0200, Patrick Wildt wrote:
> @@ -2307,9 +2326,9 @@ pfr_pool_get(struct pf_pool *rpool, struct pf_addr
> **raddr,
> rpool->curweight = kt->pfrkt_maxweight;
> }
>
> - pfr_prepare_network(&pfr_mask, af, ke->pfrke_net);
> + pfr_prepare_network(&mask, af, ke->pfrke_net);
> *raddr = SUNION2PF(&ke->pfrke_sa, af);
> - *rmask = SUNION2PF(&pfr_mask, af);
> + *rmask = SUNION2PF(&mask, af);
>
> if (use_counter && !PF_AZERO(counter, af)) {
> /* is supplied address within block? */
Before committing I realised that this actually returns the pointer
instead of the contents, which would mean that we would now return
a pointer to the callee's stack.
I think we should change pfr_pool_get() so that it's the caller's
responsibility to pass a structure where the rmask can fit into. Then
we should always work with that structure and copy the elements instead
of changing rmask by setting it to a different pointer.
Maybe we should do the same for raddr.
Opinions?
diff --git a/sys/net/pf_lb.c b/sys/net/pf_lb.c
index 74acc538b8f..af57272c4dc 100644
--- a/sys/net/pf_lb.c
+++ b/sys/net/pf_lb.c
@@ -339,7 +339,7 @@ pf_map_addr(sa_family_t af, struct pf_rule *r, struct
pf_addr *saddr,
unsigned char hash[16];
struct pf_addr faddr;
struct pf_addr *raddr = &rpool->addr.v.a.addr;
- struct pf_addr *rmask = &rpool->addr.v.a.mask;
+ struct pf_addr rmask = rpool->addr.v.a.mask;
u_int64_t states;
u_int16_t weight;
u_int64_t load;
@@ -361,7 +361,7 @@ pf_map_addr(sa_family_t af, struct pf_rule *r, struct
pf_addr *saddr,
!PF_POOL_DYNTYPE(rpool->opts))
return (1);
raddr = &rpool->addr.p.dyn->pfid_addr4;
- rmask = &rpool->addr.p.dyn->pfid_mask4;
+ rmask = rpool->addr.p.dyn->pfid_mask4;
break;
#ifdef INET6
case AF_INET6:
@@ -369,7 +369,7 @@ pf_map_addr(sa_family_t af, struct pf_rule *r, struct
pf_addr *saddr,
!PF_POOL_DYNTYPE(rpool->opts))
return (1);
raddr = &rpool->addr.p.dyn->pfid_addr6;
- rmask = &rpool->addr.p.dyn->pfid_mask6;
+ rmask = rpool->addr.p.dyn->pfid_mask6;
break;
#endif /* INET6 */
default:
@@ -380,7 +380,7 @@ pf_map_addr(sa_family_t af, struct pf_rule *r, struct
pf_addr *saddr,
return (1); /* unsupported */
} else {
raddr = &rpool->addr.v.a.addr;
- rmask = &rpool->addr.v.a.mask;
+ rmask = rpool->addr.v.a.mask;
}
switch (rpool->opts & PF_POOL_TYPEMASK) {
@@ -388,7 +388,7 @@ pf_map_addr(sa_family_t af, struct pf_rule *r, struct
pf_addr *saddr,
PF_ACPY(naddr, raddr, af);
break;
case PF_POOL_BITMASK:
- PF_POOLMASK(naddr, raddr, rmask, saddr, af);
+ PF_POOLMASK(naddr, raddr, &rmask, saddr, af);
break;
case PF_POOL_RANDOM:
if (rpool->addr.type == PF_ADDR_TABLE) {
@@ -418,31 +418,31 @@ pf_map_addr(sa_family_t af, struct pf_rule *r, struct
pf_addr *saddr,
break;
#ifdef INET6
case AF_INET6:
- if (rmask->addr32[3] != 0xffffffff)
+ if (rmask.addr32[3] != 0xffffffff)
rpool->counter.addr32[3] = arc4random();
else
break;
- if (rmask->addr32[2] != 0xffffffff)
+ if (rmask.addr32[2] != 0xffffffff)
rpool->counter.addr32[2] = arc4random();
else
break;
- if (rmask->addr32[1] != 0xffffffff)
+ if (rmask.addr32[1] != 0xffffffff)
rpool->counter.addr32[1] = arc4random();
else
break;
- if (rmask->addr32[0] != 0xffffffff)
+ if (rmask.addr32[0] != 0xffffffff)
rpool->counter.addr32[0] = arc4random();
break;
#endif /* INET6 */
default:
unhandled_af(af);
}
- PF_POOLMASK(naddr, raddr, rmask, &rpool->counter, af);
+ PF_POOLMASK(naddr, raddr, &rmask, &rpool->counter, af);
PF_ACPY(init_addr, naddr, af);
} else {
PF_AINC(&rpool->counter, af);
- PF_POOLMASK(naddr, raddr, rmask, &rpool->counter, af);
+ PF_POOLMASK(naddr, raddr, &rmask, &rpool->counter, af);
}
break;
case PF_POOL_SRCHASH:
@@ -469,7 +469,7 @@ pf_map_addr(sa_family_t af, struct pf_rule *r, struct
pf_addr *saddr,
return (1);
PF_ACPY(naddr, &rpool->counter, af);
} else {
- PF_POOLMASK(naddr, raddr, rmask,
+ PF_POOLMASK(naddr, raddr, &rmask,
(struct pf_addr *)&hash, af);
}
break;
@@ -485,7 +485,7 @@ pf_map_addr(sa_family_t af, struct pf_rule *r, struct
pf_addr *saddr,
if (pfr_pool_get(rpool, &raddr, &rmask, af))
return (1);
}
- } else if (pf_match_addr(0, raddr, rmask, &rpool->counter, af))
+ } else if (pf_match_addr(0, raddr, &rmask, &rpool->counter, af))
return (1);
/* iterate over table if it contains entries which are weighted
*/
@@ -527,7 +527,7 @@ pf_map_addr(sa_family_t af, struct pf_rule *r, struct
pf_addr *saddr,
if (pfr_pool_get(rpool, &raddr, &rmask, af))
return (1);
}
- } else if (pf_match_addr(0, raddr, rmask, &rpool->counter, af))
+ } else if (pf_match_addr(0, raddr, &rmask, &rpool->counter, af))
return (1);
states = rpool->states;
@@ -557,7 +557,7 @@ pf_map_addr(sa_family_t af, struct pf_rule *r, struct
pf_addr *saddr,
rpool->addr.type == PF_ADDR_DYNIFTL) {
if (pfr_pool_get(rpool, &raddr, &rmask, af))
return (1);
- } else if (pf_match_addr(0, raddr, rmask,
+ } else if (pf_match_addr(0, raddr, &rmask,
&rpool->counter, af))
return (1);
@@ -581,7 +581,7 @@ pf_map_addr(sa_family_t af, struct pf_rule *r, struct
pf_addr *saddr,
PF_AZERO(init_addr, af))
PF_ACPY(init_addr, naddr, af);
}
- } while (pf_match_addr(1, &faddr, rmask, &rpool->counter, af) &&
+ } while (pf_match_addr(1, &faddr, &rmask, &rpool->counter, af)
&&
(states > 0));
if (rpool->addr.type == PF_ADDR_TABLE) {
diff --git a/sys/net/pf_table.c b/sys/net/pf_table.c
index 7666ec7013c..f23739e8565 100644
--- a/sys/net/pf_table.c
+++ b/sys/net/pf_table.c
@@ -140,7 +140,6 @@ struct pfr_walktree {
struct pool pfr_ktable_pl;
struct pool pfr_kentry_pl[PFRKE_MAX];
struct pool pfr_kcounters_pl;
-union sockaddr_union pfr_mask;
struct pf_addr pfr_ffaddr;
int pfr_gcd(int, int);
@@ -2252,7 +2251,7 @@ pfr_islinklocal(sa_family_t af, struct pf_addr *addr)
int
pfr_pool_get(struct pf_pool *rpool, struct pf_addr **raddr,
- struct pf_addr **rmask, sa_family_t af)
+ struct pf_addr *rmask, sa_family_t af)
{
struct pfr_ktable *kt;
struct pfr_kentry *ke, *ke2;
@@ -2327,13 +2326,13 @@ pfr_pool_get(struct pf_pool *rpool, struct pf_addr
**raddr,
rpool->curweight = kt->pfrkt_maxweight;
}
- pfr_prepare_network(&pfr_mask, af, ke->pfrke_net);
+ pfr_prepare_network(&mask, af, ke->pfrke_net);
*raddr = SUNION2PF(&ke->pfrke_sa, af);
- *rmask = SUNION2PF(&pfr_mask, af);
+ *rmask = *SUNION2PF(&mask, af);
if (use_counter && !PF_AZERO(counter, af)) {
/* is supplied address within block? */
- if (!PF_MATCHA(0, *raddr, *rmask, counter, af)) {
+ if (!PF_MATCHA(0, *raddr, rmask, counter, af)) {
/* no, go to next block in table */
idx++;
use_counter = 0;
@@ -2417,7 +2416,7 @@ _next_entry:
pfr_prepare_network(&mask, AF_INET, ke2->pfrke_net);
PF_POOLMASK(addr, addr, SUNION2PF(&mask, af), &pfr_ffaddr, af);
PF_AINC(addr, af);
- if (!PF_MATCHA(0, *raddr, *rmask, addr, af)) {
+ if (!PF_MATCHA(0, *raddr, rmask, addr, af)) {
/* ok, we reached the end of our main block */
/* go to next block in table */
idx++;
diff --git a/sys/net/pfvar.h b/sys/net/pfvar.h
index 981d778e37a..982437b64ca 100644
--- a/sys/net/pfvar.h
+++ b/sys/net/pfvar.h
@@ -1725,7 +1725,7 @@ int pfr_match_addr(struct pfr_ktable *, struct
pf_addr *, sa_family_t);
void pfr_update_stats(struct pfr_ktable *, struct pf_addr *,
struct pf_pdesc *, int, int);
int pfr_pool_get(struct pf_pool *, struct pf_addr **,
- struct pf_addr **, sa_family_t);
+ struct pf_addr *, sa_family_t);
int pfr_states_increase(struct pfr_ktable *, struct pf_addr *, int);
int pfr_states_decrease(struct pfr_ktable *, struct pf_addr *, int);
struct pfr_kentry *
