Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 14 Nov 2005 15:36:16 +0000 (GMT)
From:      wpaul@FreeBSD.ORG (Bill Paul)
To:        ru@FreeBSD.org (Ruslan Ermilov)
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
Message-ID:  <20051114153616.DE50A16A421@hub.freebsd.org>
In-Reply-To: <20051114141420.GW87446@ip.net.ua> from Ruslan Ermilov at "Nov 14, 2005 04:14:20 pm"

next in thread | previous in thread | raw e-mail | index | archive | help
> Hi Bill,
> 
> On Sun, Nov 13, 2005 at 09:23:45PM +0000, Bill Paul wrote:
> > [...]
> > 
> > >     sys/compat/ndis      subr_ndis.c 
> > 
> > [...]
> > 
> > >   - 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.
> > >   
> > >   - 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.
> > 
> > For the umpity umpth time: I'm trying to keep it so that the same
> > NDIS code will compile on 7.x, 6.x and 5.x. Having one set of code
> > means I don't need to go nuts doing merges from one branch to the
> > next: I can just pull the files over and be done with it.
> > 
> Understood.
> 
> > You did
> > not consider this when modifying subr_ndis.c, and now you have broken
> > it for the 6.x and 5.x branches.
> > 
> Yes I did.
> 
> > More importantly, you broke it in
> > a very subtle way that only shows up as a kernel panic a runtime.
> > 
> Can you please give me more details about this panic?

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.

-Bill

--
=============================================================================
-Bill Paul            (510) 749-2329 | Senior Engineer, Master of Unix-Fu
                 wpaul@windriver.com | Wind River Systems
=============================================================================
              <adamw> you're just BEGGING to face the moose
=============================================================================



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20051114153616.DE50A16A421>