Re: [PATCH] Kobjects: drop child->parent ref at unregistration
On Tue, 27 Nov 2007, Greg KH wrote: > Let me finish writing up this documentation on what I do remember and > then I'll attack the block device patch and this one. Be sure to CC: me when your documentation patch is sent out. Alan Stern - 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/
Re: [PATCH] Kobjects: drop child->parent ref at unregistration
On Tue, Nov 27, 2007 at 12:41:23PM -0500, Alan Stern wrote: > On Tue, 27 Nov 2007, Greg KH wrote: > > > For now, I'd recommend dropping it from your tree Andrew, as it might > > cause odd issues on device removal (not quite sure though...) > > But then there's the counter-argument: If the patch does cause any odd > issues to shake loose, learning about them in advance by exposing the > patch in the -mm tree would be worthwhile. > > Sounds like it's time for an executive decision... :-) Heh. I'm not ignoring the patch, and will apply it if I test it out and see that it doesn't break anything. Then I will let it bake in -mm for a while. I just don't want to rush it as this area of the kobject core is nasty and full of tricks that I always forget. Let me finish writing up this documentation on what I do remember and then I'll attack the block device patch and this one. thanks, greg k-h - 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/
Re: [PATCH] Kobjects: drop child->parent ref at unregistration
On Tue, 27 Nov 2007, Greg KH wrote: > For now, I'd recommend dropping it from your tree Andrew, as it might > cause odd issues on device removal (not quite sure though...) But then there's the counter-argument: If the patch does cause any odd issues to shake loose, learning about them in advance by exposing the patch in the -mm tree would be worthwhile. Sounds like it's time for an executive decision... :-) Alan Stern P.S.: I did test the patch by running the g_serial gadget with usbserial loaded on the host. Disconnecting the gadget while a user process held the /dev/ttyUSB0 file open didn't cause any problems. - 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/
Re: [PATCH] Kobjects: drop child->parent ref at unregistration
On Mon, Nov 26, 2007 at 09:29:36PM -0500, Alan Stern wrote: > On Mon, 26 Nov 2007, Andrew Morton wrote: > > > On Mon, 19 Nov 2007 10:53:40 -0500 (EST) > > Alan Stern <[EMAIL PROTECTED]> wrote: > > > > > This patch (as1015) reverts changes that were made to the driver core > > > about four years ago. The intent back then was to avoid certain kinds > > > of invalid memory accesses by leaving kernel objects allocated as long > > > as any of their children were still allocated. The original and > > > correct approach was to wait only as long as any children were still > > > _registered_; that's what this patch reinstates. > > > > What happened with this? > > As far as I know, it's on Greg's queue. Yes, it's in my queue still. Kay and Alan want the patch as it fixes the new /sys/block -> /sys/class/block patches. When I add that one back to my tree (Kay has debugged your old G5 problem now), I'll either add this patch too, or figure out why it should not be needed. For now, I'd recommend dropping it from your tree Andrew, as it might cause odd issues on device removal (not quite sure though...) thanks, greg k-h - 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/
Re: [PATCH] Kobjects: drop child->parent ref at unregistration
On Mon, 26 Nov 2007, Andrew Morton wrote: > On Mon, 19 Nov 2007 10:53:40 -0500 (EST) > Alan Stern <[EMAIL PROTECTED]> wrote: > > > This patch (as1015) reverts changes that were made to the driver core > > about four years ago. The intent back then was to avoid certain kinds > > of invalid memory accesses by leaving kernel objects allocated as long > > as any of their children were still allocated. The original and > > correct approach was to wait only as long as any children were still > > _registered_; that's what this patch reinstates. > > What happened with this? As far as I know, it's on Greg's queue. > > This fixes a problem in the SCSI core made visible by the class_device > > to regular device conversion: A reference loop (scsi_device holds > > reference to request_queue, which is the child of a gendisk, which is > > the child of the scsi_device) prevents the data structures from being > > released, even though they are deregistered okay. > > > > It's possible that this change will cause a few bugs to surface, > > things that have been hidden for several years. They can be fixed > > easily enough by having the child device take an explicit reference to > > the parent whenever needed. > > > > How will such bugs manifest? Ideally via a nice printk and a stack trace > followed by damage avoidance. They will manifest in the same way as any other use-after-free bug: an oops message and either death of the current process or a system hang. Obviously I'm not aware of any such bugs -- if I were, I'd fix them. Greg has expressed concern that some USB serial drivers might have this problem. I'll do what testing I can (not much because I don't have any USB serial devices). > If it's via a mysterious crash or something similarly obscure then can we > improve that? I can't think of anything offhand. Maybe someone else can. Alan Stern - 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/
Re: [PATCH] Kobjects: drop child->parent ref at unregistration
On Mon, 19 Nov 2007 10:53:40 -0500 (EST) Alan Stern <[EMAIL PROTECTED]> wrote: > This patch (as1015) reverts changes that were made to the driver core > about four years ago. The intent back then was to avoid certain kinds > of invalid memory accesses by leaving kernel objects allocated as long > as any of their children were still allocated. The original and > correct approach was to wait only as long as any children were still > _registered_; that's what this patch reinstates. What happened with this? > This fixes a problem in the SCSI core made visible by the class_device > to regular device conversion: A reference loop (scsi_device holds > reference to request_queue, which is the child of a gendisk, which is > the child of the scsi_device) prevents the data structures from being > released, even though they are deregistered okay. > > It's possible that this change will cause a few bugs to surface, > things that have been hidden for several years. They can be fixed > easily enough by having the child device take an explicit reference to > the parent whenever needed. > How will such bugs manifest? Ideally via a nice printk and a stack trace followed by damage avoidance. If it's via a mysterious crash or something similarly obscure then can we improve that? - 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/
[PATCH] Kobjects: drop child->parent ref at unregistration
This patch (as1015) reverts changes that were made to the driver core
about four years ago. The intent back then was to avoid certain kinds
of invalid memory accesses by leaving kernel objects allocated as long
as any of their children were still allocated. The original and
correct approach was to wait only as long as any children were still
_registered_; that's what this patch reinstates.
This fixes a problem in the SCSI core made visible by the class_device
to regular device conversion: A reference loop (scsi_device holds
reference to request_queue, which is the child of a gendisk, which is
the child of the scsi_device) prevents the data structures from being
released, even though they are deregistered okay.
It's possible that this change will cause a few bugs to surface,
things that have been hidden for several years. They can be fixed
easily enough by having the child device take an explicit reference to
the parent whenever needed.
Signed-off-by: Alan Stern <[EMAIL PROTECTED]>
---
Greg:
I'm formally submitting this so that it can get some testing in your
development tree and in -mm. So far everyone who has considered the
matter thinks that this is a good change. Any bugs it has papered over
should be fixable.
Alan Stern
Index: usb-2.6/lib/kobject.c
===
--- usb-2.6.orig/lib/kobject.c
+++ usb-2.6/lib/kobject.c
@@ -206,12 +206,16 @@ void kobject_init(struct kobject * kobj)
static void unlink(struct kobject * kobj)
{
+ struct kobject *parent = kobj->parent;
+
if (kobj->kset) {
spin_lock(&kobj->kset->list_lock);
list_del_init(&kobj->entry);
spin_unlock(&kobj->kset->list_lock);
}
+ kobj->parent = NULL;
kobject_put(kobj);
+ kobject_put(parent);
}
/**
@@ -262,7 +266,6 @@ int kobject_add(struct kobject * kobj)
if (error) {
/* unlink does the kobject_put() for us */
unlink(kobj);
- kobject_put(parent);
/* be noisy on error issues */
if (error == -EEXIST)
@@ -516,7 +519,6 @@ void kobject_cleanup(struct kobject * ko
{
struct kobj_type * t = get_ktype(kobj);
struct kset * s = kobj->kset;
- struct kobject * parent = kobj->parent;
const char *name = kobj->k_name;
pr_debug("kobject %s: cleaning up\n",kobject_name(kobj));
@@ -533,7 +535,6 @@ void kobject_cleanup(struct kobject * ko
}
if (s)
kset_put(s);
- kobject_put(parent);
}
static void kobject_release(struct kref *kref)
-
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/
