linux-kernel - Re: [PATCH 0/3] refcounting improvements in sysfs.

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, 26 Mar 2010 13:49:36 +0900
From:	Tejun Heo <teheo@...e.de>
To:	NeilBrown <neilb@...e.de>
Cc:	Greg Kroah-Hartman <gregkh@...e.de>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 0/3] refcounting improvements in sysfs.

Hello,

On 03/24/2010 12:20 PM, NeilBrown wrote:
> This series tidies up the refcount of sysfs_dirents in sysfs,
> using kref where appropriate and a new karef for s_active.
> This achieves significant code simplification, especially the first
> patch.
> 
> This is in part inspired by http://lwn.net/Articles/336224/ :-)

Nice article.  In general, yeap, I agree it would be nice to have a
working reference count abstraction.  However, kref along with kobject
is a good example of obscurity by abstraction anti pattern.  :-)

kobject API incorrectly suggests that it deals with the last put
problem.  There still are large number of code paths which do the
following,

  if (!(kob = kobject_get(kobj)))
	return;

I believe (or at least hope) the actual problem cases are mostly fixed
now but there still are a lot of misconceptions around how stuff built
on kref/kobject is synchronized and they sometimes lead to race
conditions buried deep under several layers of abstractions and it
becomes very hard to see those race conditions when they are buried
deep.

If you want to kill refcounts w/ bias based off switch, please put it
inside an abstraction which at least synchronizes itself properly.
Open coding w/ bias at least warns you that there is some complex
stuff going on and you need to trade carefully.  Putting the switch on
a separate flag - people often forget how bits in a flag field are
synchronized - and the rest of refcount in a nice looking kref bundle
is very likely to lead to subtle race conditions which are *very*
difficult to notice.

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/