[openib-general] Re: [PATCH] fix umad object lifetime stuff

[Sean Hefty]

Roland Dreier wrote:
> I just committed the following patch for user_mad.c, which fixes
> various issues with possibly freeing various data structures before
> the last reference is gone.  For example, cdev_del() might return
> before the last reference to the cdev is gone, so freeing a structure
> containing the cdev is wrong at that point.  (Side note: it's
> essentially impossible to use cdev_init() safely unless the cdev in
> question is statically allocated as part of the module).
> 
> Something like this is probably required for ucm and anything else
> that exports a character device, since everyone seems to have copied
> my bad user_mad code.  But I haven't had a chance to do anything
> beyond user_mad and uverbs so far...

I'm just now getting back to looking at this issue.  If I understand the problem 
in the ucm correctly, struct cdev is freed as part of struct ib_ucm_device after 
cdev_del() returns; however, a user could still have a reference on the cdev. 
Also, the user could still make calls into the driver.  Is this correct?

If this is the case, isn't more protection needed that simply preventing access 
to cdev?  I.e. what prevents the user from invoking a call that tries to access 
the underlying ib_device?  Does every file operation need synchronization with 
device removal to ensure that the underlying hardware is still there?  (This 
appears to be what user_mad now does.)

Assuming that my understanding is correct (which is a stretch), it seems that 
there has to be a better way to handle this that is or can be integrated with 
the kernel, rather than adding complex reference counting, synchronization, and 
clean-up code to every driver that wants to handle device removal...

- Sean