On Tue, Jul 04, 2017 at 08:10:21PM +0200, Alexander Bluhm wrote:
> Basically we have to remove all malloc(9) from pf or make it MP
> safe.
>
> login: panic: kernel diagnostic assertion "_kernel_lock_held()" failed: file
> "/usr/src/sys/kern/kern_malloc.c", line 373
> Stopped at db_enter+0x7: leave
> TID PID UID PRFLAGS PFLAGS CPU COMMAND
> *159741 24648 0 0x14000 0x200 0 pfpurge
> db_enter(d0b75a09,f5480e38,d0baed34,f5480e38,d0be3fe0) at db_enter+0x7
> panic(d0baed34,d09c409a,d0a42528,d0a92068,175) at panic+0x71
> __assert(d09c409a,d0a92068,175,d0a42528,b8f) at __assert+0x2e
> free(d7ecdd00,5,50,f5480ecc,d07b3375) at free+0x251
> tag_unref(d0bdc660,1,f5480010,0,0) at tag_unref+0x77
> pf_tag_unref(1,1,f5480f5c,d06dcb38,d0be0908) at pf_tag_unref+0x1a
> pf_free_state(d7329380,ffffffff,40,0,1) at pf_free_state+0x186
> pf_purge_expired_states(2,d0be0908,20,d0a39846,64) at
> pf_purge_expired_states+0
> x62
> pf_purge_thread(d774e2cc) at pf_purge_thread+0x6e
Convert pf tag malloc(9) and free(9) into a pool to make it MP safe.
While there use TAILQ_FOREACH macro for traversing tags.
ok?
bluhm
Index: net/pf_ioctl.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/net/pf_ioctl.c,v
retrieving revision 1.317
diff -u -p -r1.317 pf_ioctl.c
--- net/pf_ioctl.c 28 Jun 2017 19:30:24 -0000 1.317
+++ net/pf_ioctl.c 4 Jul 2017 22:22:15 -0000
@@ -84,6 +84,8 @@
#include <net/if_pfsync.h>
#endif /* NPFSYNC > 0 */
+struct pool pf_tag_pl;
+
void pfattach(int);
void pf_thread_create(void *);
int pfopen(dev_t, int, int, struct proc *);
@@ -165,6 +167,8 @@ pfattach(int num)
IPL_SOFTNET, 0, "pfruleitem", NULL);
pool_init(&pf_queue_pl, sizeof(struct pf_queuespec), 0,
IPL_SOFTNET, 0, "pfqueue", NULL);
+ pool_init(&pf_tag_pl, sizeof(struct pf_tagname), 0,
+ IPL_SOFTNET, 0, "pftag", NULL);
hfsc_initialize();
pfr_initialize();
pfi_initialize();
@@ -348,16 +352,17 @@ tagname2tag(struct pf_tags *head, char *
*/
/* new entry */
- if (!TAILQ_EMPTY(head))
- for (p = TAILQ_FIRST(head); p != NULL &&
- p->tag == new_tagid; p = TAILQ_NEXT(p, entries))
- new_tagid = p->tag + 1;
+ TAILQ_FOREACH(p, head, entries) {
+ if (p->tag != new_tagid)
+ break;
+ new_tagid = p->tag + 1;
+ }
if (new_tagid > TAGID_MAX)
return (0);
/* allocate and fill new struct pf_tagname */
- tag = malloc(sizeof(*tag), M_RTABLE, M_NOWAIT|M_ZERO);
+ tag = pool_get(&pf_tag_pl, PR_NOWAIT | PR_ZERO);
if (tag == NULL)
return (0);
strlcpy(tag->name, tagname, sizeof(tag->name));
@@ -392,12 +397,11 @@ tag_unref(struct pf_tags *head, u_int16_
if (tag == 0)
return;
- for (p = TAILQ_FIRST(head); p != NULL; p = next) {
- next = TAILQ_NEXT(p, entries);
+ TAILQ_FOREACH_SAFE(p, head, entries, next) {
if (tag == p->tag) {
if (--p->ref == 0) {
TAILQ_REMOVE(head, p, entries);
- free(p, M_RTABLE, sizeof(*p));
+ pool_put(&pf_tag_pl, p);
}
break;
}
