[PATCH v3] kernfs: recheck of->released after acquiring the active reference

Fan Wu posted 1 patch 4 weeks, 1 day ago
fs/kernfs/file.c       | 10 ++++++++--
include/linux/kernfs.h |  2 +-
2 files changed, 9 insertions(+), 3 deletions(-)
[PATCH v3] kernfs: recheck of->released after acquiring the active reference
Posted by Fan Wu 4 weeks, 1 day ago
kernfs_get_active_of(), added by commit 3c9ba2777d6c ("kernfs: Fix UAF
in polling when open file is released"), tests @of->released before
acquiring the active reference on @of->kn. A hide/drain/show cycle can
run between the two steps: the drain path releases the open file, and
the reactivation lets kernfs_get_active() succeed again, so a file
operation can run on an already released open file. On the cgroup
pressure files, the poll callback dereferences of->priv while forming
&ctx->psi.trigger and can hit either stale, freed memory or NULL.

Re-check @of->released after the successful acquisition and drop the
active reference again if it has been set. No lock is needed:
@of->released is only ever set to true, the drain which sets it
precedes the reactivation under kernfs_rwsem, and the fully-ordered
RMW on @kn->active in kernfs_get_active() then orders the read after
that reactivation.

To allow the lockless reads to use READ_ONCE(), drop the :1 storage
from @released and store it with WRITE_ONCE() to match.

This issue was found by an in-house static analysis tool.

Fixes: 3c9ba2777d6c ("kernfs: Fix UAF in polling when open file is released")
Cc: stable@vger.kernel.org
Suggested-by: Tejun Heo <tj@kernel.org>
Acked-by: Tejun Heo <tj@kernel.org>
Assisted-by: Codex:gpt-5.6
Signed-off-by: Fan Wu <fanwu01@zju.edu.cn>
---
Changes in v3:
- Drop the :1 bit-field storage from @released so that READ_ONCE() can
  be applied to it, use READ_ONCE() for both lockless reads of the
  field, and add the matching WRITE_ONCE() on the store side.
  READ_ONCE() on a bit-field does not build, as reported by the
  kernel test robot on v2. The logic is unchanged from v2, where the
  Acked-by was obtained; happy to drop it if you prefer a fresh one.

v2: <20260821050720.14848-1-fanwu01@zju.edu.cn>
lkp report on v2: <202608230046.Ixvo8Av6-lkp@intel.com>
---
 fs/kernfs/file.c       | 10 ++++++++--
 include/linux/kernfs.h |  2 +-
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/fs/kernfs/file.c b/fs/kernfs/file.c
index 9adf36e6364b..98adceddbcfa 100644
--- a/fs/kernfs/file.c
+++ b/fs/kernfs/file.c
@@ -74,12 +74,18 @@ static struct kernfs_open_node *of_on(struct kernfs_open_file *of)
 static struct kernfs_open_file *kernfs_get_active_of(struct kernfs_open_file *of)
 {
 	/* Skip if file was already released */
-	if (unlikely(of->released))
+	if (unlikely(READ_ONCE(of->released)))
 		return NULL;
 
 	if (!kernfs_get_active(of->kn))
 		return NULL;
 
+	/* @of may have been drained in between: recheck. */
+	if (unlikely(READ_ONCE(of->released))) {
+		kernfs_put_active(of->kn);
+		return NULL;
+	}
+
 	return of;
 }
 
@@ -762,7 +768,7 @@ static void kernfs_release_file(struct kernfs_node *kn,
 		 * and being drained.  Don't use kernfs_ops().
 		 */
 		kn->attr.ops->release(of);
-		of->released = true;
+		WRITE_ONCE(of->released, true);
 		of_on(of)->nr_to_release--;
 	}
 }
diff --git a/include/linux/kernfs.h b/include/linux/kernfs.h
index b5a5f32fdfd1..e95e333e24ea 100644
--- a/include/linux/kernfs.h
+++ b/include/linux/kernfs.h
@@ -269,7 +269,7 @@ struct kernfs_open_file {
 
 	size_t			atomic_write_len;
 	bool			mmapped:1;
-	bool			released:1;
+	bool			released;
 	const struct vm_operations_struct *vm_ops;
 };