From owner-svn-src-head@FreeBSD.ORG Fri May 2 21:09:08 2014 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 8B8B7A7F; Fri, 2 May 2014 21:09:08 +0000 (UTC) Received: from mail.ipfw.ru (mail.ipfw.ru [IPv6:2a01:4f8:120:6141::2]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 231991B00; Fri, 2 May 2014 21:09:08 +0000 (UTC) Received: from secured.by.ipfw.ru ([95.143.220.47] helo=ws.su29.net) by mail.ipfw.ru with esmtpsa (TLSv1:AES128-SHA:128) (Exim 4.76 (FreeBSD)) (envelope-from ) id 1WgGnz-000592-49; Fri, 02 May 2014 20:59:23 +0400 Message-ID: <5364093A.7040607@FreeBSD.org> Date: Sat, 03 May 2014 01:08:10 +0400 From: "Alexander V. Chernikov" User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:24.0) Gecko/20100101 Thunderbird/24.4.0 MIME-Version: 1.0 To: Alan Somers Subject: Re: svn commit: r265232 - head/sys/net References: <201405021624.s42GO9Hi034947@svn.freebsd.org> <5363CF6C.2000305@FreeBSD.org> In-Reply-To: X-Enigmail-Version: 1.6 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Cc: "svn-src-head@freebsd.org" , "svn-src-all@freebsd.org" , "src-committers@freebsd.org" X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.17 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 02 May 2014 21:09:08 -0000 On 03.05.2014 00:22, Alan Somers wrote: > On Fri, May 2, 2014 at 11:01 AM, Alexander V. Chernikov > wrote: >> On 02.05.2014 20:24, Alan Somers wrote: >>> >>> Author: asomers >>> Date: Fri May 2 16:24:09 2014 >>> New Revision: 265232 >>> URL: http://svnweb.freebsd.org/changeset/base/265232 >>> >>> Log: >>> Fix a panic caused by doing "ifconfig -am" while a lagg is being >>> destroyed. >>> The thread that is destroying the lagg has already set sc->sc_psc=NULL >>> when >>> the "ifconfig -am" thread gets to lacp_req(). It tries to dereference >>> sc->sc_psc and panics. The solution is for lacp_req() to check the >>> value of >>> sc->sc_psc. If NULL, harmlessly return an lacp_opreq structure full of >>> zeros. Full details in GNATS. >> >> Sorry, it looks like I've missed -net@ discussion. > > Thanks for the retroactive review; it's good too. > >> >> While this patch minimizes panics, they still can occur. >> If one thread performs lagg_clone_destroy() -> lagg_lacp_detach() (1) -> >> lacp_detach(), executing >> >> struct lacp_softc *lsc = LACP_SOFTC(sc); (e.g. one line before sc_psc = NULL >> assignment) >> >> At the same moment, another thread (initiated by ifconfig) executes >> >> >> struct lacp_softc *lsc = LACP_SOFTC(sc); >> >> Then, thread #1 goes further to >> >> LACP_LOCK_DESTROY(lsc); >> free(lsc, M_DEVBUF); >> >> After that, thread #2 performs >> >> bzero(req, sizeof(struct lacp_opreq)); >> >> passes lsc check for NULL and crashes on destroyed mutex. >> >> >> Briefly looking, we can remove WUNLOCK() before lacp_detach() in >> lagg_lacp_detach() (I'm unsure about the reasons why do we need unlock >> here). >> lacp_req() is already protected by at least LAGG_RLOCK() so we should get >> consistent view of sc->sc_psc. > > The WUNLOCK was added in r168561 without comment. Removing it seems > like it would work. It doesn't cause any new LORs or WITNESS > warnings. And I can no longer reproduce the panic in lacp_req. > However, it doesn't fix another panic in lagg_port_ioctl line 825 (my > patch didn't either). Do you have any good ideas what to do about > that? Interesting. Line numbers look a bit shifted. Is line 825 'LAGG_RUNLOCK(sc, &tracker)' ? Are the steps to reproduce it the same as in kern/189003? > > #9 0xffffffff808cee81 in __mtx_lock_sleep (c=0xfffff800052bb648, > tid=18446735277704396800, opts=, > file=, line=) > at /usr/home/alans/freebsd/head/sys/kern/kern_mutex.c:430 > #10 0xffffffff808ced02 in __mtx_lock_flags (c=, opts=0, > file=0xffffffff80f6dd8c > "/usr/home/alans/freebsd/head/sys/kern/kern_rmlock.c", line=407) at > /usr/home/alans/freebsd/head/sys/kern/kern_mutex.c:223 > #11 0xffffffff808e0032 in _rm_rlock (rm=0xfffff800052bb608, > tracker=0xfffffe0097751918, trylock=) > at /usr/home/alans/freebsd/head/sys/kern/kern_rmlock.c:407 > #12 0xffffffff808e0812 in _rm_rlock_debug (rm=0xfffff800052bb608, > tracker=0xfffffe0097751918, trylock=0, > file=0xffffffff81c1dd13 > "/usr/home/alans/freebsd/head/sys/modules/if_lagg/../../net/if_lagg.c", > line=825) > at /usr/home/alans/freebsd/head/sys/kern/kern_rmlock.c:659 > #13 0xffffffff81c19f62 in lagg_port_ioctl (ifp=0xfffff8000590d800, > cmd=, data=0xfffff80005582c00 "tap0") > at /usr/home/alans/freebsd/head/sys/modules/if_lagg/../../net/if_lagg.c:825 > #14 0xffffffff809ae407 in ifioctl (so=, cmd=3225971084, > data=0xfffff80005582c00 "tap0", td=) > at /usr/home/alans/freebsd/head/sys/net/if.c:2643 > #15 0xffffffff8093b9fb in kern_ioctl (td=, > fd=, com=) at file.h:323 > #16 0xffffffff8093b77c in sys_ioctl (td=0xfffff800053cc000, > uap=0xfffffe0097751b80) > at /usr/home/alans/freebsd/head/sys/kern/sys_generic.c:702 >