linux-kernel - Re: [PATCH] sysfs: differentiate between locking links and non-links

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, 10 Feb 2010 17:31:30 -0800
From:	ebiederm@...ssion.com (Eric W. Biederman)
To:	Greg KH <gregkh@...e.de>
Cc:	Tejun Heo <tj@...nel.org>,
	Américo Wang <xiyou.wangcong@...il.com>,
	Neil Brown <neilb@...e.de>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] sysfs: differentiate between locking links and non-links

Greg KH <gregkh@...e.de> writes:

> On Wed, Feb 10, 2010 at 10:25:21AM -0800, Eric W. Biederman wrote:
>> Tejun Heo <tj@...nel.org> writes:
>> 
>> > On 02/10/2010 05:03 PM, Eric W. Biederman wrote:
>> >> Tejun Heo <tj@...nel.org> writes:
>> >> 
>> >>> Hello,
>> >>>
>> >>> On 02/10/2010 11:08 AM, Américo Wang wrote:
>> >>>> This bug report is new for me. Recently we received lots of sysfs lockdep
>> >>>> warnings, I am working on a patch to fix all the bogus ones.
>> >>>>
>> >>>> However, this one is _not_ similar to the other cases, as you decribed.
>> >>>> This patch could fix the problem, but not a good fix, IMO. We need more
>> >>>> work in sysfs layer to fix this kind of things. I will take care of this.
>> >>>
>> >>> Can't we just give each s_active lock a separate class?  Would that be
>> >>> too costly?
>> >> 
>> >> When I asked the question earlier I was told that that locking classes
>> >> require static storage.  Where would that static storage come from?
>> >
>> > Maybe I'm glossly misunderstanding it but wouldn't embedding struct
>> > lockdep_map into sysfs_node as in work_struct do the trick?
>> 
>> In lockdep_init_map there is the following check:
>> 
>> 	/*
>> 	 * Sanity check, the lock-class key must be persistent:
>> 	 */
>> 	if (!static_obj(key)) {
>> 		printk("BUG: key %p not in .data!\n", key);
>> 		DEBUG_LOCKS_WARN_ON(1);
>> 		return;
>> 	}
>> 
>> It needs playing with but I think we can embed something in struct
>> attribute, and simply disallow dynamically allocated instances of
>> struct attribute.
>
> I think some code dynamically creates attributes today, as this has
> never been a restriction.
>
> So I don't know if this is going to work :(

I need to see how locks do this, but they face the same problem.  For
normal locks this is resolved by having a requirement to call a special
initializer in the dynamic case.  Adding that requirement for the
rare dynamically allocated sysfs attributes seems reasonable.

Additionally sysfs attributes are exactly the right granularity for
lock classes because that is where the behavior is the same or
changes.

We have 845 instances of static struct attributes which certainly makes
that the dominant case and worth aiming at.


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