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?
Thanks,
ozaki-r
