[Top] [All Lists]

Re: modular net drivers, take 2

To: Keith Owens <kaos@xxxxxxxxxx>
Subject: Re: modular net drivers, take 2
From: Andrew Morton <andrewm@xxxxxxxxxx>
Date: Wed, 21 Jun 2000 01:18:43 +0000
Cc: "netdev@xxxxxxxxxxx" <netdev@xxxxxxxxxxx>
References: Your message of "Tue, 20 Jun 2000 17:38:57 +0200." <> <4490.961537968@ocs3.ocs-net>
Sender: owner-netdev@xxxxxxxxxxx
Keith Owens wrote:
> On Tue, 20 Jun 2000 17:38:57 +0200,
> Andi Kleen <ak@xxxxxx> wrote:
> >On Tue, Jun 20, 2000 at 02:04:35PM +0200, Andrew Morton wrote:
> >> - sys_ioctl() and sys_delete_module() both already claim
> >>   the big lock, so where's the race anyway?  I feel I'm missing
> >>   something..
> >I guess there are some time critical ioctls that should be run outside
> >kernel lock though. It is far too late to audit them all though.
> ioctls are not a problem, as long as they use a file descriptor, i.e.
> no global ioctls.  Getting a file descriptor requires open() or its
> equivalent which set the module use_count.  The race is in open, I
> don't know of any races after use_count is set and open() has complete
> and left the module.

I don't think you're right here, Keith.

ioctls on the netdevice don't require a descriptor which is associated
with dev->open().  For example, Donald's mii-diag application and
ifconfig both call device-specific functions without ever having called

modprobe driver
mii-diag -v eth0
ifconfig -a

In this example, both dev->ioctl() and dev->get_stats() are called while
the module refcount is zero.   So they're as risky as open(); these code
paths need to be audited for races wrt kmalloc->schedule()

How about a totally different approach:

In the module_exit() we locate all netdevices associated with this
module and overwrite all their function pointers with the addresses of
non-modular stub functions which return ENODEV.  Then we don't have to
worry about device methods being called after unload.

The PCI code does it for us; not sure about non-PCI device management

in xxx_probe1():
        dev->owner = THIS_MODULE;       /* Sorry, Rusty */

in xx_remove1():

        dev->open = err_open;
        dev->start_xmit = err_start_xmit;

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