NetBSD Problem Report #49386

From www@NetBSD.org  Wed Nov 12 03:16:26 2014
Return-Path: <www@NetBSD.org>
Received: from mail.netbsd.org (mail.netbsd.org [149.20.53.66])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
	(Client CN "mail.netbsd.org", Issuer "Postmaster NetBSD.org" (verified OK))
	by mollari.NetBSD.org (Postfix) with ESMTPS id 5EAF7A66A3
	for <gnats-bugs@gnats.NetBSD.org>; Wed, 12 Nov 2014 03:16:26 +0000 (UTC)
Message-Id: <20141112031624.AD10EA66A2@mollari.NetBSD.org>
Date: Wed, 12 Nov 2014 03:16:24 +0000 (UTC)
From: ozaki-r@netbsd.org
Reply-To: ozaki-r@netbsd.org
To: gnats-bugs@NetBSD.org
Subject: Possible panics on bpf attach/detach
X-Send-Pr-Version: www-1.0

>Number:         49386
>Category:       kern
>Synopsis:       Possible panics on bpf attach/detach
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Wed Nov 12 03:20:00 +0000 2014
>Closed-Date:    Sat Jun 16 23:46:27 +0000 2018
>Last-Modified:  Sat Jun 16 23:46:27 +0000 2018
>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:/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;

>Release-Note:

>Audit-Trail:
From: "Christos Zoulas" <christos@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/49386 CVS commit: src/sys/net
Date: Wed, 14 Oct 2015 15:40:10 -0400

 Module Name:	src
 Committed By:	christos
 Date:		Wed Oct 14 19:40:09 UTC 2015

 Modified Files:
 	src/sys/net: bpf.c

 Log Message:
 PR/49386: Ryota Ozaki: Add a mutex for bpf creation/removal to avoid races.
 Add M_CANFAIL to malloc.


 To generate a diff of this commit:
 cvs rdiff -u -r1.191 -r1.192 src/sys/net/bpf.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: open->closed
State-Changed-By: dholland@NetBSD.org
State-Changed-When: Sat, 16 Jun 2018 23:46:27 +0000
State-Changed-Why:
committed by christos in 2015


>Unformatted:

NetBSD Home
NetBSD PR Database Search

(Contact us) $NetBSD: query-full-pr,v 1.43 2018/01/16 07:36:43 maya Exp $
$NetBSD: gnats_config.sh,v 1.9 2014/08/02 14:16:04 spz Exp $
Copyright © 1994-2017 The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.