netdev
[Top] [All Lists]

Re: modular net drivers

To: Rusty Russell <rusty@xxxxxxxxxxxxxxxx>
Subject: Re: modular net drivers
From: Keith Owens <kaos@xxxxxxxxxx>
Date: Wed, 21 Jun 2000 07:49:40 +1000
Cc: "netdev@xxxxxxxxxxx" <netdev@xxxxxxxxxxx>, Donald Becker <becker@xxxxxxxxx>, Andrew Morton <andrewm@xxxxxxxxxx>
In-reply-to: Your message of "Tue, 20 Jun 2000 17:01:56 +1000." <20000620070156.D030D817F@halfway>
Sender: owner-netdev@xxxxxxxxxxx
On Tue, 20 Jun 2000 17:01:56 +1000, 
Rusty Russell <rusty@xxxxxxxxxxxxxxxx> wrote:
>Keith Owens wrote:
>> It is also an important bug fix.  The module code has suffered from
>> unload races ever since the kernel locking became fine grained, users
>> can crash the kernel.
>
>Races which can be largely solved at the moment by having the module
>page removal code sync all bh's and softirqs after calling cleanup().
>Hell, we could even poll all CPUs and check they're not executing in
>the about-to-be-freed pages.  Speed is completely unimportant here.

This race is not obvious but IMHO it exists.  The original theory was 

  Kernel load and unload code runs under the big kernel lock.
  open() and similar code runs under the big kernel lock.
  If the code does MOD_INC_USE_COUNT before sleeping then we are safe.

But consider this race, even on UP.

  Module has been used, nothing is currently using it, use_count == 0.
  rmmod runs, either manual or autoclean.
  The module is marked as being deleted.
  module_cleanup() is entered, does I/O, sleeps, loses big kernel lock.
  open() is entered, calls the module code, does MOD_INC_USE_COUNT.
  open() code in module sleeps, loosing big kernel lock.
  module_cleanup() resumes, gets big kernel lock, unloads the module.
  open() code in module resumes - the code pages have gone.

Checking in module_cleanup() to see if the use count has changed is not
a solution.  module_cleanup() may already have destroyed structures
that the open() code expects to use, either immediately or later.

Polling bh and softirq is not the answer either.  While the code sleeps
it is in schedule() - the only thing that says a process is in a module
is some return address 4+ layers up the stack!  Linus suggested only
allowing module unload when all processors where idle but that has the
same problem - schedule is idle.

AFAICT the only safe mechanism is one that checks the module state
*before* entering the module.  Once you enter the module and sleep all
bets are off.  And that means exporting the module information to the
open() layer, which is what Al Viro has been doing.


<Prev in Thread] Current Thread [Next in Thread>