On Wed, Aug 10, 2005 at 03:35:53PM -0700, Greg KH wrote:
> On Wed, Aug 10, 2005 at 03:36:36PM +0530, Maneesh Soni wrote:
> > On Wed, Aug 10, 2005 at 04:26:51PM +1000, Keith Owens wrote:
> > > FYI, the intermittent free after use in sysfs is still there in
> > > 2.6.13-rc6.
> > >
> >
> > The race condition is known here. It is some thing in the upper layer.
> > In this case "driver/base/class.c" which frees the kobject's attributes
> > even if there are live references to kobject.
> >
> >
> > open sysfs file unregister class device
> > sysfs_open_file() class_device_del()
> > -> takes a ref on kobject -> kfree attribute struct
> > -> accesses attributes -> kobject_del()
> > -> kref_put()
> > close sysfs file
> > sysfs_release()
> > -> acesses attributes using s_element
> > -> drops ref to kobject
> >
> > Solution could be either we have reference counting for attributes also
> > or keep attributes alive till the last reference to the kobject. Both these
> > needs changes in the driver core.
> >
> > Greg, will the following patch make sense? This postpones the kfree() of
> > devt_attr till class_dev_release() is called.
>
> Yes, that patch looks good, if you fix up the space vs. tabs issue :)
>
yuk.. sorry for that.. I corrected that now.
> But will that really fix this race? I was under the impression the oops
> didn't come from trying to access the devt_attr, but the sysfs s_element
> pointer?
I am not able to recreate the race with or without the patch though. As per
the stack traces, and the debug messages from antother thread also,
I could see that it is use after free for attribute structure. s_element
is a pointer to the attribute structure in case of sysfs "files". Most of
the times this attribute structure is not allocated/freed dynamically unlike
/sys/class/vc/vcs*/dev, so we don't see any crashes.
> > Please check this patch out, if this helps or not.
>
> I'd be interested in seeing if this fixes it.
>
I just cc-ed others also who reported similar oops.
Thanks
Maneesh
o following patch moves the code to free devt_attr from class_device_del()
to class_dev_release() which is called after the last reference to the
corresponding kobject() is gone. This allows to keep the devt_attr
alive while the corresponding sysfs file is open.
Signed-off-by: Maneesh Soni <[EMAIL PROTECTED]>
---
linux-2.6.13-rc6-maneesh/drivers/base/class.c | 14 +++++++++-----
1 files changed, 9 insertions(+), 5 deletions(-)
diff -puN drivers/base/class.c~fix-class-attributes-race drivers/base/class.c
--- linux-2.6.13-rc6/drivers/base/class.c~fix-class-attributes-race
2005-08-10 12:35:06.000000000 +0530
+++ linux-2.6.13-rc6-maneesh/drivers/base/class.c 2005-08-11
09:27:17.202081088 +0530
@@ -299,6 +299,11 @@ static void class_dev_release(struct kob
pr_debug("device class '%s': release.\n", cd->class_id);
+ if (cd->devt_attr) {
+ kfree(cd->devt_attr);
+ cd->devt_attr = NULL;
+ }
+
if (cls->release)
cls->release(cd);
else {
@@ -591,11 +596,10 @@ void class_device_del(struct class_devic
if (class_dev->dev)
sysfs_remove_link(&class_dev->kobj, "device");
- if (class_dev->devt_attr) {
- class_device_remove_file(class_dev, class_dev->devt_attr);
- kfree(class_dev->devt_attr);
- class_dev->devt_attr = NULL;
- }
+
+ if (class_dev->devt_attr)
+ class_device_remove_file(class_dev, class_dev->devt_attr);
+
class_device_remove_attrs(class_dev);
kobject_hotplug(&class_dev->kobj, KOBJ_REMOVE);
_
--
Maneesh Soni
Linux Technology Center,
IBM India Software Labs,
Bangalore, India
email: [EMAIL PROTECTED]
Phone: 91-80-25044990
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
