linux-kernel - [PATCH] proc_sysctl: remove RCU annotations for accessing ->sysctl

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-id: <174978315268.608730.1330012617868311392@noble.neil.brown.name>
Date: Fri, 13 Jun 2025 12:52:32 +1000
From: "NeilBrown" <neil@...wn.name>
To: Al Viro <viro@...iv.linux.org.uk>, Kees Cook <kees@...nel.org>,
 Joel Granados <joel.granados@...nel.org>
Cc: linux-fsdevel@...r.kernel.org, LKML <linux-kernel@...r.kernel.org>
Subject: [PATCH] proc_sysctl: remove RCU annotations for accessing ->sysctl

The ->sysctl field of a procfs inode is only set when the inode is
created, and when it is being evicted.  In both these cases there cannot
be concurrent accesses and so using RCU_INIT_POINTER() and
rcu_dereference() is misleading.

I discovered this with some devel code which called d_same_name()
without holding the rcu_read_lock() - rcu_dereference() triggered a
warning.  In mainline ->d_compare is called from d_alloc_parallel()
without rcu_read_lock() after taking ->d_lock.  It is conceivable that
the d_inode will have been set while waiting for that lock so mainline
could trigger the same warning.

This patch removes those accessor call.  Note that the sysctl field is
not marked __rcu so sparse complains too.

Signed-off-by: NeilBrown <neil@...wn.name>
---
 fs/proc/inode.c       | 2 +-
 fs/proc/proc_sysctl.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/proc/inode.c b/fs/proc/inode.c
index a3eb3b740f76..c3991dd314d9 100644
--- a/fs/proc/inode.c
+++ b/fs/proc/inode.c
@@ -42,7 +42,7 @@ static void proc_evict_inode(struct inode *inode)

 	head = ei->sysctl;
 	if (head) {
-		RCU_INIT_POINTER(ei->sysctl, NULL);
+		ei->sysctl = NULL;
 		proc_sys_evict_inode(inode, head);
 	}
 }
diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
index cc9d74a06ff0..976d7605560f 100644
--- a/fs/proc/proc_sysctl.c
+++ b/fs/proc/proc_sysctl.c
@@ -928,7 +928,7 @@ static int proc_sys_compare(const struct dentry *dentry,
 		return 1;
 	if (memcmp(name->name, str, len))
 		return 1;
-	head = rcu_dereference(PROC_I(inode)->sysctl);
+	head = PROC_I(inode)->sysctl;
 	return !head || !sysctl_is_seen(head);
 }

-- 
2.49.0