From owner-svn-src-all@FreeBSD.ORG Fri Apr 20 08:57:47 2012 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id E18C9106566B; Fri, 20 Apr 2012 08:57:47 +0000 (UTC) (envelope-from melifaro@ipfw.ru) Received: from mail.ipfw.ru (unknown [IPv6:2a01:4f8:120:6141::2]) by mx1.freebsd.org (Postfix) with ESMTP id 7481A8FC08; Fri, 20 Apr 2012 08:57:47 +0000 (UTC) Received: from [2a02:6b8:0:401:222:4dff:fe50:cd2f] (helo=dhcp170-36-red.yandex.net) by mail.ipfw.ru with esmtpsa (TLSv1:CAMELLIA256-SHA:256) (Exim 4.76 (FreeBSD)) (envelope-from ) id 1SL9f7-0004Sc-OK; Fri, 20 Apr 2012 12:57:53 +0400 Message-ID: <4F91240C.3050703@ipfw.ru> Date: Fri, 20 Apr 2012 12:53:32 +0400 From: "Alexander V. Chernikov" User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:8.0) Gecko/20111117 Thunderbird/8.0 MIME-Version: 1.0 To: Adrian Chadd References: <201204060653.q366rwLa096182@svn.freebsd.org> <4F7E9413.20602@FreeBSD.org> <4F8BBD4E.1040106@FreeBSD.org> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: svn-src-head@freebsd.org, freebsd-net@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r233937 - in head/sys: kern net security/mac X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 20 Apr 2012 08:57:48 -0000 On 17.04.2012 01:29, Adrian Chadd wrote: > On 15 April 2012 23:33, Alexander V. Chernikov wrote: >> On 16.04.2012 01:17, Adrian Chadd wrote: >>> >>> Hi, >>> >>> This has broken (at least) net80211 and bpf, with LOR: >> >> Yes, it is. Please try the attached patch > > Hi, Hello! Sorry for the late reply, answering for both letters. > > This seems like a very, very complicated diff. > > * You've removed BPF_LOCK_ASSERT() inside bpf_detachd_locked() - why'd > you do that? > * You removed a comment ("We're already protected by the global lock") > which is still relevant/valid Both should be added back, thanks. > * There are lots of modifications to the read/write locks here - I'm > not sure whether they're at all relevant to my immediate problem and > may belong in separate commits Most of the patch is not directly relevant to the problem. It solves several new problems and a bunch of very old bugs due to lack of locking. > > Is there a document somewhere which describes what the "new" style BPF > locking should be? Are there any other places (except src) where such documentation should reside? > > I "just" added BPF_LOCK() / BPF_UNLOCK() around all the calls to > bpf_detachd() which weren't locked (there were a few.) Unfortunately, this is not enough. There is possibility that bpf_setif() is called immediately before rw_destroy() in bpfdetach(). For example, you can easily trigger panic on any 8/9/current SMP system with 'while true; do ifconfig vlan222 create vlan 222 vlandev em0 up ; tcpdump -pi vlan222 & ; ifconfig vlan222 destroy ; done' There is also possible use-after-free for bpfif structure (since we're freeing it _before_ interface routes are cleaned up). This is why delayed free is needed. > > One final question - should the BPF global lock be recursive? It seems it really should be recursive now. > > thanks, > > > > Adrian >