From owner-cvs-src@FreeBSD.ORG Fri Sep 16 19:44:09 2005 Return-Path: X-Original-To: cvs-src@FreeBSD.org Delivered-To: cvs-src@FreeBSD.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 70BA116A41F; Fri, 16 Sep 2005 19:44:09 +0000 (GMT) (envelope-from ru@ip.net.ua) Received: from tigra.ip.net.ua (tigra.ip.net.ua [82.193.96.10]) by mx1.FreeBSD.org (Postfix) with ESMTP id CFE3743D45; Fri, 16 Sep 2005 19:44:08 +0000 (GMT) (envelope-from ru@ip.net.ua) Received: from localhost (rocky.ip.net.ua [82.193.96.2]) by tigra.ip.net.ua (8.12.11/8.12.11) with ESMTP id j8GJhqhG048565; Fri, 16 Sep 2005 22:43:52 +0300 (EEST) (envelope-from ru@ip.net.ua) Received: from tigra.ip.net.ua ([82.193.96.10]) by localhost (rocky.ipnet [82.193.96.2]) (amavisd-new, port 10024) with LMTP id 93757-18; Fri, 16 Sep 2005 22:43:52 +0300 (EEST) Received: from heffalump.ip.net.ua (heffalump.ip.net.ua [82.193.96.213]) by tigra.ip.net.ua (8.12.11/8.12.11) with ESMTP id j8GJhptt048562 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Fri, 16 Sep 2005 22:43:51 +0300 (EEST) (envelope-from ru@ip.net.ua) Received: (from ru@localhost) by heffalump.ip.net.ua (8.13.3/8.13.3) id j8GJi6C8068306; Fri, 16 Sep 2005 22:44:06 +0300 (EEST) (envelope-from ru) Date: Fri, 16 Sep 2005 22:44:05 +0300 From: Ruslan Ermilov To: "M. Warner Losh" Message-ID: <20050916194405.GB24879@ip.net.ua> References: <200509151521.14204.jhb@FreeBSD.org> <20050915205639.GD88456@ip.net.ua> <20050916091928.GG88456@ip.net.ua> <20050916.090140.58827157.imp@bsdimp.com> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="1UWUbFP1cBYEclgG" Content-Disposition: inline In-Reply-To: <20050916.090140.58827157.imp@bsdimp.com> User-Agent: Mutt/1.5.9i X-Virus-Scanned: by amavisd-new at ip.net.ua Cc: cvs-src@FreeBSD.org, src-committers@FreeBSD.org, cvs-all@FreeBSD.org, jhb@FreeBSD.org Subject: Re: cvs commit: src/sys/dev/re if_re.c X-BeenThere: cvs-src@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: CVS commit messages for the src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 16 Sep 2005 19:44:09 -0000 --1UWUbFP1cBYEclgG Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Sep 16, 2005 at 09:01:40AM -0600, M. Warner Losh wrote: > In message: <20050916091928.GG88456@ip.net.ua> > Ruslan Ermilov writes: > : On Thu, Sep 15, 2005 at 11:56:39PM +0300, Ruslan Ermilov wrote: > : > The first is the BPF detach bad interaction with foo_detach(), > : > as described in re_detach(). This panic is real with (I think) > : > all drivers. And testing IFF_DRV_RUNNING here doesn't seem to > : > be able to prevent the panic. Perhaps the fix would be to > : > move ether_ifdetach() before foo_stop() in foo_detach(), I'm > : > not yet sure. > : >=20 > : I tried with rl(4) PCCARD, by moving ether_ifdetach() before > : rl_stop() in rl_detach(). It fixes the panic when you eject > : the card, but doesn't fix it when kldunloading the module. > : The difference is that rl_detach() is called already after > : miibus0 and rlphy0 has been detached when kldunloading the > : module. When ejecting the card, rl_detach() is called first. > : What happens when you kldunload the module with BPF listener > : attached, is that bpfdetach() calls rl_ioctl() to reset > : promisc, that calls rl_init_locked(), and that results in > :=20 > : mii =3D device_get_softc(sc->rl_miibus); > :=20 > : being NULL (remember the miibus has already been detached), > : and that panics later here: > :=20 > : mii_mediachg(mii); > :=20 > : When we reset IFF_UP, rl_ioctl(SIOCSIFFLAGS) silently exits > : and no harm is done. So the question is: how do we prevent > : this from happening without resetting IFF_UP. One possible > : solution would be to add sc->detaching, similar to > : sc->suspended, abd check it in rl_ioctl(). >=20 > Ugg. In ed, we check to make sure that we still have a child before > doing things with mii bus. A similar fix could be made. >=20 No, ed(4) has the same problem: if (sc->miibus !=3D NULL) { struct mii_data *mii; mii =3D device_get_softc(sc->miibus); mii_mediachg(mii); } The device (sc->miibus) will still be there but already detached, and its softc will already be freed, so "mii" will be NULL, and mii_mediachg(NULL) will panic the system. xl(4), for example, also checks for "mii" to be not NULL: /* XXX Downcall to miibus. */ if (mii !=3D NULL) mii_mediachg(mii); Somehow I think it's plain incorrect to implicitly call start() after explicitly calling stop() from detach() -- except for possibly panicing the system, it also re-enables interrupts, re-schedules just cancelled callouts, etc. Cheers, --=20 Ruslan Ermilov ru@FreeBSD.org FreeBSD committer --1UWUbFP1cBYEclgG Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.1 (FreeBSD) iD8DBQFDKyCFqRfpzJluFF4RAtzYAKCI8FrGhA6bZllV0eVwrBlLPKDElgCeMncG y2MdRABLbL7BC6WsIKsggJY= =0AaR -----END PGP SIGNATURE----- --1UWUbFP1cBYEclgG--