All of lore.kernel.org
 help / color / mirror / Atom feed
From: Al Viro <viro@ZenIV.linux.org.uk>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Eric Paris <eparis@redhat.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	Paul McKenney <paulmck@linux.vnet.ibm.com>,
	Dave Chinner <david@fromorbit.com>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>,
	James Morris <james.l.morris@oracle.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Stephen Smalley <sds@tycho.nsa.gov>,
	"Theodore Ts'o" <tytso@mit.edu>, stable <stable@vger.kernel.org>,
	Paul Moore <paul@paul-moore.com>,
	LKML <linux-kernel@vger.kernel.org>,
	Matthew Wilcox <matthew@wil.cx>,
	Christoph Hellwig <hch@infradead.org>
Subject: Re: [PATCH] vfs: Fix possible NULL pointer dereference in inode_permission()
Date: Fri, 10 Jan 2014 00:06:42 +0000	[thread overview]
Message-ID: <20140110000642.GN10323@ZenIV.linux.org.uk> (raw)
In-Reply-To: <CA+55aFzd2nw=JU4s0u=PJbATK0bwhm0kot3zRH=anLLT6THRFQ@mail.gmail.com>

On Fri, Jan 10, 2014 at 07:53:41AM +0800, Linus Torvalds wrote:
> On Fri, Jan 10, 2014 at 7:37 AM, Eric Paris <eparis@redhat.com> wrote:
> >
> > but at least from an SELinux PoV, I think it's quick and easy, but wrong
> > for maintainability...
> 
> Yeah, it's a hack, and it's wrong, and we should figure out how to do
> it right. Likely we should just tie the lifetime of the i_security
> member directly to the lifetime of the inode itself, and just make the
> rule be that security_inode_free() gets called from whatever frees the
> inode itself, and *not* have an extra rcu callback etc. But that
> sounds like a bigger change than I'm comfy with right now, so the
> hacky one might be the band-aid to do for stable..
> 
> The problem, of course, is that all the different filesystems have
> their own inode allocations/freeing. Of course, they all tend to share
> the same pattern ("call_rcu xyz_i_callback"), so maybe we could try to
> make that a more generic thing? Like have a "free_inode" vfs callback,
> and do the call_rcu delaying at the VFS level..
> 
> And maybe, just maybe, we could just say that that is what
> "destroy_inode()" is, and that we will just call it from rcu context.
> All the IO has hopefully been done earlier  Yes/no?

Check what XFS is doing ;-/  That's where those call_rcu() have come from.
Sure, we can separate the simple "just do call_rcu(...->free_inode)" case
and hit it whenever full ->free_inode is there and ->destroy_inode isn't.
Not too pretty, but removal of tons of boilerplate might be worth doing
that anyway.  But ->destroy_inode() is still needed for cases where fs
has its own idea of inode lifetime rules.  Again, check what XFS is doing
in that area...

There's an extra source of headache, BTW - what about the "LSM stacking"
crowd and their plans?

  reply	other threads:[~2014-01-10  0:06 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-01-09 21:27 [PATCH] vfs: Fix possible NULL pointer dereference in inode_permission() Steven Rostedt
2014-01-09 21:42 ` Matthew Wilcox
2014-01-09 21:50   ` Steven Rostedt
2014-01-09 22:31     ` Al Viro
     [not found]       ` <CA+55aFzCTPYEQCPnLBi1CwmMTocVqCFiCuJ391HkVx1CMw61ug@mail.gmail.com>
2014-01-09 23:10         ` Paul E. McKenney
2014-01-09 23:25         ` Steven Rostedt
2014-01-09 23:27           ` Steven Rostedt
2014-01-09 23:37             ` Eric Paris
2014-01-09 23:45               ` Steven Rostedt
2014-01-09 23:53               ` Linus Torvalds
2014-01-10  0:06                 ` Al Viro [this message]
2014-01-10  0:09                   ` Al Viro
2014-01-10  0:18                   ` Linus Torvalds
2014-01-10  2:36                     ` James Morris
2014-01-10  9:31                   ` Christoph Hellwig
2014-01-10  9:31                     ` Christoph Hellwig
2014-01-10 18:14                     ` Ben Myers
2014-01-10 18:14                       ` Ben Myers
2014-01-11 10:32                       ` Christoph Hellwig
2014-01-11 10:32                         ` Christoph Hellwig
2014-01-09 23:45             ` Paul E. McKenney
2014-01-09 23:59               ` Steven Rostedt
2014-01-10  0:44                 ` Paul E. McKenney

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=20140110000642.GN10323@ZenIV.linux.org.uk \
    --to=viro@zeniv.linux.org.uk \
    --cc=akpm@linux-foundation.org \
    --cc=david@fromorbit.com \
    --cc=eparis@redhat.com \
    --cc=hch@infradead.org \
    --cc=james.l.morris@oracle.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=matthew@wil.cx \
    --cc=paul@paul-moore.com \
    --cc=paulmck@linux.vnet.ibm.com \
    --cc=rostedt@goodmis.org \
    --cc=sds@tycho.nsa.gov \
    --cc=stable@vger.kernel.org \
    --cc=torvalds@linux-foundation.org \
    --cc=tytso@mit.edu \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.