[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
Re: struct ifnet and ifaddr handling [was: Re: Making global variables of if.c MPSAFE]
On Mon, Dec 1, 2014 at 7:59 PM, Ryota Ozaki <ozaki-r%netbsd.org@localhost> wrote:
> I found an defect of ifnet object initializations.
> - if_attach and if_alloc_sadl is called in each
> interface XXXattach function
> - if_alloc_sadl may be called via XXX_ifattach
> (e.g., ether_ifattach)
> - if_attach initializes an ifnet object, but
> the initialization is incomplete and if_alloc_sadl
> does the rest
> - for example, ifp->if_dl and ifp->if_sadl
> - if_attach enables entering ioctl (doifioctl)
> - ioctl can be called between if_attach and
> - ioctl may touch uninitialized member values
> of ifnet
> - then boom
> - sysctl also has the same issue
> We have to prevent ioctl from being called until
> if_alloc_sadl is finished somehow. We can achieve
> the goal by postponing enabling ioctl or registering
> an ifnet object to ifnet_list until if_alloc_sadl
> is finished. To this end, we need to call something
> after if_alloc_sadl.
> My approach is to split if_attach into if_init and
> if_attach; put initializations of if_attach into
> if_init and put registrations (e.g., adding a ifnet
> object to ifnet_list and sysctl setups) into if_attach.
> Then, call new if_attach after if_alloc_sadl so that
> we can ensure ioctl and sysctl are called after all
> initializations done.
> A concern of the approach is that it requires
> modifications to every places where if_attach
> is used (i.e., if_*).
> Here is a patch: http://www.netbsd.org/~ozaki-r/if_attach.diff
> (This patch includes if_attach modifications of only some if_*,
> not all, yet.)
> Any comments?
I've changed my mind. Changing every drivers isn't feasible at this
point. So I keep if_attach and add new two API, if_initialize
and if_register. if_attach just calls the two so that users of
if_attach works as it used to be. I'll change only some drivers that
I need, to use the new APIs. That's enough to me for now.
So here is a new patch: http://www.netbsd.org/~ozaki-r/if_attach.diff
Main Index |
Thread Index |