From owner-cvs-all@FreeBSD.ORG Thu Sep 15 20:56:37 2005 Return-Path: X-Original-To: cvs-all@freebsd.org Delivered-To: cvs-all@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 186AE16A41F; Thu, 15 Sep 2005 20:56:37 +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 7AADD43D45; Thu, 15 Sep 2005 20:56:36 +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 j8FKuYgx001400; Thu, 15 Sep 2005 23:56:34 +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 40719-12; Thu, 15 Sep 2005 23:56:33 +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 j8FKuOtK001389 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Thu, 15 Sep 2005 23:56:29 +0300 (EEST) (envelope-from ru@ip.net.ua) Received: (from ru@localhost) by heffalump.ip.net.ua (8.13.3/8.13.3) id j8FKudAs064689; Thu, 15 Sep 2005 23:56:39 +0300 (EEST) (envelope-from ru) Date: Thu, 15 Sep 2005 23:56:39 +0300 From: Ruslan Ermilov To: John Baldwin Message-ID: <20050915205639.GD88456@ip.net.ua> References: <200509151859.j8FIxY6v007639@repoman.freebsd.org> <200509151521.14204.jhb@FreeBSD.org> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="/unnNtmY43mpUSKx" Content-Disposition: inline In-Reply-To: <200509151521.14204.jhb@FreeBSD.org> 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 Subject: Re: cvs commit: src/sys/dev/re if_re.c X-BeenThere: cvs-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: CVS commit messages for the entire tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 15 Sep 2005 20:56:37 -0000 --/unnNtmY43mpUSKx Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Sep 15, 2005 at 03:21:12PM -0400, John Baldwin wrote: > On Thursday 15 September 2005 02:59 pm, Ruslan Ermilov wrote: > > ru 2005-09-15 18:59:34 UTC > > > > FreeBSD src repository > > > > Modified files: > > sys/dev/re if_re.c > > Log: > > re_detach() fixes: > > > > - Fixed if_free() logic screw-up that can either result > > in freeing a NULL pointer or leaking "struct ifnet". > > - Move if_free() after re_stop(); the latter accesses > > "struct ifnet". This bug was masked by a previous bug. > > - Restore the fix for a panic on detach caused by racing > > with BPF detach code by Bill by moving ether_ifdetach() > > after re_stop() and resetting IFF_UP; this got screwed > > up in revs. 1.30 and 1.36. >=20 > Device drivers should not modify IFF_UP. Instead, the interrupt handler= =20 > should be checking IFF_DRV_RUNNING rather than IFF_UP (foo_stop() clears= =20 > IFF_DRV_RUNNING). >=20 Ideally they shoudn't, yes. But there are at least two scenarios where resetting IFF_UP is currently used to attack bugs. 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. Another panic sometimes seen on SMP machines on shutdown is when foo_intr() is called after foo_shutdown() has already been called. Some drivers (if_re, if_vr, if_ath) attack this problem by clearing IFF_UP so that foo_intr() bails out quickly. I don't know if anybody diagnosed the roots of this problem, but I have the following idea: foo_shutdown() calls foo_stop() which disables device interrupts. After foo_stop() has freed resources but before interrupt has been teared down, another device that shares the same IRQ generated an interrupt, and foo_intr() is called again. It checks ISR register, and it has some interrupt active, and the code calls some function that accesses already freed memory. I don't have any real SMP hardware to play with, so the above is pure theoretical. Do you think this is possible? If so, checking IFF_DRV_RUNNING in foo_intr() should fix this. If we can also fix the "BPF detach MII tick reactivation" panic without involving resetting IFF_UP, all drivers can be cleaned up to not much with IFF_UP. Cheers, --=20 Ruslan Ermilov ru@FreeBSD.org FreeBSD committer --/unnNtmY43mpUSKx Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.1 (FreeBSD) iD8DBQFDKeAGqRfpzJluFF4RAqU9AJwMvHZoD2tN1+oKuRn0xglCqK4JaACfYQ10 rmr8m41YHBHtZfIysdpmQAY= =9Isy -----END PGP SIGNATURE----- --/unnNtmY43mpUSKx--