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 00:43:13 +0100	[thread overview]
Message-ID: <20250703234313.GM1880847@ZenIV> (raw)
In-Reply-To: <175004219130.608730.907040844486871388@noble.neil.brown.name>

On Mon, Jun 16, 2025 at 12:49:51PM +1000, NeilBrown wrote:

> The reality is that ->sysctl does not need rcu protection.  There is no
> concurrent update except that it can be set to NULL which is pointless.

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.

  reply	other threads:[~2025-07-03 23:43 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 [this message]
2025-07-04  1:02       ` Al Viro
2025-07-04  1:39         ` NeilBrown
2025-07-04  2:19           ` Al Viro
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=20250703234313.GM1880847@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.