Re: [RFC PATCH 0/3] sysfs: allow suicide

[Tejun Heo <htejun@gmail.com>]

Hello,

Eric W. Biederman wrote:
> Tejun Heo <htejun@gmail.com> writes:
> 
>> Thanks for the points.  I do agree that it could be a bit too clever,
>> but the thing is that protecting the code area from going underneath
>> something is a pretty special thing to begin with and I think it's
>> better to apply special solution rather than trying to work around it
>> using general mechanisms.  So, I actually think the global inhibit
>> thing is one of the better ways to deal with the nasty-in-nature
>> problem.
> 
> Protecting the data structures from going away is just as important,
> and the module_inhibit does not address that.

Yeap, I was talking about the code issue only.

> When I looked at it I could not see any touches of kobj in the sysfs
> code after we dropped the reference count in a strange place, but
> I haven't been able to convince myself that we will be safe.

The reference is dropped when the suiciding thread calls delete on the
sysfs node.  It forfeits its right to access the object when it
deletes it, which makes sense.  The things which are guaranteed after
deleting the base object are the code it's running off of and the
sysfs object itself.  I think it's pretty intuitive from user's POV.

> My view is that this is a general hotplug problem and not a sysfs
> problem.  Further I see inhibiting module reload as only solving
> have the problem as dropping the kobject reference opens a window to
> a use after free on the kobj.

kobject_del(obj); obj->whatever; isn't any different from kfree(p);
*p;.  If the caller accesses the object after deleting it, it's gonna
fail unless it already held a separate reference count.  There is no
window.

> The problem that I see is that we are missing support from the device
> model for hotunplug.  Running the device remove method from process
> context is required.  Typically hotplug controllers discover a
> device has been removed or will be removed in interrupt context.
> 
> Therefore every hotplug driver I have looked at has it's own workqueue
> to solve the problem of getting the notification of a hotplug event
> from an inappropriate context.
> 
> So the general problem that I see is that I need a solution to trigger
> a remove from interrupt context and that same solution will happen to
> work just fine for sysfs.

I think the problem is more driver domain specific and not quite sure
whether one size would fit all.  We have a lot of drivers in the tree.
I think the best approach would be trying to move upwards from the
bottom.  ie. Consolidate hotplug / error handling support from low
level drivers to specific driver subsystem, from driver subsystems to
higher layer (ie. block layer) and then see whether there can be more
commonalities which can be factored, but the chance is that once
things are pushed upwards enough, moving it into the kobject layer
probably wouldn't worth the trouble.  Well, it's all speculations at
this point tho.

Thanks.

-- 
tejun