linux-kernel - Re: [PATCH RFD] alternative kobject release wait mechanism

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 20 Apr 2007 17:03:40 -0700
From:	Greg KH <greg@...ah.com>
To:	Alan Stern <stern@...land.harvard.edu>
Cc:	Tejun Heo <htejun@...il.com>,
	Cornelia Huck <cornelia.huck@...ibm.com>,
	linux-kernel <linux-kernel@...r.kernel.org>,
	Rusty Russell <rusty@...tcorp.com.au>,
	Dmitry Torokhov <dmitry.torokhov@...il.com>
Subject: Re: [PATCH RFD] alternative kobject release wait mechanism

On Fri, Apr 20, 2007 at 11:40:39AM -0400, Alan Stern wrote:
> Here's a patch to do what I mentioned earlier.  Not tested -- it may 
> expose some existing bugs.  It may even break something, but I'm not aware 
> of anything that depends on it explicitly.
> 
> Greg, do you know of anything in particular that depends on a kobjects not 
> being released before their children are released?

Yes, the whole driver model :)

When adding a new device, we always grab a reference to the parent
device so it can not go away before we do.

Look at the last kobject_put(parent); in kobject_cleanup() which ensures
this.

> Index: usb-2.6/lib/kobject.c
> ===================================================================
> --- usb-2.6.orig/lib/kobject.c
> +++ usb-2.6/lib/kobject.c
> @@ -192,12 +192,15 @@ 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);
>  	}
>  	kobject_put(kobj);
> +	kobject_put(parent);
>  }
>  
>  /**
> @@ -241,7 +244,6 @@ int kobject_shadow_add(struct kobject * 
>  	if (error) {
>  		/* unlink does the kobject_put() for us */
>  		unlink(kobj);
> -		kobject_put(parent);
>  
>  		/* be noisy on error issues */
>  		if (error == -EEXIST)
> @@ -489,7 +491,6 @@ void kobject_cleanup(struct kobject * ko
>  {
>  	struct kobj_type * t = get_ktype(kobj);
>  	struct kset * s = kobj->kset;
> -	struct kobject * parent = kobj->parent;
>  
>  	pr_debug("kobject %s: cleaning up\n",kobject_name(kobj));
>  	if (kobj->k_name != kobj->name)
> @@ -505,7 +506,6 @@ void kobject_cleanup(struct kobject * ko
>  
>  	if (s)
>  		kset_put(s);
> -	kobject_put(parent);
>  }

Ick, no, I think this used to be the way things worked, but bad things
would end up happening, so we fixed it up to be the way things are
today.  Read the comments for the changelog for this file for details.

Specifically, look at commit 10921a8f1305b8ec97794941db78b825db5839bc
in the history.git repo which is almost exactly what you are proposing
to be reverted...

thanks,

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