From: Al Viro <viro@ZenIV.linux.org.uk>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: James Morris <jmorris@namei.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
LSM List <linux-security-module@vger.kernel.org>,
linux-fsdevel <linux-fsdevel@vger.kernel.org>
Subject: Re: security: lockless stat() issues
Date: Fri, 4 Oct 2013 21:15:12 +0100 [thread overview]
Message-ID: <20131004201512.GQ13318@ZenIV.linux.org.uk> (raw)
In-Reply-To: <CA+55aFxndx0J0sTLTx0qu25TkoLLfGd+8qiLhdG5CYwpO1NvVQ@mail.gmail.com>
On Fri, Oct 04, 2013 at 12:33:17PM -0700, Linus Torvalds wrote:
> So how do people feel about passing a "mode" value for
> security_inode_getattr(), the same way we do for
> security_inode_permission()? The only flag would be that MAY_NOT_BLOCK
> flag that gets set for RCU lookup, and the semantics would be the same
> (return -ECHILD if you need to sleep).
>
> Attached is a patch that adds the interface, and then makes all
> security layers just do that ECHILD thing (and nobody actually sets
> MAY_NOT_BLOCK yet). So it's purely preparatory. It's also
> insufficient, because we'll need the same kind o fflag for the
> low-level filesystem "i_op->getattr()" call, but that's an independent
> issue.
>
> Al, any comments?
Just a couple of days ago you'd been complaining about filesystems exposure
to rcuwalk details and now you propose to increase the contact surface
by one more method? OK...
For one thing, that will definitely require lazy freeing of struct
super_block itself. At the very least, you want ->i_sb->s_dev, even
on default pathway. Another thing is that API is wrong for that
kind of stuff - we pass vfsmount/dentry pair and we end up looking
at the inode guts. Fine, but in RCU case you must cope with the
possibility of dentry->d_inode going NULL under you. Which isn't
all that terrible, but we need to slap ACCESS_ONCE() into each method
instance (all 34 of them). IOW, it's worse than just "oh, we need to
add a flag"; we'll need to pass the sodding inode separately, with
dire warnings not to use dentry->d_inode.
It's not impossible to do, of course, but it won't be fun...
Oh, well... In any case, we need something in Documentation/filesystems
describing the RCU-related rules for fs writers - exposure surface, what
to avoid, etc.
next prev parent reply other threads:[~2013-10-04 20:15 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-10-04 19:33 security: lockless stat() issues Linus Torvalds
2013-10-04 20:15 ` Al Viro [this message]
2013-10-04 20:49 ` Linus Torvalds
2013-10-04 21:15 ` 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=20131004201512.GQ13318@ZenIV.linux.org.uk \
--to=viro@zeniv.linux.org.uk \
--cc=jmorris@namei.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-security-module@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