kern/49386: Possible panics on bpf attach/detach

[ozaki-r%netbsd.org@localhost]

>Number:         49386
>Category:       kern
>Synopsis:       Possible panics on bpf attach/detach
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Wed Nov 12 03:20:00 +0000 2014
>Originator:     Ryota Ozaki
>Release:        current
>Organization:
>Environment:
NetBSD kvm 7.99.1 NetBSD 7.99.1 (GENERIC.201411100400Z) #0: Mon Nov 10 04:55:44 UTC 2014  snj%b45.netbsd.org@localhost:/home/builds/ab/HEAD/amd64/201411100400Z-obj/home/source/ab/HEAD/src/sys/arch/amd64/compile/GENERIC amd64
>Description:
bpf is racy on attach and detach. We encounter the following panics
although they're very rare cases.

# panic#1
uvm_fault(0xfffffe8001fea2e0, 0x0, 1) -> e
fatal page fault in supervisor mode
trap type 6 code 0 rip ffffffff803e8802 cs 8 rflags 10246 cr2 39 ilevel 6 rsp fffffe8002463d70
curlwp 0xfffffe8002041600 pid 2338.1 lowest kstack 0xfffffe80024602c0
kernel: page fault trap, code=0
Stopped in pid 2338.1 (tcpdump) at      netbsd:ifpromisc+0xa:   movl    38(%rdi)
,%r12d
db{2}> bt
ifpromisc() at netbsd:ifpromisc+0xa
bpf_ioctl() at netbsd:bpf_ioctl+0x825
sys_ioctl() at netbsd:sys_ioctl+0x17e
syscall() at netbsd:syscall+0x9a
--- syscall (number 54) ---
7f7ff66cea0a:

# panic#2
uvm_fault(0xfffffe800319d468, 0x700000000, 2) -> e
fatal page fault in supervisor mode
trap type 6 code 2 rip ffffffff80219327 cs 8 rflags 10246 cr2 700000006 ilevel 6 rsp fffffe8001fbed90
curlwp 0xfffffe8002eca6e0 pid 792.1 lowest kstack 0xfffffe8001fbb2c0
kernel: page fault trap, code=0
Stopped in pid 792.1 (tcpdump) at       netbsd:bpf_ioctl+0x75f: movq    %r12,0(%
rax)
db{2}> bt
bpf_ioctl() at netbsd:bpf_ioctl+0x75f
sys_ioctl() at netbsd:sys_ioctl+0x17e
syscall() at netbsd:syscall+0x9a
--- syscall (number 54) ---
7f7ff66cea0a:

>How-To-Repeat:
The below script causes the above panic on KVM with 4 VCPUs.
A different machine would require different parameters.

#!/bin/sh
IF0=vioif0
IF1=vioif1
 
tap_create()
{
  name=$1
  ifconfig $name create
  ifconfig $name up
}
 
tap_destroy()
{
  name=$1
  ifconfig $name destroy
}
 
while true; do tcpdump -n -i $IF0 -c 10 > /dev/null 2>&1; done &
while true; do tcpdump -n -i $IF0 -c 10 > /dev/null 2>&1; done &
while true; do tcpdump -n -i $IF1 -c 10 > /dev/null 2>&1; done &
while true; do tcpdump -n -i $IF1 -c 10 > /dev/null 2>&1; done &
 
while true; do
  tap_create tap0 &
  for i in $(seq 0 2); do
    tcpdump -n -i tap0 -c 10 > /dev/null 2>&1 &
  done
  pkill tcpdump
  tap_destroy tap0 &
done
# EOF
>Fix:
diff --git a/sys/net/bpf.c b/sys/net/bpf.c
index f66e783..2e68ad1 100644
--- a/sys/net/bpf.c
+++ b/sys/net/bpf.c
@@ -340,6 +340,7 @@ bad:
 static void
 bpf_attachd(struct bpf_d *d, struct bpf_if *bp)
 {
+	KASSERT(mutex_owned(&bpf_mtx));
 	/*
 	 * Point d at bp, and add d to the interface's list of listeners.
 	 * Finally, point the driver's bpf cookie at the interface so
@@ -361,6 +362,8 @@ bpf_detachd(struct bpf_d *d)
 	struct bpf_d **p;
 	struct bpf_if *bp;
 
+	KASSERT(mutex_owned(&bpf_mtx));
+
 	bp = d->bd_bif;
 	/*
 	 * Check if this descriptor had requested promiscuous mode.
@@ -476,6 +479,7 @@ bpf_close(struct file *fp)
 	int s;
 
 	KERNEL_LOCK(1, NULL);
+	mutex_enter(&bpf_mtx);
 
 	/*
 	 * Refresh the PID associated with this bpf file.
@@ -490,15 +494,14 @@ bpf_close(struct file *fp)
 		bpf_detachd(d);
 	splx(s);
 	bpf_freed(d);
-	mutex_enter(&bpf_mtx);
 	LIST_REMOVE(d, bd_list);
-	mutex_exit(&bpf_mtx);
 	callout_destroy(&d->bd_callout);
 	seldestroy(&d->bd_sel);
 	softint_disestablish(d->bd_sih);
 	free(d, M_DEVBUF);
 	fp->f_bpf = NULL;
 
+	mutex_exit(&bpf_mtx);
 	KERNEL_UNLOCK_ONE(NULL);
 
 	return (0);
@@ -900,10 +903,12 @@ bpf_ioctl(struct file *fp, u_long cmd, void *addr)
 	 * Set device parameters.
 	 */
 	case BIOCSDLT:
+		mutex_enter(&bpf_mtx);
 		if (d->bd_bif == NULL)
 			error = EINVAL;
 		else
 			error = bpf_setdlt(d, *(u_int *)addr);
+		mutex_exit(&bpf_mtx);
 		break;
 
 	/*
@@ -926,7 +931,9 @@ bpf_ioctl(struct file *fp, u_long cmd, void *addr)
 	case OBIOCSETIF:
 #endif
 	case BIOCSETIF:
+		mutex_enter(&bpf_mtx);
 		error = bpf_setif(d, addr);
+		mutex_exit(&bpf_mtx);
 		break;
 
 	/*
@@ -1152,6 +1159,7 @@ bpf_setif(struct bpf_d *d, struct ifreq *ifr)
 	char *cp;
 	int unit_seen, i, s, error;
 
+	KASSERT(mutex_owned(&bpf_mtx));
 	/*
 	 * Make sure the provided name has a unit number, and default
 	 * it to '0' if not specified.
@@ -1718,10 +1726,10 @@ static int
 bpf_allocbufs(struct bpf_d *d)
 {
 
-	d->bd_fbuf = malloc(d->bd_bufsize, M_DEVBUF, M_WAITOK | M_CANFAIL);
+	d->bd_fbuf = malloc(d->bd_bufsize, M_DEVBUF, M_NOWAIT);
 	if (!d->bd_fbuf)
 		return (ENOBUFS);
-	d->bd_sbuf = malloc(d->bd_bufsize, M_DEVBUF, M_WAITOK | M_CANFAIL);
+	d->bd_sbuf = malloc(d->bd_bufsize, M_DEVBUF, M_NOWAIT);
 	if (!d->bd_sbuf) {
 		free(d->bd_fbuf, M_DEVBUF);
 		return (ENOBUFS);
@@ -1771,6 +1779,7 @@ _bpfattach(struct ifnet *ifp, u_int dlt, u_int hdrlen, struct bpf_if **driverp)
 	if (bp == NULL)
 		panic("bpfattach");
 
+	mutex_enter(&bpf_mtx);
 	bp->bif_dlist = NULL;
 	bp->bif_driverp = driverp;
 	bp->bif_ifp = ifp;
@@ -1782,6 +1791,7 @@ _bpfattach(struct ifnet *ifp, u_int dlt, u_int hdrlen, struct bpf_if **driverp)
 	*bp->bif_driverp = NULL;
 
 	bp->bif_hdrlen = hdrlen;
+	mutex_exit(&bpf_mtx);
 #if 0
 	printf("bpf: %s attached\n", ifp->if_xname);
 #endif
@@ -1797,6 +1807,7 @@ _bpfdetach(struct ifnet *ifp)
 	struct bpf_d *d;
 	int s;
 
+	mutex_enter(&bpf_mtx);
 	/* Nuke the vnodes for any open instances */
 	LIST_FOREACH(d, &bpf_list, bd_list) {
 		if (d->bd_bif != NULL && d->bd_bif->bif_ifp == ifp) {
@@ -1820,6 +1831,7 @@ _bpfdetach(struct ifnet *ifp)
 			goto again;
 		}
 	}
+	mutex_exit(&bpf_mtx);
 }
 
 /*
@@ -1880,6 +1892,8 @@ bpf_setdlt(struct bpf_d *d, u_int dlt)
 	struct ifnet *ifp;
 	struct bpf_if *bp;
 
+	KASSERT(mutex_owned(&bpf_mtx));
+
 	if (d->bd_bif->bif_dlt == dlt)
 		return 0;
 	ifp = d->bd_bif->bif_ifp;