On Mon, Dec 1, 2014 at 7:59 PM, Ryota Ozaki <[email protected]> wrote: > Hi, > > 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 > if_alloc_sadl > - 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. > > [before] > if_attach(); > if_alloc_sadl(); > > [after] > if_init(); > if_alloc_sadl(); > if_attach(); > > 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 ozaki-r
