Message-ID: <39503AD1.99A54D57@uow.edu.au>
Date:   Wed, 21 Jun 2000 03:47:29 +0000
From:   Andrew Morton <andrewm@uow.edu.au>
MIME-Version: 1.0
To:     Donald Becker <becker@scyld.com>
CC:     "netdev@oss.sgi.com" <netdev@oss.sgi.com>
Subject: Re: modular net drivers, take 2
References: <395017F3.516165AD@uow.edu.au> <Pine.LNX.4.10.10006202140210.26261-100000@vaio.greennet>
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
Sender: owner-netdev@oss.sgi.com
Precedence: bulk

Donald Becker wrote:
> 
> On Wed, 21 Jun 2000, Andrew Morton wrote:
> 
> > 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.
> 
> Hmmm, now this is an important point.
> 
> I just "know" that ioctl() and get_stats() should be "simple" functions that
> shouldn't do call any kernel function, except for perhaps printk().  This is
> not documented in the skeleton driver, or elsewhere.
> 
> I'm guessing that the documentation should advise locking the module if any
> operation is done in get_stats() or private_ioctl() that might result in a
> reschedule.

Yes, these functions tend to be atomic in-and-out.  But there are
opportunities for module unload prior to them even being called.

The call path appears to be:

sys_ioctl()
  lock_kernel()
  -> fd.file_operations.ioctl()  (sock_ioctl())
       unlock_kernel()
       -> sock.proto_ops.ioctl() (inet_ioctl() for af_inet)
            -> dev_ioctl()
               dev_load()
                 -> dev_ifsioc()
                      __dev_get_by_name() (Fails if unloaded)
                      netif_device_present()
                        -> dev->ioctl()

So there are some small SMP-only opportunities for the module
to be unloaded prior to netdevice.do_ioctl() even being called.

This is getting sticky, isn't it?

[ Yipes.  3c527.c does a sleep_on() when the module refcount is zero ]