Re: Are modunload and ifconfig create racy?

[Kengo NAKAHARA <k-nakahara%iij.ad.jp@localhost>]

Hi,

On 2017/01/21 6:47, Paul Goyette wrote:
> (Replying _without_ looking at the code!  I can look more deeply if you 
> could re-send a pointer to the code...)

In the example of gre(4) interface, if CPU#0 do "modunload if_gre" and
CPU#1 do "ifconfig gre0 create", they seems to have below race as far as
if_gre.c.
    + CPU#1
      - call gre_clone_create()
        - before done gre_count increment, that is before line 369
          https://nxr.netbsd.org/xref/src/sys/net/if_gre.c#327
    + CPU#0
      - call gredetach()
        - done gre_count check, that is, reached line 1464
          https://nxr.netbsd.org/xref/src/sys/net/if_gre.c#1464
    + CPU#1
      - continue gre_clone_create()
        - there is no checking code if gredetach() is being called or not
          in gre_clone_create(), so continue
At this point, CPU#0 continue to modunload processing while CPU#1 continue
to ifconfig create. What will happen? I think fault would arise and panic
in CPU#1 at some point.


> Generally, the module's unload code should detach the devsw entry for 
> the driver first, to prevent any new accesses.
> 
> Then, the config_fini_component() call will fail if existing devices 
> cannot be detached.  If this does fail, the module unload code should 
> re-attach the devsw entry and return failure.

The pseudo interface modules' load/unload code is generated by IF_MODULE
macro
    https://nxr.netbsd.org/xref/src/sys/net/if_module.h#54

The unload code seems to call each inteface's detach code(such as gredetach()),
and then, the code calls config_cfdriver_detach()(not config_fini_component()).
Hmm, isn't this general way?


> The only chance I see for any race condition is if the open/create logic 
> can clone a new entry which is not seen by config_cfdata_detach() when 
> it scans for device instances which need to be detached.
> 
> If there is a race condition, it should be solved in each interface's 
> driver module, or within the autoconfig framework.  Each driver module 
> is responsible for managing its own integration with autoconfig (ie, 
> calling config_{init,fini}_component and devsw_{att},det}ach routines).

I think the race condition should solved in IF_MODULE macro. However,
I don't have an appropriate solution. It may be required to modify
if_clone_create()...


Thanks,

-- 
//////////////////////////////////////////////////////////////////////
Internet Initiative Japan Inc.

Device Engineering Section,
IoT Platform Development Department,
Network Division,
Technology Unit

Kengo NAKAHARA <k-nakahara%iij.ad.jp@localhost>