All of lore.kernel.org
 help / color / mirror / Atom feed
From: Al Viro <viro@zeniv.linux.org.uk>
To: NeilBrown <neil@brown.name>
Cc: Kees Cook <kees@kernel.org>,
	Joel Granados <joel.granados@kernel.org>,
	linux-fsdevel@vger.kernel.org,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v3?] proc_sysctl: remove rcu_dereference() for accessing ->sysctl
Date: Fri, 4 Jul 2025 03:19:44 +0100	[thread overview]
Message-ID: <20250704021944.GO1880847@ZenIV> (raw)
In-Reply-To: <175159319224.565058.14007562517229235836@noble.neil.brown.name>

On Fri, Jul 04, 2025 at 11:39:52AM +1000, NeilBrown wrote:
> On Fri, 04 Jul 2025, Al Viro wrote:
> > On Fri, Jul 04, 2025 at 12:43:13AM +0100, Al Viro wrote:
> > 
> > > I would rather *not* leave a dangling pointer there, and yes, it can
> > > end up being dangling.  kfree_rcu() from inside the ->evict_inode()
> > > may very well happen earlier than (also RCU-delayed) freeing of struct
> > > inode itself.
> > > 
> > > What we can do is WRITE_ONCE() to set it to NULL on the evict_inode
> > > side and READ_ONCE() in the proc_sys_compare().
> > > 
> > > The reason why the latter is memory-safe is that ->d_compare() for
> > > non-in-lookup dentries is called either under rcu_read_lock() (in which
> > > case observing non-NULL means that kfree_rcu() couldn't have gotten to
> > > freeing the sucker) *or* under ->d_lock, in which case the inode can't
> > > reach ->evict_inode() until we are done.
> > > 
> > > So this predicate is very much relevant.  Have that fucker called with
> > > neither rcu_read_lock() nor ->d_lock, and you might very well end up
> > > with dereferencing an already freed ctl_table_header.
> > 
> > IOW, I would prefer to do this:
> 
> Looks good - thanks,
> NeilBrown

See viro/vfs.git #fixes...

  reply	other threads:[~2025-07-04  2:19 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-06-15 23:00 [PATCH v3?] proc_sysctl: remove rcu_dereference() for accessing ->sysctl NeilBrown
2025-06-15 23:57 ` Al Viro
2025-06-16  2:49   ` NeilBrown
2025-07-03 23:43     ` Al Viro
2025-07-04  1:02       ` Al Viro
2025-07-04  1:39         ` NeilBrown
2025-07-04  2:19           ` Al Viro [this message]
2025-07-04  1:39       ` NeilBrown
2025-07-04  2:03         ` 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=20250704021944.GO1880847@ZenIV \
    --to=viro@zeniv.linux.org.uk \
    --cc=joel.granados@kernel.org \
    --cc=kees@kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=neil@brown.name \
    /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.