From owner-cvs-src@FreeBSD.ORG Mon Nov 14 18:39:13 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 3159116A420; Mon, 14 Nov 2005 18:39:13 +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 A763843D67; Mon, 14 Nov 2005 18:38:58 +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 jAEIcu6S079426; Mon, 14 Nov 2005 20:38:57 +0200 (EET) (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 66181-01-3; Mon, 14 Nov 2005 20:38:55 +0200 (EET) 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 jAEIc91R079290 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Mon, 14 Nov 2005 20:38:09 +0200 (EET) (envelope-from ru@ip.net.ua) Received: (from ru@localhost) by heffalump.ip.net.ua (8.13.4/8.13.4) id jAEIcHMK055949; Mon, 14 Nov 2005 20:38:17 +0200 (EET) (envelope-from ru) Date: Mon, 14 Nov 2005 20:38:16 +0200 From: Ruslan Ermilov To: Bill Paul Message-ID: <20051114183816.GX87446@ip.net.ua> References: <20051114141420.GW87446@ip.net.ua> <20051114153616.DE50A16A421@hub.freebsd.org> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="S5Rg6oz6PtEXgjQf" Content-Disposition: inline In-Reply-To: <20051114153616.DE50A16A421@hub.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/net if.c if_arcsubr.c if_arp.h if_bridge.c if_ef.c if_ethersubr.c if_fddisubr.c if_fwsubr.c if_i 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: Mon, 14 Nov 2005 18:39:13 -0000 --S5Rg6oz6PtEXgjQf Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Nov 14, 2005 at 03:36:16PM +0000, Bill Paul wrote: > > On Sun, Nov 13, 2005 at 09:23:45PM +0000, Bill Paul wrote: > > > [...] > > >=20 > > > > sys/compat/ndis subr_ndis.c=20 > > >=20 > > > [...] > > >=20 > > > > - Store pointer to the link-level address right in "struct ifnet" > > > > rather than in ifindex_table[]; all (except one) accesses are > > > > through ifp anyway. IF_LLADDR() works faster, and all (except > > > > one) ifaddr_byindex() users were converted to use ifp->if_addr. > > > > =20 > > > > - Stop storing a (pointer to) Ethernet address in "struct arpcom", > > > > and drop the IFP2ENADDR() macro; all users have been converted > > > > to use IF_LLADDR() instead. > > >=20 [...] > > > More importantly, you broke it in > > > a very subtle way that only shows up as a kernel panic a runtime. > > >=20 > > Can you please give me more details about this panic? >=20 > I already fixed it, but to see it, take the previous version of > subr_ndis.c (with your change) and compile it on a 6.0 system, then > try loading an NDIS driver. The problem is that IF_LLADDR() exists > on earlier versions of FreeBSD, but using it on 6.0 leads to a > NULL pointer dereference in NdisReadNetworkAddress() at driver > load time. On 6.0, you have to continue using IFP2ENADDR() anyway > (until this change is merged). I can only assume that on 6.x, > using IF_LLADDR() in this circumstance references a pointer that > hasn't been initialized yet. >=20 Yes. I've been able to reproduce it on 7.0 too, independently. The problem is that (at least) this function accesses "struct ifnet" before it has been fully initialized by if_attach(), through e.g. a call to ether_ifattach(), where the first (AF_LINK) address is added to the interface address list. The effect of using IF_LLADDR() at this unfortunate time is that in all of 5.x, 6.x, and 7.0 it will dereference a null pointer. I've just committed a fix for this to HEAD after playing with an NDIS driver for RTL8139 pccard and confirming and fixing the problem. If it's possible to avoid using "struct ifnet" before it's if_attached()'ed and initialized (not sure), it would best. If not, there should be a branch-compatible way to tell if an address list has been initialized; then IF_LLADDR() can be used anywhere it's defined instead of IFP2ENADDR(). Moreover, I'm not sure, but maybe a check against an "empty" MAC address is redundant now, you should know better. ;) Cheers, --=20 Ruslan Ermilov ru@FreeBSD.org FreeBSD committer --S5Rg6oz6PtEXgjQf Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.2 (FreeBSD) iD8DBQFDeNmYqRfpzJluFF4RAutDAJ9zhbjEUyTDr8IehIut6QfUNG8lmACfbEBG HfpPoQ9vbi2DDzeGpxDQk6w= =UKuX -----END PGP SIGNATURE----- --S5Rg6oz6PtEXgjQf--