Linux driver-core infrastructure
 help / color / mirror / Atom feed
* [PATCH] kernfs: recheck of->released after acquiring the active reference
@ 2026-08-20  2:18 Fan Wu
  2026-08-20 17:46 ` Tejun Heo
  0 siblings, 1 reply; 4+ messages in thread
From: Fan Wu @ 2026-08-20  2:18 UTC (permalink / raw)
  To: gregkh, tj; +Cc: chenridong, driver-core, linux-kernel, Fan Wu, stable

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 those steps: the drain path releases the open file, and the
reactivation lets kernfs_get_active() succeed again.  Any entry guarded
by kernfs_get_active_of() can consequently run its file operation 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.

  CPU 0 (kernfs_fop_poll)       CPU 1 (echo 0/1 > cgroup.pressure)
  -------------------------     ---------------------------------
  of->released == false         kernfs_show(kn, false)
  ... preempted ...               kernfs_drain()
                                   kernfs_release_file()
                                     ->release(of)  (frees of->priv)
                                     of->released = true
                                kernfs_show(kn, true)
                                  kernfs_activate_one(kn)
  kernfs_get_active(of->kn)
  ops->poll(of)

The cycle needs the file operation to be delayed between the two
steps, but kernfs_show() cycles like the one above are fully
userspace driven.

Acquire the active reference first and re-check @of->released under
kernfs_open_file_mutex.  While the reference is held, @kn cannot be
drained: kernfs_drain() waits for kn->active to reach
KN_DEACTIVATED_BIAS before draining open files, and the only other
kernfs_release_file() caller, kernfs_fop_release(), is serialized
against in-flight file operations by the VFS.  Since
kernfs_release_file() sets @of->released under the same mutex, a
false re-read settles the question for good.

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
Assisted-by: Codex:gpt-5.6
Signed-off-by: Fan Wu <fanwu01@zju.edu.cn>
---
 fs/kernfs/file.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/fs/kernfs/file.c b/fs/kernfs/file.c
index 9adf36e6364b..561f66e4095a 100644
--- a/fs/kernfs/file.c
+++ b/fs/kernfs/file.c
@@ -73,12 +73,22 @@ static struct kernfs_open_node *of_on(struct kernfs_open_file *of)
 /* Get active reference to kernfs node for an open file */
 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 (!kernfs_get_active(of->kn))
 		return NULL;
 
-	if (!kernfs_get_active(of->kn))
+	/*
+	 * @of->released is set under kernfs_open_file_mutex.  While the
+	 * active reference is held, @kn can't be drained anymore and
+	 * kernfs_fop_release() can't run, so re-reading @of->released
+	 * here settles whether @of was released for good.
+	 */
+	mutex_lock(kernfs_open_file_mutex_ptr(of->kn));
+	if (unlikely(of->released)) {
+		mutex_unlock(kernfs_open_file_mutex_ptr(of->kn));
+		kernfs_put_active(of->kn);
 		return NULL;
+	}
+	mutex_unlock(kernfs_open_file_mutex_ptr(of->kn));
 
 	return of;
 }


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] kernfs: recheck of->released after acquiring the active reference
  2026-08-20  2:18 [PATCH] kernfs: recheck of->released after acquiring the active reference Fan Wu
@ 2026-08-20 17:46 ` Tejun Heo
  2026-08-21  5:07   ` [PATCH v2] " Fan Wu
  0 siblings, 1 reply; 4+ messages in thread
From: Tejun Heo @ 2026-08-20 17:46 UTC (permalink / raw)
  To: Fan Wu; +Cc: gregkh, chenridong, driver-core, linux-kernel, stable

Hello,

On Thu, Aug 20, 2026 at 02:18:10AM +0000, Fan Wu wrote:
> @@ -73,12 +73,22 @@ static struct kernfs_open_node *of_on(struct kernfs_open_file *of)
>  /* Get active reference to kernfs node for an open file */
>  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 (!kernfs_get_active(of->kn))
>  		return NULL;
>  
> -	if (!kernfs_get_active(of->kn))
> +	/*
> +	 * @of->released is set under kernfs_open_file_mutex.  While the
> +	 * active reference is held, @kn can't be drained anymore and
> +	 * kernfs_fop_release() can't run, so re-reading @of->released
> +	 * here settles whether @of was released for good.
> +	 */
> +	mutex_lock(kernfs_open_file_mutex_ptr(of->kn));
> +	if (unlikely(of->released)) {
> +		mutex_unlock(kernfs_open_file_mutex_ptr(of->kn));
> +		kernfs_put_active(of->kn);
>  		return NULL;
> +	}
> +	mutex_unlock(kernfs_open_file_mutex_ptr(of->kn));

"lock -> test something -> unlock" pattern is usually unnecessary. What's
the mutex achieving doing? Doesn't kernfs_get_active() already have strong
enough memory barrier? If not, it'd be better to solve it by iterlocking
kernfs_get_active() and released clearing.

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v2] kernfs: recheck of->released after acquiring the active reference
  2026-08-20 17:46 ` Tejun Heo
@ 2026-08-21  5:07   ` Fan Wu
  2026-08-21 19:21     ` Tejun Heo
  0 siblings, 1 reply; 4+ messages in thread
From: Fan Wu @ 2026-08-21  5:07 UTC (permalink / raw)
  To: gregkh, tj; +Cc: chenridong, driver-core, linux-kernel, Fan Wu, stable

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 those steps: the drain path releases the open file, and the
reactivation lets kernfs_get_active() succeed again.  Any entry guarded
by kernfs_get_active_of() can consequently run its file operation 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.

  CPU 0 (kernfs_fop_poll)       CPU 1 (echo 0/1 > cgroup.pressure)
  -------------------------     ---------------------------------
  of->released == false         kernfs_show(kn, false)
  ... preempted ...               kernfs_drain()
                                   kernfs_release_file()
                                     ->release(of)  (frees of->priv)
                                     of->released = true
                                kernfs_show(kn, true)
                                  kernfs_activate_one(kn)
  kernfs_get_active(of->kn)
  ops->poll(of)

The cycle needs the file operation to be delayed between the two
steps, but kernfs_show() cycles like the one above are fully
userspace driven.

Acquire the active reference first and re-check @of->released after
it.  While the reference is held, @kn cannot be drained: kernfs_drain()
waits for kn->active to reach KN_DEACTIVATED_BIAS before draining open
files, and the only other kernfs_release_file() caller,
kernfs_fop_release(), is serialized against in-flight file operations
by the VFS, so the re-read settles whether @of was released for good.
The re-check needs no lock: @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.

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>
Assisted-by: Codex:gpt-5.6
Signed-off-by: Fan Wu <fanwu01@zju.edu.cn>
---
Changes since v1:
- Drop the kernfs_open_file_mutex re-check; the fully-ordered RMW in
  kernfs_get_active() together with the kernfs_rwsem serialization of
  drain and reactivation already orders the released re-read.
- Read @of->released with READ_ONCE().
---
 fs/kernfs/file.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/fs/kernfs/file.c b/fs/kernfs/file.c
index 9adf36e6364b..44d40d9e6dd1 100644
--- a/fs/kernfs/file.c
+++ b/fs/kernfs/file.c
@@ -73,12 +73,17 @@ static struct kernfs_open_node *of_on(struct kernfs_open_file *of)
 /* Get active reference to kernfs node for an open file */
 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 (!kernfs_get_active(of->kn))
 		return NULL;
 
-	if (!kernfs_get_active(of->kn))
+	/*
+	 * A successful active reference prevents a new drain and orders this
+	 * check after an earlier reactivation.
+	 */
+	if (unlikely(READ_ONCE(of->released))) {
+		kernfs_put_active(of->kn);
 		return NULL;
+	}
 
 	return of;
 }


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] kernfs: recheck of->released after acquiring the active reference
  2026-08-21  5:07   ` [PATCH v2] " Fan Wu
@ 2026-08-21 19:21     ` Tejun Heo
  0 siblings, 0 replies; 4+ messages in thread
From: Tejun Heo @ 2026-08-21 19:21 UTC (permalink / raw)
  To: Fan Wu; +Cc: gregkh, chenridong, driver-core, linux-kernel, stable

On Fri, Aug 21, 2026 at 05:07:20AM +0000, Fan Wu wrote:
> 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 those steps: the drain path releases the open file, and the
> reactivation lets kernfs_get_active() succeed again.  Any entry guarded
> by kernfs_get_active_of() can consequently run its file operation 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.
> 
>   CPU 0 (kernfs_fop_poll)       CPU 1 (echo 0/1 > cgroup.pressure)
>   -------------------------     ---------------------------------
>   of->released == false         kernfs_show(kn, false)
>   ... preempted ...               kernfs_drain()
>                                    kernfs_release_file()
>                                      ->release(of)  (frees of->priv)
>                                      of->released = true
>                                 kernfs_show(kn, true)
>                                   kernfs_activate_one(kn)
>   kernfs_get_active(of->kn)
>   ops->poll(of)
> 
> The cycle needs the file operation to be delayed between the two
> steps, but kernfs_show() cycles like the one above are fully
> userspace driven.
> 
> Acquire the active reference first and re-check @of->released after
> it.  While the reference is held, @kn cannot be drained: kernfs_drain()
> waits for kn->active to reach KN_DEACTIVATED_BIAS before draining open
> files, and the only other kernfs_release_file() caller,
> kernfs_fop_release(), is serialized against in-flight file operations
> by the VFS, so the re-read settles whether @of was released for good.
> The re-check needs no lock: @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.
> 
> 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>
> Assisted-by: Codex:gpt-5.6
> Signed-off-by: Fan Wu <fanwu01@zju.edu.cn>

Acked-by: Tejun Heo <tj@kernel.org>

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-08-21 19:21 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-20  2:18 [PATCH] kernfs: recheck of->released after acquiring the active reference Fan Wu
2026-08-20 17:46 ` Tejun Heo
2026-08-21  5:07   ` [PATCH v2] " Fan Wu
2026-08-21 19:21     ` Tejun Heo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox