Message-ID: <395017F3.516165AD@uow.edu.au>
Date:   Wed, 21 Jun 2000 01:18:43 +0000
From:   Andrew Morton <andrewm@uow.edu.au>
MIME-Version: 1.0
To:     Keith Owens <kaos@ocs.com.au>
CC:     "netdev@oss.sgi.com" <netdev@oss.sgi.com>
Subject: Re: modular net drivers, take 2
References: Your message of "Tue,
            20 Jun 2000 17:38:57 +0200." <20000620173857.A4089@fred.muc.de> <4490.961537968@ocs3.ocs-net>
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
Sender: owner-netdev@oss.sgi.com
Precedence: bulk

Keith Owens wrote:
> 
> On Tue, 20 Jun 2000 17:38:57 +0200,
> Andi Kleen <ak@muc.de> 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
dev->open():

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()
opportunities.


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
though.

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

in xx_remove1():
	zap_netdevice(dev);

zap_netdevice(dev)
{
	dev->open = err_open;
	dev->start_xmit = err_start_xmit;
	etc
|