Re: bpfilter_create kassert panic
On Sun, Jul 24, 2016 at 09:31:08AM -0400, Ted Unangst wrote:
> Alexander Bluhm wrote:
> > Hi,
> >
> > When running some scapy regression tests, a current i386 machine
> > triggered this panic.
>
> dunno if this is best. maybe. the refcounting of units implies that they can
> last longer than close(). therefore, if open() is expecting close() to remove
> the unit from the list, we must make sure close does that. dangling refs then
> will be freed when the refcount drops.
>
> The bpfilter_destory function is obfuscation at this point.
>
> Alas, no time to test right now.
I completely missed that bpf devices can hang around after close. With
that in mind your diff makes sense to me. I think it would be wise to
remove the dependency from bpfdetach() on the bpf_d_list, as it is not
actually required since the introduction of bd_unit, to make sure the
early LIST_REMOVE() doesn't produce any side-effects. See updated diff
below.
Alexander, could you verify this fixes the panic on your i386 machine?
natano
Index: net/bpf.c
===
RCS file: /cvs/src/sys/net/bpf.c,v
retrieving revision 1.142
diff -u -p -r1.142 bpf.c
--- net/bpf.c 10 Jun 2016 20:33:29 - 1.142
+++ net/bpf.c 24 Jul 2016 16:30:30 -
@@ -117,7 +117,6 @@ int bpf_sysctl_locked(int *, u_int, void
struct bpf_d *bpfilter_lookup(int);
struct bpf_d *bpfilter_create(int);
-void bpfilter_destroy(struct bpf_d *);
/*
* Reference count access to descriptor buffers
@@ -368,6 +367,7 @@ bpfclose(dev_t dev, int flag, int mode,
if (d->bd_bif)
bpf_detachd(d);
bpf_wakeup(d);
+ LIST_REMOVE(d, bd_list);
D_PUT(d);
splx(s);
@@ -1494,7 +1494,7 @@ bpf_freed(struct bpf_d *d)
srp_update_locked(_insn_gc, >bd_rfilter, NULL);
srp_update_locked(_insn_gc, >bd_wfilter, NULL);
- bpfilter_destroy(d);
+ free(d, M_DEVBUF, sizeof(*d));
}
/*
@@ -1548,20 +1548,8 @@ bpfdetach(struct ifnet *ifp)
if (cdevsw[maj].d_open == bpfopen)
break;
- while ((bd = SRPL_FIRST_LOCKED(>bif_dlist))) {
- struct bpf_d *d;
-
- /*
-* Locate the minor number and nuke the vnode
-* for any open instance.
-*/
- LIST_FOREACH(d, _d_list, bd_list)
- if (d == bd) {
- vdevgone(maj, d->bd_unit,
- d->bd_unit, VCHR);
- break;
- }
- }
+ while ((bd = SRPL_FIRST_LOCKED(>bif_dlist)))
+ vdevgone(maj, bd->bd_unit, bd->bd_unit, VCHR);
free(bp, M_DEVBUF, sizeof *bp);
} else
@@ -1649,13 +1637,6 @@ bpfilter_create(int unit)
LIST_INSERT_HEAD(_d_list, bd, bd_list);
}
return (bd);
-}
-
-void
-bpfilter_destroy(struct bpf_d *bd)
-{
- LIST_REMOVE(bd, bd_list);
- free(bd, M_DEVBUF, sizeof(*bd));
}
/*
Re: bpfilter_create kassert panic
Alexander Bluhm wrote:
> Hi,
>
> When running some scapy regression tests, a current i386 machine
> triggered this panic.
dunno if this is best. maybe. the refcounting of units implies that they can
last longer than close(). therefore, if open() is expecting close() to remove
the unit from the list, we must make sure close does that. dangling refs then
will be freed when the refcount drops.
The bpfilter_destory function is obfuscation at this point.
Alas, no time to test right now.
Index: bpf.c
===
RCS file: /cvs/src/sys/net/bpf.c,v
retrieving revision 1.142
diff -u -p -r1.142 bpf.c
--- bpf.c 10 Jun 2016 20:33:29 - 1.142
+++ bpf.c 24 Jul 2016 13:30:29 -
@@ -117,7 +117,6 @@ int bpf_sysctl_locked(int *, u_int, void
struct bpf_d *bpfilter_lookup(int);
struct bpf_d *bpfilter_create(int);
-void bpfilter_destroy(struct bpf_d *);
/*
* Reference count access to descriptor buffers
@@ -368,6 +367,7 @@ bpfclose(dev_t dev, int flag, int mode,
if (d->bd_bif)
bpf_detachd(d);
bpf_wakeup(d);
+ LIST_REMOVE(d, bd_list);
D_PUT(d);
splx(s);
@@ -1494,7 +1494,7 @@ bpf_freed(struct bpf_d *d)
srp_update_locked(_insn_gc, >bd_rfilter, NULL);
srp_update_locked(_insn_gc, >bd_wfilter, NULL);
- bpfilter_destroy(d);
+ free(d, M_DEVBUF, sizeof(*d));
}
/*
@@ -1651,12 +1651,6 @@ bpfilter_create(int unit)
return (bd);
}
-void
-bpfilter_destroy(struct bpf_d *bd)
-{
- LIST_REMOVE(bd, bd_list);
- free(bd, M_DEVBUF, sizeof(*bd));
-}
/*
* Get a list of available data link type of the interface.
bpfilter_create kassert panic
Hi,
When running some scapy regression tests, a current i386 machine
triggered this panic.
bluhm
panic: kernel diagnostic assertion "bpfilter_lookup(unit) == NULL" failed: file
"../../../../net/bpf.c", line 1645
Stopped at Debugger+0x7: leave
TIDPIDUID PRFLAGS PFLAGS CPU COMMAND
*18583 18583 0 0x2 01 python2.7
39680 39680 0 0x14000 0x2102 softnet
Debugger(d0a01fd4,f570cb98,d09dca30,f570cb98,20) at Debugger+0x7
panic(d09dca30,d0959766,d09e37dc,d09e37fa,66d) at panic+0x71
__assert(d0959766,d09e37fa,66d,d09e37dc,db60f7c0) at __assert+0x2e
bpfilter_create(200,3f8,f570cc5c,db529af4,db6f0cec) at bpfilter_create+0x8b
bpfopen(21700,2,2000,db6f0cec,f570ce48) at bpfopen+0x29
spec_open_clone(f570ccb8,2180,0,0,db6f0cec) at spec_open_clone+0x158
spec_open(f570ccb8,48,f570ccd8,f570ccd4,5d568c47) at spec_open+0x1c3
VOP_OPEN(db529af4,2,db7ee960,db6f0cec,dad6d424) at VOP_OPEN+0x3f
vn_open(f570ce48,2,0,0,ff9c) at vn_open+0x16b
doopenat(db6f0cec,ff9c,cf7cbe68,1,0) at doopenat+0x176
sys_open(db6f0cec,f570cf5c,f570cf7c,0,f570cfa8) at sys_open+0x37
syscall() at syscall+0x24c
--- syscall (number 1) ---
0x7:
http://www.openbsd.org/ddb.html describes the minimum info required in bug
reports. Insufficient info makes it difficult to find and fix bugs.
ddb{1}> show register
ds 0x10
es 0x10
fs 0x20clean_idt
gs 0
edi0x100clean_idt+0xe0
esi 0xd09dca30__func__.6218+0x27d
ebp 0xf570cb4c
ebx 0xf570cb98
edx 0x1
ecx 0xd0b5727ckprintf_mutex
eax 0x1
eip 0xd055fe27Debugger+0x7
cs 0x8
eflags 0x200286start_phys+0x1a6
esp 0xf570cb3c
ss 0x10
Debugger+0x7: leave
ddb{1}> ps
TID PPID PGRPUID S FLAGS WAIT COMMAND
*18583 59926 82362 0 7 0x2python2.7
59926 6716 82362 0 30x10008a pause sh
6716 16109 82362 0 30x100082 piperdmake
16109 82362 82362 0 30x10008a pause sh
97424 87959 87959 57 30x100083 netio ftp
87959 42203 87959 0 30x83 piperdperl
82362 96135 82362 0 30x10008a pause make
43744 26800 7482 0 30x100082 piperdgzip
26800 96135 7482 0 30x100082 piperdpax
96135 7482 7482 0 30x82 piperdperl
7482 50744 7482 0 30x10008a pause ksh
50744 6457 50744 0 30x92 selectsshd
42203 59249 42203 0 30x10008b pause ksh
59249 6457 59249 0 30x92 selectsshd
91019 95762 95762 73 30x100090 kqreadsyslogd
95762 1 95762 0 30x100080 netio syslogd
63490 1 63490 0 30x100090 kqreadinetd
6457 1 6457 0 30x80 selectsshd
3755 1 3755 0 30x100083 ttyin ksh
94216 1 94216 0 30x100083 ttyin getty
84616 1 84616 0 30x100083 ttyin getty
99119 1 99119 0 30x100083 ttyin getty
37582 1 37582 0 30x100083 ttyin getty
61646 1 61646 0 30x100083 ttyin getty
77551 1 77551 0 30x100098 poll cron
92485 1 92485 99 30x100090 poll sndiod
19492 1 19492110 30x100090 poll sndiod
75722 80348 80348 95 30x100092 kqreadsmtpd
13828 80348 80348103 30x100092 kqreadsmtpd
97935 80348 80348 95 30x100092 kqreadsmtpd
12504 80348 80348 95 30x100092 kqreadsmtpd
5603 80348 80348 95 30x100092 kqreadsmtpd
90661 80348 80348 95 30x100092 kqreadsmtpd
80348 1 80348 0 30x100080 kqreadsmtpd
55186 0 0 0 3 0x14280 nfsidlnfsio
56545 0 0 0 3 0x14280 nfsidlnfsio
48066 0 0 0 3 0x14280 nfsidlnfsio
45832 0 0 0 3 0x14280 nfsidlnfsio
61138 97667 84727 83 30x100090 poll ntpd
97667 84727 84727 83 30x100090 poll ntpd
84727 1 84727 0 30x80 poll ntpd
54276 34610 34610 74 30x100090 bpf pflogd
34610 1 34610 0 30x80 netio pflogd
40161 0 0 0 3 0x14200 pgzerozerothread
86681 0 0 0 3 0x14200 aiodoned aiodoned
2152 0 0 0 3 0x14200 syncerupdate
32663 0 0 0 3 0x14200 cleaner cleaner
15241
