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:	Wed, 18 Apr 2007 17:36:21 +0900
From:	Tejun Heo <htejun@...il.com>
To:	Cornelia Huck <cornelia.huck@...ibm.com>
CC:	linux-kernel <linux-kernel@...r.kernel.org>,
	Alan Stern <stern@...land.harvard.edu>,
	Greg K-H <greg@...ah.com>,
	Rusty Russell <rusty@...tcorp.com.au>,
	dmitry.torokhov@...il.com
Subject: Re: [PATCH RFD] alternative kobject release wait mechanism

Cornelia Huck wrote:
> On Wed, 18 Apr 2007 03:41:10 +0900,
> Tejun Heo <htejun@...il.com> wrote:
> 
>> Hello, all.
>>
>> Agreed with the problem but I'm not very enthusiastic for adding
>> kobj->owner.  How about the following?  exit() routines will have to
>> do device_unregister_wait() instead of device_unregister().  On return
>> from it, it's guaranteed that all references to it are dropped and
>> ->release is finished.
> 
> Sounds interesting. Kind of like the completion approach, but with the
> dangerous bits outside the module.
> 
>> The caller is responsible for avoiding
>> deadlock, of course.
> 
> I think that wording is a bit too strong. Of course, if the device is
> unregistered in the exit routine, the module must make sure it gave up
> all references it itself obtained. However, it doesn't have any control
> about who obtained a reference during the object's lifetime. If an
> outsider holds on to a reference, we'll lock up until this reference
> has been given up (but I guess this is what we want).

Right, the better wording would be "the caller is responsible for not
causing deadlocks by dropping all references it owns on entry".

>> +void kobject_put_wait(struct kobject * kobj)
>> +{
>> +	struct kobj_wait kwait;
>> +	unsigned long flags;
>> +
>> +	if (!kobj)
>> +		return;
>> +
>> +	BUG_ON(!list_empty(&kobj->entry));
>> +
>> +	init_completion(&kwait.cmpl);
>> +	kwait.kobj = kobj;
>> +
>> +	spin_lock_irqsave(&kobj_wait_lock, flags);
>> +	list_add(&kwait.list, &kobj_wait_list);
>> +	spin_unlock_irqrestore(&kobj_wait_lock, flags);
>> +
>> +	kobject_put(kobj);
>> +
>> +	if (!wait_for_completion_timeout(&kwait.cmpl, 30 * HZ)) {
>> +		printk(KERN_WARNING "kobject_put_wait: kobject %p is still "
>> +		       "alive after 30s, possible reference count bug\n", kobj);
>> +		dump_stack();
>> +		wait_for_completion(&kwait.cmpl);
>> +	}
>> +}
>>
> 
> Couldn't this waiting be made simpler?
> - add completion to kobject in kobject_unregister_wait()
> - call kobject_put(), then wait_for_completion()
> - in kobject_cleanup(), save completion from kobject, call release
> function, then complete() on saved completion

I was trying to avoid adding a completion to all kobjects.

Thanks.

-- 
tejun
-
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/