[PATCH] eventpoll: pin files while checking reverse paths

[PATCH] eventpoll: pin files while checking reverse paths

[Guidong Han]

Commit 319c15174757 ("epoll: take epitem list out of struct file")
intentionally removed temporary file references from the reverse path
check list. At the time, both epitems and their files were freed after
an RCU grace period, so unlist_file() could obtain file->f_lock through
an epitem while clear_tfile_check_list() held rcu_read_lock().

Commit 0ede61d8589c ("file: convert to SLAB_TYPESAFE_BY_RCU") made
struct file SLAB_TYPESAFE_BY_RCU and removed its RCU-delayed freeing.
RCU still protects the epitem, but no longer keeps the referenced file
from being freed and reused. A concurrent close can therefore make
unlist_file() lock or unlock f_lock in a recycled file object.

This violates the documented SLAB_TYPESAFE_BY_RCU rule requiring a
reference before acquiring an object's lock. The race was reproduced,
causing a wild unlock of f_lock in a recycled file and breaking its
mutual exclusion.

Add ->file to epitems_head to remember the pinned file independently of
->epitems. A concurrent EPOLL_CTL_DEL can empty ->epitems before the head
is unlisted, leaving no epi->ffd.file from which to drop the reference.

In list_file(), acquire the reference before adding the head to the
check list. The caller either owns a reference or holds the ep->mtx for
the epitem leading to the file. In the latter case, file_ref_get() can
fail after the last reference is dropped, but eventpoll_release_file()
must acquire the same mutex before the file can be freed. The dying leaf
can be skipped because removing links cannot increase the reverse path
count.

In unlist_file(), epnested_mutex excludes another list_file() or
unlist_file(), while head->next prevents a concurrent EPOLL_CTL_DEL from
freeing the head. Save head->file locally, clear it with head->next
under f_lock, and drop the reference after the RCU-protected operation.

Reported-by: Qi Tang <tpluszz77@gmail.com>
Reported-by: Junxi Qian <qjx1298677004@gmail.com>
Fixes: 0ede61d8589c ("file: convert to SLAB_TYPESAFE_BY_RCU")
Cc: stable@vger.kernel.org
Signed-off-by: Guidong Han <2045gemini@gmail.com>
---
SLAB_TYPESAFE_BY_RCU allows a slab slot to be reused while an RCU reader
still holds its old address. Once that address contains a new live
struct file, KASAN sees valid, unpoisoned memory and cannot distinguish
the stale object identity. CONFIG_DEBUG_SPINLOCK exposes the failure
instead.

The failing interleaving is:

CPU0: nested EPOLL_CTL_ADD             CPU1: close/open churn
------------------------------------   ---------------------------------
p = hlist_first_rcu(&head->epitems)
epi = container_of(p, ...)
                                       close(victim)
                                         __fput()
                                           eventpoll_release_file()
                                           file_free(victim)
                                       // the slot is free; f_lock remains
spin_lock(&epi->ffd.file->f_lock)
                                       open() reuses the slot as new_file
                                         spin_lock_init(&new_file->f_lock)
spin_unlock(&epi->ffd.file->f_lock)     // wild unlock of new_file's lock

CONFIG_DEBUG_SPINLOCK reports:

BUG: spinlock already unlocked on CPU#0, poc_unlist/150
 lock: 0xffff8880067fb200, .magic: dead4ead, .owner: <none>/-1, .owner_cpu: -1
CPU: 0 UID: 1000 PID: 150 Comm: poc_unlist Not tainted 7.2.0-rc3-dirty #22 PREEMPTLAZY 
Hardware name: QEMU Ubuntu 24.04 PC v2 (i440FX + PIIX, arch_caps fix, 1996), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
Call Trace:
 <TASK>
 dump_stack_lvl+0x64/0x80
 do_raw_spin_unlock+0x75/0xb0
 _raw_spin_unlock+0xe/0x30
 clear_tfile_check_list+0x88/0xe0
 do_epoll_ctl_file+0x519/0xcf0
 ? __pfx_ep_ptable_queue_proc+0x10/0x10
 do_epoll_ctl+0x8f/0x100
 __x64_sys_epoll_ctl+0x6f/0xa0
 do_syscall_64+0xdc/0x520
 ? srso_alias_return_thunk+0x5/0xfbef5
 entry_SYSCALL_64_after_hwframe+0x76/0x7e
RIP: 0033:0x42034e
Code: 48 c7 c1 b8 ff ff ff f7 d8 64 89 01 48 83 c8 ff c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa 49 89 ca b8 e9 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 b8 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007a657ff3c198 EFLAGS: 00000202 ORIG_RAX: 00000000000000e9
RAX: ffffffffffffffda RBX: 00007a657ff3ccdc RCX: 000000000042034e
RDX: 0000000000000003 RSI: 0000000000000001 RDI: 0000000000000004
RBP: 00007a657ff3c2f0 R08: 0000000000000000 R09: 00007a657ff3c6c0
R10: 00007a657ff3c1a4 R11: 0000000000000202 R12: 00007a657ff3c6c0
R13: ffffffffffffffb8 R14: 000000000000000d R15: 00007fffb7de0210
 </TASK>
------------[ cut here ]------------

unlist_file() does not appear as a separate frame because it was inlined
into clear_tfile_check_list(). This report was obtained with mdelay()
instrumentation immediately before spin_lock() and spin_unlock() in
unlist_file() to widen the two race windows.

More importantly, this is a wild unlock. The stale unlock can target
f_lock of a different live file and invalidate mutual exclusion for
state protected by that lock. Turning this into a reliable exploit
would require precise scheduling and same-slot reuse and is likely
difficult, but the primitive is potentially exploitable.

Qi Tang and Junxi Qian helped identify this issue and are therefore
credited by the Reported-by tags above.
---
 fs/eventpoll.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/fs/eventpoll.c b/fs/eventpoll.c
index 0e65c7431dfc..eed8cecd94e3 100644
--- a/fs/eventpoll.c
+++ b/fs/eventpoll.c
@@ -459,11 +459,14 @@ static struct kmem_cache *pwq_cache __ro_after_init;
  * Wrapper anchor for file->f_ep when the watched file is not itself an
  * eventpoll; for the epoll-watches-epoll case, file->f_ep points at
  * &watched_ep->refs directly. The ->next field threads
- * ctx->tfile_check_list during one EPOLL_CTL_ADD path check.
+ * ctx->tfile_check_list during one EPOLL_CTL_ADD path check. The ->file
+ * field holds a reference to the associated file while the head is on
+ * the list.
  */
 struct epitems_head {
 	struct hlist_head epitems;
 	struct epitems_head *next;
+	struct file *file;
 };
 
 static struct kmem_cache *ephead_cache __ro_after_init;
@@ -480,6 +483,16 @@ static void list_file(struct file *file, struct ep_ctl_ctx *ctx)
 
 	head = container_of(file->f_ep, struct epitems_head, epitems);
 	if (!head->next) {
+		/*
+		 * The caller owns a reference to @file or holds the ep->mtx for the
+		 * epitem that led here. The latter blocks eventpoll_release_file()
+		 * before the file allocation can be freed and reused. A dying leaf
+		 * can be skipped since removing links cannot increase the reverse
+		 * path count.
+		 */
+		if (!file_ref_get(&file->f_ref))
+			return;
+		head->file = file;
 		head->next = ctx->tfile_check_list;
 		ctx->tfile_check_list = head;
 	}
@@ -489,15 +502,18 @@ static void unlist_file(struct epitems_head *head)
 {
 	struct epitems_head *to_free = head;
 	struct hlist_node *p = rcu_dereference(hlist_first_rcu(&head->epitems));
+	struct file *file = head->file;
 	if (p) {
 		struct epitem *epi= container_of(p, struct epitem, fllink);
 		spin_lock(&epi->ffd.file->f_lock);
 		if (!hlist_empty(&head->epitems))
 			to_free = NULL;
 		head->next = NULL;
+		head->file = NULL;
 		spin_unlock(&epi->ffd.file->f_lock);
 	}
 	free_ephead(to_free);
+	fput(file);
 }
 
 #ifdef CONFIG_SYSCTL
-- 
2.43.0

Re: [PATCH] eventpoll: pin files while checking reverse paths

[David Laight]

On Sat, 18 Jul 2026 18:44:06 +0800
Guidong Han <2045gemini@gmail.com> wrote:

> Commit 319c15174757 ("epoll: take epitem list out of struct file")
> intentionally removed temporary file references from the reverse path
> check list. At the time, both epitems and their files were freed after
> an RCU grace period, so unlist_file() could obtain file->f_lock through
> an epitem while clear_tfile_check_list() held rcu_read_lock().
> 
> Commit 0ede61d8589c ("file: convert to SLAB_TYPESAFE_BY_RCU") made
> struct file SLAB_TYPESAFE_BY_RCU and removed its RCU-delayed freeing.
> RCU still protects the epitem, but no longer keeps the referenced file
> from being freed and reused. A concurrent close can therefore make
> unlist_file() lock or unlock f_lock in a recycled file object.
> 
> This violates the documented SLAB_TYPESAFE_BY_RCU rule requiring a
> reference before acquiring an object's lock. The race was reproduced,
> causing a wild unlock of f_lock in a recycled file and breaking its
> mutual exclusion.
> 
> Add ->file to epitems_head to remember the pinned file independently of
> ->epitems. A concurrent EPOLL_CTL_DEL can empty ->epitems before the head  
> is unlisted, leaving no epi->ffd.file from which to drop the reference.
> 
> In list_file(), acquire the reference before adding the head to the
> check list. The caller either owns a reference or holds the ep->mtx for
> the epitem leading to the file. In the latter case, file_ref_get() can
> fail after the last reference is dropped, but eventpoll_release_file()
> must acquire the same mutex before the file can be freed. The dying leaf
> can be skipped because removing links cannot increase the reverse path
> count.
> 
> In unlist_file(), epnested_mutex excludes another list_file() or
> unlist_file(), while head->next prevents a concurrent EPOLL_CTL_DEL from
> freeing the head. Save head->file locally, clear it with head->next
> under f_lock, and drop the reference after the RCU-protected operation.

That seems to hold a reference to 'file' that persists after EPOLL_CTL_ADD
returns.
That breaks the requirement that the final close() will automagically
remove an fd from any epoll lists.

	David

> 
> Reported-by: Qi Tang <tpluszz77@gmail.com>
> Reported-by: Junxi Qian <qjx1298677004@gmail.com>
> Fixes: 0ede61d8589c ("file: convert to SLAB_TYPESAFE_BY_RCU")
> Cc: stable@vger.kernel.org
> Signed-off-by: Guidong Han <2045gemini@gmail.com>
> ---
> SLAB_TYPESAFE_BY_RCU allows a slab slot to be reused while an RCU reader
> still holds its old address. Once that address contains a new live
> struct file, KASAN sees valid, unpoisoned memory and cannot distinguish
> the stale object identity. CONFIG_DEBUG_SPINLOCK exposes the failure
> instead.
> 
> The failing interleaving is:
> 
> CPU0: nested EPOLL_CTL_ADD             CPU1: close/open churn
> ------------------------------------   ---------------------------------
> p = hlist_first_rcu(&head->epitems)
> epi = container_of(p, ...)
>                                        close(victim)
>                                          __fput()
>                                            eventpoll_release_file()
>                                            file_free(victim)
>                                        // the slot is free; f_lock remains
> spin_lock(&epi->ffd.file->f_lock)
>                                        open() reuses the slot as new_file
>                                          spin_lock_init(&new_file->f_lock)
> spin_unlock(&epi->ffd.file->f_lock)     // wild unlock of new_file's lock
> 
> CONFIG_DEBUG_SPINLOCK reports:
> 
> BUG: spinlock already unlocked on CPU#0, poc_unlist/150
>  lock: 0xffff8880067fb200, .magic: dead4ead, .owner: <none>/-1, .owner_cpu: -1
> CPU: 0 UID: 1000 PID: 150 Comm: poc_unlist Not tainted 7.2.0-rc3-dirty #22 PREEMPTLAZY 
> Hardware name: QEMU Ubuntu 24.04 PC v2 (i440FX + PIIX, arch_caps fix, 1996), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
> Call Trace:
>  <TASK>
>  dump_stack_lvl+0x64/0x80
>  do_raw_spin_unlock+0x75/0xb0
>  _raw_spin_unlock+0xe/0x30
>  clear_tfile_check_list+0x88/0xe0
>  do_epoll_ctl_file+0x519/0xcf0
>  ? __pfx_ep_ptable_queue_proc+0x10/0x10
>  do_epoll_ctl+0x8f/0x100
>  __x64_sys_epoll_ctl+0x6f/0xa0
>  do_syscall_64+0xdc/0x520
>  ? srso_alias_return_thunk+0x5/0xfbef5
>  entry_SYSCALL_64_after_hwframe+0x76/0x7e
> RIP: 0033:0x42034e
> Code: 48 c7 c1 b8 ff ff ff f7 d8 64 89 01 48 83 c8 ff c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa 49 89 ca b8 e9 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 b8 ff ff ff f7 d8 64 89 01 48
> RSP: 002b:00007a657ff3c198 EFLAGS: 00000202 ORIG_RAX: 00000000000000e9
> RAX: ffffffffffffffda RBX: 00007a657ff3ccdc RCX: 000000000042034e
> RDX: 0000000000000003 RSI: 0000000000000001 RDI: 0000000000000004
> RBP: 00007a657ff3c2f0 R08: 0000000000000000 R09: 00007a657ff3c6c0
> R10: 00007a657ff3c1a4 R11: 0000000000000202 R12: 00007a657ff3c6c0
> R13: ffffffffffffffb8 R14: 000000000000000d R15: 00007fffb7de0210
>  </TASK>
> ------------[ cut here ]------------
> 
> unlist_file() does not appear as a separate frame because it was inlined
> into clear_tfile_check_list(). This report was obtained with mdelay()
> instrumentation immediately before spin_lock() and spin_unlock() in
> unlist_file() to widen the two race windows.
> 
> More importantly, this is a wild unlock. The stale unlock can target
> f_lock of a different live file and invalidate mutual exclusion for
> state protected by that lock. Turning this into a reliable exploit
> would require precise scheduling and same-slot reuse and is likely
> difficult, but the primitive is potentially exploitable.
> 
> Qi Tang and Junxi Qian helped identify this issue and are therefore
> credited by the Reported-by tags above.
> ---
>  fs/eventpoll.c | 18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/eventpoll.c b/fs/eventpoll.c
> index 0e65c7431dfc..eed8cecd94e3 100644
> --- a/fs/eventpoll.c
> +++ b/fs/eventpoll.c
> @@ -459,11 +459,14 @@ static struct kmem_cache *pwq_cache __ro_after_init;
>   * Wrapper anchor for file->f_ep when the watched file is not itself an
>   * eventpoll; for the epoll-watches-epoll case, file->f_ep points at
>   * &watched_ep->refs directly. The ->next field threads
> - * ctx->tfile_check_list during one EPOLL_CTL_ADD path check.
> + * ctx->tfile_check_list during one EPOLL_CTL_ADD path check. The ->file
> + * field holds a reference to the associated file while the head is on
> + * the list.
>   */
>  struct epitems_head {
>  	struct hlist_head epitems;
>  	struct epitems_head *next;
> +	struct file *file;
>  };
>  
>  static struct kmem_cache *ephead_cache __ro_after_init;
> @@ -480,6 +483,16 @@ static void list_file(struct file *file, struct ep_ctl_ctx *ctx)
>  
>  	head = container_of(file->f_ep, struct epitems_head, epitems);
>  	if (!head->next) {
> +		/*
> +		 * The caller owns a reference to @file or holds the ep->mtx for the
> +		 * epitem that led here. The latter blocks eventpoll_release_file()
> +		 * before the file allocation can be freed and reused. A dying leaf
> +		 * can be skipped since removing links cannot increase the reverse
> +		 * path count.
> +		 */
> +		if (!file_ref_get(&file->f_ref))
> +			return;
> +		head->file = file;
>  		head->next = ctx->tfile_check_list;
>  		ctx->tfile_check_list = head;
>  	}
> @@ -489,15 +502,18 @@ static void unlist_file(struct epitems_head *head)
>  {
>  	struct epitems_head *to_free = head;
>  	struct hlist_node *p = rcu_dereference(hlist_first_rcu(&head->epitems));
> +	struct file *file = head->file;
>  	if (p) {
>  		struct epitem *epi= container_of(p, struct epitem, fllink);
>  		spin_lock(&epi->ffd.file->f_lock);
>  		if (!hlist_empty(&head->epitems))
>  			to_free = NULL;
>  		head->next = NULL;
> +		head->file = NULL;
>  		spin_unlock(&epi->ffd.file->f_lock);
>  	}
>  	free_ephead(to_free);
> +	fput(file);
>  }
>  
>  #ifdef CONFIG_SYSCTL

Re: [PATCH] eventpoll: pin files while checking reverse paths

[Mateusz Guzik]

On Sat, Jul 18, 2026 at 06:44:06PM +0800, Guidong Han wrote:
> Commit 319c15174757 ("epoll: take epitem list out of struct file")
> intentionally removed temporary file references from the reverse path
> check list. At the time, both epitems and their files were freed after
> an RCU grace period, so unlist_file() could obtain file->f_lock through
> an epitem while clear_tfile_check_list() held rcu_read_lock().
> 
> Commit 0ede61d8589c ("file: convert to SLAB_TYPESAFE_BY_RCU") made
> struct file SLAB_TYPESAFE_BY_RCU and removed its RCU-delayed freeing.
> RCU still protects the epitem, but no longer keeps the referenced file
> from being freed and reused. A concurrent close can therefore make
> unlist_file() lock or unlock f_lock in a recycled file object.
> 
> This violates the documented SLAB_TYPESAFE_BY_RCU rule requiring a
> reference before acquiring an object's lock. The race was reproduced,
> causing a wild unlock of f_lock in a recycled file and breaking its
> mutual exclusion.
> 
> Add ->file to epitems_head to remember the pinned file independently of
> ->epitems. A concurrent EPOLL_CTL_DEL can empty ->epitems before the head
> is unlisted, leaving no epi->ffd.file from which to drop the reference.
> 
> In list_file(), acquire the reference before adding the head to the
> check list. The caller either owns a reference or holds the ep->mtx for
> the epitem leading to the file. In the latter case, file_ref_get() can
> fail after the last reference is dropped, but eventpoll_release_file()
> must acquire the same mutex before the file can be freed. The dying leaf
> can be skipped because removing links cannot increase the reverse path
> count.
> 
> In unlist_file(), epnested_mutex excludes another list_file() or
> unlist_file(), while head->next prevents a concurrent EPOLL_CTL_DEL from
> freeing the head. Save head->file locally, clear it with head->next
> under f_lock, and drop the reference after the RCU-protected operation.
> 
> Reported-by: Qi Tang <tpluszz77@gmail.com>
> Reported-by: Junxi Qian <qjx1298677004@gmail.com>
> Fixes: 0ede61d8589c ("file: convert to SLAB_TYPESAFE_BY_RCU")
> Cc: stable@vger.kernel.org
> Signed-off-by: Guidong Han <2045gemini@gmail.com>
> ---
> SLAB_TYPESAFE_BY_RCU allows a slab slot to be reused while an RCU reader
> still holds its old address. Once that address contains a new live
> struct file, KASAN sees valid, unpoisoned memory and cannot distinguish
> the stale object identity. CONFIG_DEBUG_SPINLOCK exposes the failure
> instead.
> 
> The failing interleaving is:
> 
> CPU0: nested EPOLL_CTL_ADD             CPU1: close/open churn
> ------------------------------------   ---------------------------------
> p = hlist_first_rcu(&head->epitems)
> epi = container_of(p, ...)
>                                        close(victim)
>                                          __fput()
>                                            eventpoll_release_file()
>                                            file_free(victim)
>                                        // the slot is free; f_lock remains
> spin_lock(&epi->ffd.file->f_lock)
>                                        open() reuses the slot as new_file
>                                          spin_lock_init(&new_file->f_lock)
> spin_unlock(&epi->ffd.file->f_lock)     // wild unlock of new_file's lock
> 
> CONFIG_DEBUG_SPINLOCK reports:
> 
> BUG: spinlock already unlocked on CPU#0, poc_unlist/150


Well I'm no expert on epoll, but on cursory reading I think this is
fixable in a simpler manner and without the extra overhead.

If you were to add a constructor to the files slab, you could move the
spinlock and mutex initialization into it. Then unlock can safely
proceed and maybe that's good enough (as in there are no other issues
stemming from the typesafe thing)?

Re: [PATCH] eventpoll: pin files while checking reverse paths

[Guidong Han]

On Fri, Jul 24, 2026 at 2:06 AM David Laight
<david.laight.linux@gmail.com> wrote:
>
> On Sat, 18 Jul 2026 18:44:06 +0800
> Guidong Han <2045gemini@gmail.com> wrote:
>
> > Commit 319c15174757 ("epoll: take epitem list out of struct file")
> > intentionally removed temporary file references from the reverse path
> > check list. At the time, both epitems and their files were freed after
> > an RCU grace period, so unlist_file() could obtain file->f_lock through
> > an epitem while clear_tfile_check_list() held rcu_read_lock().
> >
> > Commit 0ede61d8589c ("file: convert to SLAB_TYPESAFE_BY_RCU") made
> > struct file SLAB_TYPESAFE_BY_RCU and removed its RCU-delayed freeing.
> > RCU still protects the epitem, but no longer keeps the referenced file
> > from being freed and reused. A concurrent close can therefore make
> > unlist_file() lock or unlock f_lock in a recycled file object.
> >
> > This violates the documented SLAB_TYPESAFE_BY_RCU rule requiring a
> > reference before acquiring an object's lock. The race was reproduced,
> > causing a wild unlock of f_lock in a recycled file and breaking its
> > mutual exclusion.
> >
> > Add ->file to epitems_head to remember the pinned file independently of
> > ->epitems. A concurrent EPOLL_CTL_DEL can empty ->epitems before the head
> > is unlisted, leaving no epi->ffd.file from which to drop the reference.
> >
> > In list_file(), acquire the reference before adding the head to the
> > check list. The caller either owns a reference or holds the ep->mtx for
> > the epitem leading to the file. In the latter case, file_ref_get() can
> > fail after the last reference is dropped, but eventpoll_release_file()
> > must acquire the same mutex before the file can be freed. The dying leaf
> > can be skipped because removing links cannot increase the reverse path
> > count.
> >
> > In unlist_file(), epnested_mutex excludes another list_file() or
> > unlist_file(), while head->next prevents a concurrent EPOLL_CTL_DEL from
> > freeing the head. Save head->file locally, clear it with head->next
> > under f_lock, and drop the reference after the RCU-protected operation.
>
> That seems to hold a reference to 'file' that persists after EPOLL_CTL_ADD
> returns.

Thanks for checking.

The reference does not persist after EPOLL_CTL_ADD returns. I went back
and carefully audited both list_file() call sites and every subsequent
exit path. Whenever list_file() adds a head to tfile_check_list,
clear_tfile_check_list() drops the temporary reference before
do_epoll_ctl_file() returns, including on error paths.

Re: [PATCH] eventpoll: pin files while checking reverse paths

[Guidong Han]

On Fri, Jul 24, 2026 at 3:51 AM Mateusz Guzik <mjguzik@gmail.com> wrote:
>
> On Sat, Jul 18, 2026 at 06:44:06PM +0800, Guidong Han wrote:
> > Commit 319c15174757 ("epoll: take epitem list out of struct file")
> > intentionally removed temporary file references from the reverse path
> > check list. At the time, both epitems and their files were freed after
> > an RCU grace period, so unlist_file() could obtain file->f_lock through
> > an epitem while clear_tfile_check_list() held rcu_read_lock().
> >
> > Commit 0ede61d8589c ("file: convert to SLAB_TYPESAFE_BY_RCU") made
> > struct file SLAB_TYPESAFE_BY_RCU and removed its RCU-delayed freeing.
> > RCU still protects the epitem, but no longer keeps the referenced file
> > from being freed and reused. A concurrent close can therefore make
> > unlist_file() lock or unlock f_lock in a recycled file object.
> >
> > This violates the documented SLAB_TYPESAFE_BY_RCU rule requiring a
> > reference before acquiring an object's lock. The race was reproduced,
> > causing a wild unlock of f_lock in a recycled file and breaking its
> > mutual exclusion.
> >
> > Add ->file to epitems_head to remember the pinned file independently of
> > ->epitems. A concurrent EPOLL_CTL_DEL can empty ->epitems before the head
> > is unlisted, leaving no epi->ffd.file from which to drop the reference.
> >
> > In list_file(), acquire the reference before adding the head to the
> > check list. The caller either owns a reference or holds the ep->mtx for
> > the epitem leading to the file. In the latter case, file_ref_get() can
> > fail after the last reference is dropped, but eventpoll_release_file()
> > must acquire the same mutex before the file can be freed. The dying leaf
> > can be skipped because removing links cannot increase the reverse path
> > count.
> >
> > In unlist_file(), epnested_mutex excludes another list_file() or
> > unlist_file(), while head->next prevents a concurrent EPOLL_CTL_DEL from
> > freeing the head. Save head->file locally, clear it with head->next
> > under f_lock, and drop the reference after the RCU-protected operation.
> >
> > Reported-by: Qi Tang <tpluszz77@gmail.com>
> > Reported-by: Junxi Qian <qjx1298677004@gmail.com>
> > Fixes: 0ede61d8589c ("file: convert to SLAB_TYPESAFE_BY_RCU")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Guidong Han <2045gemini@gmail.com>
> > ---
> > SLAB_TYPESAFE_BY_RCU allows a slab slot to be reused while an RCU reader
> > still holds its old address. Once that address contains a new live
> > struct file, KASAN sees valid, unpoisoned memory and cannot distinguish
> > the stale object identity. CONFIG_DEBUG_SPINLOCK exposes the failure
> > instead.
> >
> > The failing interleaving is:
> >
> > CPU0: nested EPOLL_CTL_ADD             CPU1: close/open churn
> > ------------------------------------   ---------------------------------
> > p = hlist_first_rcu(&head->epitems)
> > epi = container_of(p, ...)
> >                                        close(victim)
> >                                          __fput()
> >                                            eventpoll_release_file()
> >                                            file_free(victim)
> >                                        // the slot is free; f_lock remains
> > spin_lock(&epi->ffd.file->f_lock)
> >                                        open() reuses the slot as new_file
> >                                          spin_lock_init(&new_file->f_lock)
> > spin_unlock(&epi->ffd.file->f_lock)     // wild unlock of new_file's lock
> >
> > CONFIG_DEBUG_SPINLOCK reports:
> >
> > BUG: spinlock already unlocked on CPU#0, poc_unlist/150
>
>
> Well I'm no expert on epoll, but on cursory reading I think this is
> fixable in a simpler manner and without the extra overhead.
>
> If you were to add a constructor to the files slab, you could move the
> spinlock and mutex initialization into it. Then unlock can safely
> proceed and maybe that's good enough (as in there are no other issues
> stemming from the typesafe thing)?

Thanks for the suggestion.

I also looked at other struct file users while investigating this. A
constructor would make this particular stale lock/unlock safe, but it is
not a general solution to stale file identity.

For example, commit a6dc643c6931 ("eventpoll: fix ep_remove struct
eventpoll / struct file UAF") used file->f_ep and is_file_epoll(file).
Immediate reuse could select the wrong free path and cause an invalid
free. This case happens to use the stale file pointer only for f_lock.

I therefore preferred pinning the file, which keeps its identity stable,
limits the change to eventpoll, and is straightforward to backport.