Re: struct ifnet and ifaddr handling [was: Re: Making global variables of if.c MPSAFE]

[Ryota Ozaki <ozaki-r%netbsd.org@localhost>]

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