From: Al Viro <viro@ZenIV.linux.org.uk>
To: "Eric W. Biederman" <ebiederm@aristanetworks.com>
Cc: linux-fsdevel@vger.kernel.org,
Linus Torvalds <torvalds@linux-foundation.org>
Subject: [RFC] breakage in sysfs_readdir() and s_instances abuse in sysfs
Date: Sat, 4 Jun 2011 01:15:19 +0100 [thread overview]
Message-ID: <20110604001518.GT11521@ZenIV.linux.org.uk> (raw)
sysfs_readdir() starts with doing
ns = sysfs_info(dentry->d_sb)->ns[type];
then proceeds to scan the directory with sysfs_dir_{,next_}pos(ns, ...)
even though we have no promise whatsoever that sysfs_exit_ns() has not
cleared that element of array in the meanwhile.
Granted, sysfs_dir_pos() doesn't dereference ns; however, it does compare
with it. And ns might have been freed and reused by that point.
I don't like what's going on there; that code looks inherently racy.
We never set ->ns[...] non-NULL outside of mount(). So it looks like
the intended behaviour is to have all ns-specific entries of that type
disappear forever from that fs instance. Having entries for _another_
namespace to show up for a (short) while, and that only in readdir()
(lookup runs completely under sysfs_mutex, so we don't have that race
there)...
I also don't like the s_instances abuse in sysfs_exit_ns() but I'd like
to understand what is that code *for* before starting to massage it into
something saner. What are the possible locking conditions in callers of
that thing? I've tried to look for callers, but the (too long) callchain
has led me to pernet_operations ->exit() and at that point I'd given up -
that over the top for grepping...
BTW, is kobj_ns_current() pinned down? Can that ns disappear from under
sysfs_mount()? It looks like it shouldn't, but then I haven't traced the
callers of sysfs_exit_ns()...
What I'm planning to do (for unrelated reasons - ubifs needs it) is to add
an analog of iterate_supers() that would go over the superblocks of given
type and call a function on them. I would like to convert sysfs_exit_ns()
to it and kill the last abuser of s_instances (and one of the last sb_lock
ones), but that really depends on what kind of locking is needed for
readdir() and friends - as it is, the damn thing looks *wrong*.
next reply other threads:[~2011-06-04 0:15 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-06-04 0:15 Al Viro [this message]
2011-06-04 21:22 ` [RFC] breakage in sysfs_readdir() and s_instances abuse in sysfs Al Viro
2011-06-04 21:55 ` Linus Torvalds
2011-06-04 22:23 ` Al Viro
2011-06-06 19:03 ` Eric W. Biederman
2011-06-07 21:58 ` Al Viro
2011-06-07 22:59 ` Al Viro
2011-06-09 1:26 ` Al Viro
2011-06-12 7:15 ` Eric W. Biederman
2011-06-12 17:59 ` Linus Torvalds
2011-06-12 18:17 ` Al Viro
2011-06-12 18:35 ` Al Viro
-- strict thread matches above, loose matches on Subject: below --
2011-06-04 22:25 Al Viro
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20110604001518.GT11521@ZenIV.linux.org.uk \
--to=viro@zeniv.linux.org.uk \
--cc=ebiederm@aristanetworks.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=torvalds@linux-foundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).