Re: [PATCH next] eventpoll: delay file list memory deallocation until unlisting

[Christian Brauner]

On Sun, May 24, 2026 at 06:27:59PM +0800, Edward Adam Davis wrote:
> Commit e09c77d94003 implicitly sets the head->next value within list_file()
> to NULL; this occurs because ctx->tfile_check_list which resides on the stack
> is initialized to NULL. This introduces a potential risk to ep_remove_file(),
> as the decision to reclaim epitems_head depends solely on whether the next
> pointer is NULL. Prior to the introduction of e09c77d94003, the presence of
> the sentinel value EP_UNACTIVE_PTR prevented the next pointer from becoming
> NULL prematurely; the next value would only be updated to NULL during the
> execution of unlist_file(). 
> However, following the introduction of e09c77d94003, list_file() also updates
> the next value to NULL, which ultimately led to the uaf reported in [1].
> 
> To mitigate the risk posed by the potentially NULL next pointer, the memory
> reclamation for the file list originally performed within ep_remove_file()
> has been deferred to unlist_file().
> 
> [1]
> BUG: KASAN: slab-use-after-free in clear_tfile_check_list+0x114/0x380 fs/eventpoll.c:2443
> Read of size 8 at addr ffff88803f021568 by task syz.0.74/5985
> Call Trace:
>  clear_tfile_check_list+0x114/0x380 fs/eventpoll.c:2443
>  do_epoll_ctl_file+0x8fd/0xed0 fs/eventpoll.c:-1
>  
> Allocated by task 5985:
>  ep_attach_file fs/eventpoll.c:1751 [inline]
>  ep_register_epitem fs/eventpoll.c:1833 [inline]
>  ep_insert+0x512/0x1820 fs/eventpoll.c:1876
>  do_epoll_ctl_file+0x8bb/0xed0 fs/eventpoll.c:2651
>  
> Freed by task 5985:
>  kmem_cache_free+0x187/0x6c0 mm/slub.c:6411
>  ep_remove+0x155/0x2a0 fs/eventpoll.c:1135
>  ep_insert+0x1372/0x1820 fs/eventpoll.c:-1
>  do_epoll_ctl_file+0x8bb/0xed0 fs/eventpoll.c:2651
> 
> Fixes: e09c77d94003 ("eventpoll: hoist CTL_ADD scratch state into struct ep_ctl_ctx")
> Reported-by: syzbot+69a3d7738ad3aa175caf@xxxxxxxxxxxxxxxxxxxxxxxxx
> Closes: https://syzkaller.appspot.com/bug?extid=69a3d7738ad3aa175caf
> Signed-off-by: Edward Adam Davis <eadavis@xxxxxx>
> ---
>  fs/eventpoll.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/fs/eventpoll.c b/fs/eventpoll.c
> index a569e98d4a99..66aa4f200909 100644
> --- a/fs/eventpoll.c
> +++ b/fs/eventpoll.c
> @@ -1080,6 +1080,8 @@ static void ep_remove_file(struct eventpoll *ep, struct epitem *epi,
>  			v = container_of(head, struct epitems_head, epitems);
>  			if (!smp_load_acquire(&v->next))
>  				to_free = v;
> +			if (!hlist_empty(&v->epitems))
> +				to_free = NULL;

Hm, at the singular-node check, hlist_del_rcu hasn't run yet so
!hlist_empty(&v->epitems) is unconditionally true. So this stops freeing
heads in all cases. Heads that were never on ctx->tfile_check_list
(CTL_ADD without full_check, or any CTL_DEL of a previously-unlisted
head) leak permanently.

Wouldn't something like:

  diff --git a/fs/eventpoll.c b/fs/eventpoll.c
  @@ static int reverse_path_check(struct ep_ctl_ctx *ctx)
  -     for (p = ctx->tfile_check_list; p; p = p->next) {
  +     for (p = ctx->tfile_check_list; p != EP_UNACTIVE_PTR; p = p->next) {

  @@ static void clear_tfile_check_list(struct ep_ctl_ctx *ctx)
        rcu_read_lock();
  -     while (ctx->tfile_check_list) {
  +     while (ctx->tfile_check_list != EP_UNACTIVE_PTR) {
                struct epitems_head *head = ctx->tfile_check_list;
                ctx->tfile_check_list = head->next;

  @@ int do_epoll_ctl_file(...)
  -     struct ep_ctl_ctx ctx = { };
  +     struct ep_ctl_ctx ctx = {
  +             .tfile_check_list = EP_UNACTIVE_PTR,
  +     };

be better?