Re: another syzkaller problem in pf
On Sun, Jun 26, 2022 at 12:02:41PM +0200, Moritz Buhl wrote:
> On Tue, May 03, 2022 at 09:31:51PM +0200, Alexander Bluhm wrote:
> ...
> > The code is too complex to be sure what the reason of the syzkaller
> > panic is. Sleep in malloc is correct anyway and may improve the
> > situation.
> >
> > Functions with argument values 0 or 1 are hard to read. It would
> > be much nicer to pass M_WAITOK or M_NOWAIT. And the variable name
> > "intr" does not make sense anymore. pf does not run in interrupt
> > context. Call it "mflags" like in pfi_kif_alloc(). Or "wait" like
> > in other functions.
> >
> > Could you cleanup that also?
>
> I renamed the intr parameter for pfr_create_ktable and pfr_attach_table
> and passed it further up the call stack.
> In pf_addr_setup I noticed that pf_tbladdr_setup also used to set intr.
> I changed it to wait for both, pfi_dynaddr_setup and pf_tbladdr_setup.
>
> Now it should be a little easier to notice sleeping points in pf_ioctl.c.
>
> OK?
OK bluhm@
> Index: sys/net/pf.c
> ===
> RCS file: /cvs/src/sys/net/pf.c,v
> retrieving revision 1.1133
> diff -u -p -r1.1133 pf.c
> --- sys/net/pf.c 13 Jun 2022 12:48:00 - 1.1133
> +++ sys/net/pf.c 26 Jun 2022 09:21:21 -
> @@ -1585,11 +1585,11 @@ pf_purge_expired_states(u_int32_t maxche
> }
>
> int
> -pf_tbladdr_setup(struct pf_ruleset *rs, struct pf_addr_wrap *aw)
> +pf_tbladdr_setup(struct pf_ruleset *rs, struct pf_addr_wrap *aw, int wait)
> {
> if (aw->type != PF_ADDR_TABLE)
> return (0);
> - if ((aw->p.tbl = pfr_attach_table(rs, aw->v.tblname, 1)) == NULL)
> + if ((aw->p.tbl = pfr_attach_table(rs, aw->v.tblname, wait)) == NULL)
> return (1);
> return (0);
> }
> Index: sys/net/pf_if.c
> ===
> RCS file: /cvs/src/sys/net/pf_if.c,v
> retrieving revision 1.105
> diff -u -p -r1.105 pf_if.c
> --- sys/net/pf_if.c 16 May 2022 13:31:19 - 1.105
> +++ sys/net/pf_if.c 26 Jun 2022 09:38:43 -
> @@ -423,7 +423,7 @@ pfi_match_addr(struct pfi_dynaddr *dyn,
> }
>
> int
> -pfi_dynaddr_setup(struct pf_addr_wrap *aw, sa_family_t af)
> +pfi_dynaddr_setup(struct pf_addr_wrap *aw, sa_family_t af, int wait)
> {
> struct pfi_dynaddr *dyn;
> char tblname[PF_TABLE_NAME_SIZE];
> @@ -432,8 +432,7 @@ pfi_dynaddr_setup(struct pf_addr_wrap *a
>
> if (aw->type != PF_ADDR_DYNIFTL)
> return (0);
> - if ((dyn = pool_get(_addr_pl, PR_WAITOK | PR_LIMITFAIL | PR_ZERO))
> - == NULL)
> + if ((dyn = pool_get(_addr_pl, wait|PR_LIMITFAIL|PR_ZERO)) == NULL)
> return (1);
>
> if (!strcmp(aw->v.ifname, "self"))
> @@ -466,7 +465,7 @@ pfi_dynaddr_setup(struct pf_addr_wrap *a
> goto _bad;
> }
>
> - if ((dyn->pfid_kt = pfr_attach_table(ruleset, tblname, 1)) == NULL) {
> + if ((dyn->pfid_kt = pfr_attach_table(ruleset, tblname, wait)) == NULL) {
> rv = 1;
> goto _bad;
> }
> Index: sys/net/pf_ioctl.c
> ===
> RCS file: /cvs/src/sys/net/pf_ioctl.c,v
> retrieving revision 1.381
> diff -u -p -r1.381 pf_ioctl.c
> --- sys/net/pf_ioctl.c10 May 2022 23:12:25 - 1.381
> +++ sys/net/pf_ioctl.c26 Jun 2022 09:35:17 -
> @@ -891,8 +891,8 @@ int
> pf_addr_setup(struct pf_ruleset *ruleset, struct pf_addr_wrap *addr,
> sa_family_t af)
> {
> - if (pfi_dynaddr_setup(addr, af) ||
> - pf_tbladdr_setup(ruleset, addr) ||
> + if (pfi_dynaddr_setup(addr, af, PR_WAITOK) ||
> + pf_tbladdr_setup(ruleset, addr, PR_WAITOK) ||
> pf_rtlabel_add(addr))
> return (EINVAL);
>
> @@ -1413,7 +1413,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
>
> if (rule->overload_tblname[0]) {
> if ((rule->overload_tbl = pfr_attach_table(ruleset,
> - rule->overload_tblname, 0)) == NULL)
> + rule->overload_tblname, PR_WAITOK)) == NULL)
> error = EINVAL;
> else
> rule->overload_tbl->pfrkt_flags |=
> PFR_TFLAG_ACTIVE;
> @@ -1636,7 +1636,8 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
>
> if (newrule->overload_tblname[0]) {
> newrule->overload_tbl = pfr_attach_table(
> - ruleset, newrule->overload_tblname, 0);
> + ruleset, newrule->overload_tblname,
> + PR_WAITOK);
> if (newrule->overload_tbl == NULL)
> error = EINVAL;
> else
> Index: sys/net/pf_table.c
>
Re: another syzkaller problem in pf
On Tue, May 03, 2022 at 09:31:51PM +0200, Alexander Bluhm wrote:
...
> The code is too complex to be sure what the reason of the syzkaller
> panic is. Sleep in malloc is correct anyway and may improve the
> situation.
>
> Functions with argument values 0 or 1 are hard to read. It would
> be much nicer to pass M_WAITOK or M_NOWAIT. And the variable name
> "intr" does not make sense anymore. pf does not run in interrupt
> context. Call it "mflags" like in pfi_kif_alloc(). Or "wait" like
> in other functions.
>
> Could you cleanup that also?
I renamed the intr parameter for pfr_create_ktable and pfr_attach_table
and passed it further up the call stack.
In pf_addr_setup I noticed that pf_tbladdr_setup also used to set intr.
I changed it to wait for both, pfi_dynaddr_setup and pf_tbladdr_setup.
Now it should be a little easier to notice sleeping points in pf_ioctl.c.
OK?
mbuhl
Index: sys/net/pf.c
===
RCS file: /cvs/src/sys/net/pf.c,v
retrieving revision 1.1133
diff -u -p -r1.1133 pf.c
--- sys/net/pf.c13 Jun 2022 12:48:00 - 1.1133
+++ sys/net/pf.c26 Jun 2022 09:21:21 -
@@ -1585,11 +1585,11 @@ pf_purge_expired_states(u_int32_t maxche
}
int
-pf_tbladdr_setup(struct pf_ruleset *rs, struct pf_addr_wrap *aw)
+pf_tbladdr_setup(struct pf_ruleset *rs, struct pf_addr_wrap *aw, int wait)
{
if (aw->type != PF_ADDR_TABLE)
return (0);
- if ((aw->p.tbl = pfr_attach_table(rs, aw->v.tblname, 1)) == NULL)
+ if ((aw->p.tbl = pfr_attach_table(rs, aw->v.tblname, wait)) == NULL)
return (1);
return (0);
}
Index: sys/net/pf_if.c
===
RCS file: /cvs/src/sys/net/pf_if.c,v
retrieving revision 1.105
diff -u -p -r1.105 pf_if.c
--- sys/net/pf_if.c 16 May 2022 13:31:19 - 1.105
+++ sys/net/pf_if.c 26 Jun 2022 09:38:43 -
@@ -423,7 +423,7 @@ pfi_match_addr(struct pfi_dynaddr *dyn,
}
int
-pfi_dynaddr_setup(struct pf_addr_wrap *aw, sa_family_t af)
+pfi_dynaddr_setup(struct pf_addr_wrap *aw, sa_family_t af, int wait)
{
struct pfi_dynaddr *dyn;
char tblname[PF_TABLE_NAME_SIZE];
@@ -432,8 +432,7 @@ pfi_dynaddr_setup(struct pf_addr_wrap *a
if (aw->type != PF_ADDR_DYNIFTL)
return (0);
- if ((dyn = pool_get(_addr_pl, PR_WAITOK | PR_LIMITFAIL | PR_ZERO))
- == NULL)
+ if ((dyn = pool_get(_addr_pl, wait|PR_LIMITFAIL|PR_ZERO)) == NULL)
return (1);
if (!strcmp(aw->v.ifname, "self"))
@@ -466,7 +465,7 @@ pfi_dynaddr_setup(struct pf_addr_wrap *a
goto _bad;
}
- if ((dyn->pfid_kt = pfr_attach_table(ruleset, tblname, 1)) == NULL) {
+ if ((dyn->pfid_kt = pfr_attach_table(ruleset, tblname, wait)) == NULL) {
rv = 1;
goto _bad;
}
Index: sys/net/pf_ioctl.c
===
RCS file: /cvs/src/sys/net/pf_ioctl.c,v
retrieving revision 1.381
diff -u -p -r1.381 pf_ioctl.c
--- sys/net/pf_ioctl.c 10 May 2022 23:12:25 - 1.381
+++ sys/net/pf_ioctl.c 26 Jun 2022 09:35:17 -
@@ -891,8 +891,8 @@ int
pf_addr_setup(struct pf_ruleset *ruleset, struct pf_addr_wrap *addr,
sa_family_t af)
{
- if (pfi_dynaddr_setup(addr, af) ||
- pf_tbladdr_setup(ruleset, addr) ||
+ if (pfi_dynaddr_setup(addr, af, PR_WAITOK) ||
+ pf_tbladdr_setup(ruleset, addr, PR_WAITOK) ||
pf_rtlabel_add(addr))
return (EINVAL);
@@ -1413,7 +1413,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
if (rule->overload_tblname[0]) {
if ((rule->overload_tbl = pfr_attach_table(ruleset,
- rule->overload_tblname, 0)) == NULL)
+ rule->overload_tblname, PR_WAITOK)) == NULL)
error = EINVAL;
else
rule->overload_tbl->pfrkt_flags |=
PFR_TFLAG_ACTIVE;
@@ -1636,7 +1636,8 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
if (newrule->overload_tblname[0]) {
newrule->overload_tbl = pfr_attach_table(
- ruleset, newrule->overload_tblname, 0);
+ ruleset, newrule->overload_tblname,
+ PR_WAITOK);
if (newrule->overload_tbl == NULL)
error = EINVAL;
else
Index: sys/net/pf_table.c
===
RCS file: /cvs/src/sys/net/pf_table.c,v
retrieving revision 1.142
diff -u -p -r1.142 pf_table.c
--- sys/net/pf_table.c 16 Jun 2022 20:47:26 - 1.142
+++ sys/net/pf_table.c 26 Jun 2022
Re: [External] : Re: another syzkaller problem in pf
On Wed, May 04, 2022 at 04:26:18PM +0200, Alexander Bluhm wrote: > On Wed, May 04, 2022 at 02:21:11PM +0200, Alexandr Nedvedicky wrote: > > I'm not sure flipping a flag is a right change. In general we don't want > > to hold NET_LOCK()/PF_LOCK() while waiting for memory. > > - We must not wait for memory when in the packet processing hot path. > Drop the packet instead. > > - We should not wait for memory when holding pf lock. Then we can > replace rw lock with a mutex or something else later. > > - Waiting for memory with net lock is unavoidable. We have to > wait when coming from system call. Doing preallocation may be > possible in some cases but code may get too complicated elsewhere. > > If getting the allocation out of the locks is doable here, it could > be the best solution. I think it's doable. I'll try to finish the for tables and start sending diffs hopefully next week. regards sashan
Re: [External] : Re: another syzkaller problem in pf
On Wed, May 04, 2022 at 02:21:11PM +0200, Alexandr Nedvedicky wrote: > I'm not sure flipping a flag is a right change. In general we don't want > to hold NET_LOCK()/PF_LOCK() while waiting for memory. - We must not wait for memory when in the packet processing hot path. Drop the packet instead. - We should not wait for memory when holding pf lock. Then we can replace rw lock with a mutex or something else later. - Waiting for memory with net lock is unavoidable. We have to wait when coming from system call. Doing preallocation may be possible in some cases but code may get too complicated elsewhere. If getting the allocation out of the locks is doable here, it could be the best solution. bluhm
Re: [External] : Re: another syzkaller problem in pf
On Tue, May 03, 2022 at 09:31:51PM +0200, Alexander Bluhm wrote:
> On Tue, May 03, 2022 at 07:42:34PM +0200, Moritz Buhl wrote:
> > commit 4b3977248902c22d96aaebdb5784840debc2631c
> > Author: mikeb
> > Date: Mon Nov 24 13:22:09 2008 +
> >
> > Fix splasserts seen in pr 5987 by propagating a flag that discribes
> > whether we're called from the interrupt context to the functions
> > performing allocations.
>
> These days pf was protected with kernel lock and spl. Both are
> released when sleeping. Now we have netlock and pflock. These are
> rwlocks and not released during sleep. So this old race should not
> exist anymore.
>
> > And we are not in interrupt context.
>
> Yes, it is ioctl(2). I think we should always malloc with M_WAITOK
> when in syscall. Otherwise userland would have to cope with randomly
> failing syscalls.
>
> > If this is sound, then the only reason why pfr_destroy_ktable was called
> > is that pool_get is called with PR_NOWAIT. And then the following diff
> > would help.
>
> The code is too complex to be sure what the reason of the syzkaller
> panic is. Sleep in malloc is correct anyway and may improve the
> situation.
>
> Functions with argument values 0 or 1 are hard to read. It would
> be much nicer to pass M_WAITOK or M_NOWAIT. And the variable name
> "intr" does not make sense anymore. pf does not run in interrupt
> context. Call it "mflags" like in pfi_kif_alloc(). Or "wait" like
> in other functions.
>
> Could you cleanup that also?
I have a diff, which touches the same area. It's a work in progress change.
It moves all memory allocations outside of NET_LOCK/PF_LOCK. I'm just
sending it for your reference now.
I'm not sure flipping a flag is a right change. In general we don't want
to hold NET_LOCK()/PF_LOCK() while waiting for memory.
regards
sashan
8<---8<---8<--8<
diff --git a/sys/net/pf_ioctl.c b/sys/net/pf_ioctl.c
index 8315b115474..1f036e1368f 100644
--- a/sys/net/pf_ioctl.c
+++ b/sys/net/pf_ioctl.c
@@ -2217,12 +2217,8 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags,
struct proc *p)
error = ENODEV;
goto fail;
}
- NET_LOCK();
- PF_LOCK();
error = pfr_add_tables(io->pfrio_buffer, io->pfrio_size,
>pfrio_nadd, io->pfrio_flags | PFR_FLAG_USERIOCTL);
- PF_UNLOCK();
- NET_UNLOCK();
break;
}
diff --git a/sys/net/pf_table.c b/sys/net/pf_table.c
index fb23bcabe04..cbaa728b105 100644
--- a/sys/net/pf_table.c
+++ b/sys/net/pf_table.c
@@ -189,6 +189,7 @@ void pfr_clstats_ktable(struct
pfr_ktable *, time_t, int);
struct pfr_ktable *pfr_create_ktable(struct pfr_table *, time_t, int,
int);
voidpfr_destroy_ktables(struct pfr_ktableworkq *, int);
+voidpfr_destroy_ktables_aux(struct pfr_ktableworkq *);
voidpfr_destroy_ktable(struct pfr_ktable *, int);
int pfr_ktable_compare(struct pfr_ktable *,
struct pfr_ktable *);
@@ -1493,14 +1494,16 @@ pfr_clr_tables(struct pfr_table *filter, int *ndel, int
flags)
int
pfr_add_tables(struct pfr_table *tbl, int size, int *nadd, int flags)
{
- struct pfr_ktableworkq addq, changeq;
- struct pfr_ktable *p, *q, *r, key;
+ struct pfr_ktableworkq addq, changeq, auxq;
+ struct pfr_ktable *p, *q, *r, *n, *w, key;
int i, rv, xadd = 0;
time_t tzero = gettime();
ACCEPT_FLAGS(flags, PFR_FLAG_DUMMY);
SLIST_INIT();
SLIST_INIT();
+ SLIST_INIT();
+ /* pre-allocate all memory outside of locks */
for (i = 0; i < size; i++) {
YIELD(flags & PFR_FLAG_USERIOCTL);
if (COPYIN(tbl+i, _t, sizeof(key.pfrkt_t), flags))
@@ -1509,65 +1512,140 @@ pfr_add_tables(struct pfr_table *tbl, int size, int
*nadd, int flags)
flags & PFR_FLAG_USERIOCTL))
senderr(EINVAL);
key.pfrkt_flags |= PFR_TFLAG_ACTIVE;
- p = RB_FIND(pfr_ktablehead, _ktables, );
- if (p == NULL) {
- p = pfr_create_ktable(_t, tzero, 1,
- !(flags & PFR_FLAG_USERIOCTL));
- if (p == NULL)
- senderr(ENOMEM);
- SLIST_FOREACH(q, , pfrkt_workq) {
- if (!pfr_ktable_compare(p, q)) {
- pfr_destroy_ktable(p, 0);
- goto _skip;
- }
- }
- SLIST_INSERT_HEAD(, p, pfrkt_workq);
- xadd++;
-
Re: another syzkaller problem in pf
On Tue, May 03, 2022 at 07:42:34PM +0200, Moritz Buhl wrote:
> commit 4b3977248902c22d96aaebdb5784840debc2631c
> Author: mikeb
> Date: Mon Nov 24 13:22:09 2008 +
>
> Fix splasserts seen in pr 5987 by propagating a flag that discribes
> whether we're called from the interrupt context to the functions
> performing allocations.
These days pf was protected with kernel lock and spl. Both are
released when sleeping. Now we have netlock and pflock. These are
rwlocks and not released during sleep. So this old race should not
exist anymore.
> And we are not in interrupt context.
Yes, it is ioctl(2). I think we should always malloc with M_WAITOK
when in syscall. Otherwise userland would have to cope with randomly
failing syscalls.
> If this is sound, then the only reason why pfr_destroy_ktable was called
> is that pool_get is called with PR_NOWAIT. And then the following diff
> would help.
The code is too complex to be sure what the reason of the syzkaller
panic is. Sleep in malloc is correct anyway and may improve the
situation.
Functions with argument values 0 or 1 are hard to read. It would
be much nicer to pass M_WAITOK or M_NOWAIT. And the variable name
"intr" does not make sense anymore. pf does not run in interrupt
context. Call it "mflags" like in pfi_kif_alloc(). Or "wait" like
in other functions.
Could you cleanup that also?
bluhm
> Index: pf_if.c
> ===
> RCS file: /cvs/src/sys/net/pf_if.c,v
> retrieving revision 1.104
> diff -u -p -r1.104 pf_if.c
> --- pf_if.c 29 Apr 2022 09:55:43 - 1.104
> +++ pf_if.c 3 May 2022 16:01:23 -
> @@ -464,7 +464,7 @@ pfi_dynaddr_setup(struct pf_addr_wrap *a
> goto _bad;
> }
>
> - if ((dyn->pfid_kt = pfr_attach_table(ruleset, tblname, 1)) == NULL) {
> + if ((dyn->pfid_kt = pfr_attach_table(ruleset, tblname, 0)) == NULL) {
> rv = 1;
> goto _bad;
> }
Re: another syzkaller problem in pf
On 2022-05-03, Moritz Buhl wrote: > Hi tech@, > > Syzkaller found a few crashes in pf_anchor_global_RB_REMOVE. > https://syzkaller.appspot.com/bug?id=a97f712331903ce38b8c084a489818b9bb5c6fcb > and also > https://syzkaller.appspot.com/text?tag=CrashLog=15ace9aaf0 > > The call stack is something like this: > pf_anchor_global_RB_REMOVE > pf_remove_if_empty_ruleset > pfi_dynaddr_setup > pfioctl > > I believe this is caused by two calls to pf_remove_if_empty_ruleset > in pfi_dynaddr_setup and a possibly pfr_destroy_ktable. > > I think it is a problem that pfr_attach_table is being called > with 1 (intr) because the introducing commit says: > > commit 4b3977248902c22d96aaebdb5784840debc2631c > Author: mikeb > Date: Mon Nov 24 13:22:09 2008 + > > Fix splasserts seen in pr 5987 by propagating a flag that discribes > whether we're called from the interrupt context to the functions > performing allocations. > > Looked at by mpf@ and henning@, tested by mpf@ and Antti Harri, > the pr originator. > > ok tedu > > And we are not in interrupt context. A detailed analysis follows. It's been a while, but iirc the flag is not necessarily whether this call, happening right now, is interrupt or not, but whether the paired alloc or free was or will be. So user code may need to set the intr flag if later an interrupt context will be touching this. (Interrupts are still not ever allowed to use user context allocators.) It may be more complicated than just flipping the flag, because there may be other ways to reach this free that require it. Or it could just be a bug. Just make sure we never free anything allocated with intr in this path.
another syzkaller problem in pf
Hi tech@,
Syzkaller found a few crashes in pf_anchor_global_RB_REMOVE.
https://syzkaller.appspot.com/bug?id=a97f712331903ce38b8c084a489818b9bb5c6fcb
and also
https://syzkaller.appspot.com/text?tag=CrashLog=15ace9aaf0
The call stack is something like this:
pf_anchor_global_RB_REMOVE
pf_remove_if_empty_ruleset
pfi_dynaddr_setup
pfioctl
I believe this is caused by two calls to pf_remove_if_empty_ruleset
in pfi_dynaddr_setup and a possibly pfr_destroy_ktable.
I think it is a problem that pfr_attach_table is being called
with 1 (intr) because the introducing commit says:
commit 4b3977248902c22d96aaebdb5784840debc2631c
Author: mikeb
Date: Mon Nov 24 13:22:09 2008 +
Fix splasserts seen in pr 5987 by propagating a flag that discribes
whether we're called from the interrupt context to the functions
performing allocations.
Looked at by mpf@ and henning@, tested by mpf@ and Antti Harri,
the pr originator.
ok tedu
And we are not in interrupt context. A detailed analysis follows.
The pfioctl cd60441a is DIOCADDRULE.
The call stack fails in pf_addr_setup calls from pf_ioctl.c:
1647 if (pf_addr_setup(ruleset, >src.addr,
1648 newrule->af))
similar calls below.
One of these calls might do the following:
424 pfi_dynaddr_setup(struct pf_addr_wrap *aw, sa_family_t af)
...
462 if ((ruleset = pf_find_or_create_ruleset(PF_RESERVED_ANCHOR)) ==
NULL) {
assume false.
...
467 if ((dyn->pfid_kt = pfr_attach_table(ruleset, tblname, 1)) == NULL)
might evaluate to true, I will share my thoughts on it.
468 rv = 1;
469 goto _bad;
...
481 _bad:
482 if (dyn->pfid_kt != NULL)
483 pfr_detach_table(dyn->pfid_kt);
484 if (ruleset != NULL)
485 pf_remove_if_empty_ruleset(ruleset);
remember this call. I will show a case where we don't want this.
due to the following in pf_table.c:
2424 pfr_attach_table(struct pf_ruleset *rs, char *name, int intr)
...
2434 kt = pfr_lookup_table();
2435 if (kt == NULL) {
assume true.
2436 kt = pfr_create_ktable(, gettime(), 1, intr);
2437 if (kt == NULL)
2438 return (NULL);
2439 if (ac != NULL) {
assume true.
this means kt != NULL and kt->pfrkt_rs = our ruleset.
2440 bzero(tbl.pfrt_anchor, sizeof(tbl.pfrt_anchor));
2441 rt = pfr_lookup_table();
2442 if (rt == NULL) {
assume true.
pfr_ktable_compares pfrkt_name an pfrkt_anchor if previous was same.
pfrkt_name should be '\0', pfrkt_anchor is PF_RESERVED_ANCHOR vs '\0'
2443 rt = pfr_create_ktable(, 0, 1, intr);
let's look into this call next just to be sure.
2444 if (rt == NULL) {
assume true.
2445 pfr_destroy_ktable(kt, 0);
remember this call is possible. We look at this afterwards.
2446 return (NULL);
...
looking into create_ktable, I want to see if we can reach pfr_destroy_ktable,
still in pf_table.c:
2202 pfr_create_ktable(struct pfr_table *tbl, time_t tzero, int attachruleset
...
2208 if (intr)
2209 kt = pool_get(_ktable_pl,
PR_NOWAIT|PR_ZERO|PR_LIMITFAIL);
This can fail. Returning NULL afterwards.
Note PR_ZERO.
...
2216 if (attachruleset) {
2217 rs = pf_find_or_create_ruleset(tbl->pfrt_anchor);
2218 if (!rs) {
not reachable because pfrt_anchor comes from l.2433.
But a weird indirection for our current call stack anyway. Might be necessary.
2219 pfr_destroy_ktable(kt, 0);
2220 return (NULL);
2221 }
kt->pfrkt_rs = rs;
2223 rs->tables++;
2224 }
2225
2226 if (!rn_inithead((void **)>pfrkt_ip4,
2227 offsetof(struct sockaddr_in, sin_addr)) ||
2228 !rn_inithead((void **)>pfrkt_ip6,
2229 offsetof(struct sockaddr_in6, sin6_addr))) {
unreachable because >pfrkt_ip{4,6} are NULL.
2230 pfr_destroy_ktable(kt, 0);
2231 return (NULL);
2232 }
Back to pfr_destroy_ktable, still in pf_table.c:
2253 pfr_destroy_ktable(struct pfr_ktable *kt, int flushaddr)
2254 {
...
2268 if (kt->pfrkt_rs != NULL) {
assume true.
2269 kt->pfrkt_rs->tables--;
2270 pf_remove_if_empty_ruleset(kt->pfrkt_rs);
our first call to pf_remove_if_empty_ruleset.
The second one follows from pfi_dynaddr_setup.
If this is sound, then the only reason why pfr_destroy_ktable was called
is that pool_get is called with PR_NOWAIT. And then the following diff
would help.
mbuhl
Index: pf_if.c
===
RCS file: /cvs/src/sys/net/pf_if.c,v
retrieving revision 1.104
diff -u -p
