Re: BUG: Problem with your patches for sysfs from 2 weeks ago

[Greg KH]

On Mon, Mar 29, 2004 at 12:23:09PM +0530, Maneesh Soni wrote:
> On Sat, Mar 27, 2004 at 05:25:07PM -0500, Alan Stern wrote:
> > Maneesh:
> > 
> > I've been tracing a problem with sysfs that starting showing up just
> > recently, and it seems likely to have originated with your patches from a
> > couple of weeks ago.  I can't tell exactly what's wrong or fix it because
> > I don't understand the filesystem layer well enough.
> > 
> > The problem I found (there may be others) shows up when trying to delete a 
> > nonexistent symlink -- presumably trying to delete a nonexistent file 
> > would have a similar result.  The code in sysfs_hash_and_remove() does a 
> > lookup on the nonexistent name and sysfs_get_dentry() returns a 
> > newly-allocated dentry.  Creating the new entry increments the parent 
> > directory's d_count, of course.  But at the end of the routine, when 
> > dput() is called for the new dentry, the parent's d_count does _not_ get 
> > decremented.  The new dentry is placed on the dentry_unused list and the 
> > parent is left with an anomalously large d_count.  This doesn't ever seem 
> > to get resolved, and when the directory's kobject is deleted the reference 
> > you added doesn't get dropped.
> > 
> 
> I see the problem here. Probably same will happen even if a non-existent 
> regular file is deleted. Negative dentry and the ref. taken on the parent
> should go away eventually when there is a memory pressure. But the situation
> here needs immediate removal of the negative dentry.

Yes.

> This could be solved partially if sysfs_hash_and_remove() handles negative 
> dentries as well by unhashing them before calling dput(). 

Care to fix this?

> Partially because sysfs_hash_and_remove() is called only if some kernel 
> level code (drivers) issues sysfs_remove_file() or sysfs_create_file(). 
> But if a user just goes to the corresponding directory and does rm 
> for a non-existent file, I guess we will have the same situation. This needs
> some solution.

Agreed.

> The patch (sysfs-pin-kobject.patch) I did was required for oops 
> (Use after free) situations like this

Yes, I agree that it was needed, just that your fix now causes other
problems.  Actually these were hinted at in the previous objections to
this patch (the PCMCIA issues no one could figure out.)  Thanks to Alan,
we have now pinpointed the true cause.

So, as your fix for one problem caused another one, care to fix this
too?  :)

thanks,

greg k-h
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/