Re: Flaw in the driver-model implementation of attributes

[Alan Stern]

On 15 Jun 2003, Jeremy Fitzhardinge wrote:

> On Sun, 2003-06-15 at 09:42, Alan Stern wrote:
> > If you're already aware of this, please forgive the intrusion.
> > 
> > There's a general problem in the driver model's implementation of 
> > attribute files, in connection with loadable kernel modules.  The 
> > sysfs_ops structure stores function pointers with no means for identifying 
> > the module that contains the corresponding code.  As a result, it's 
> > possible to call through one of these pointers even after the module has 
> > been unloaded, causing an oops.
> > 
> > It's not hard to provoke this sort of situation.  A user process can
> > open a sysfs device file, for instance, and delay trying to read it until 
> > the module containing the device driver has been removed.  When the read 
> > does occur, it runs into trouble.
> 
> I've seen this oops when a program has its cwd in a /sys directory
> corresponding to a removed (or replaced) module.  I think active
> references to a part of the /sys namespace corresponding to a module
> should just pin the module.  But I haven't looked into it really.

The question of which references should pin a module is a design decision 
that I prefer to leave to others.  But I would think that a reference the 
module can quickly and easily revoke probably shouldn't pin it, whereas 
one the module isn't even aware of probably should.

That's the difference between the example you gave and the problem I
cited.  In your example, the fact that your program parked its cwd in a
sysfs directory would force the reference count of the corresponding
kobject to be nonzero.  That the module was willing to deallocate the
kobject and exit even though the reference count was still positive is a
simple programming error.  But in the situation I described, the design of
the driver model makes it impossible for a driver to know whether any user
processes still have references to a device attribute file after
device_remove_file() has been called.

Alan Stern