Re: [PATCH 1/2] ceph: fix UAF in __kick_flushing_caps() on cf entry freed during unlock
From: Viacheslav Dubeyko
Date: Thu Jul 16 2026 - 13:42:20 EST
On Tue, 2026-07-14 at 16:13 +0800, Xiubo Li via B4 Relay wrote:
> From: Xiubo Li <xiubo.li@xxxxxxxxx>
>
> list_for_each_entry() iterates ci->i_cap_flush_list but drops
> i_ceph_lock to send cap messages. During the unlock window,
> handle_cap_flush_ack() can acquire i_ceph_lock, detach cf entries
> with tid <= flush_tid from the list, release i_ceph_lock, and free
> them via ceph_free_cap_flush() outside any lock. When the original
> thread reacquires i_ceph_lock and the for-loop macro advances via
> cf = list_next_entry(cf, i_list), it dereferences cf->i_list.next
> on freed memory.
>
> The race timeline:
>
> __kick_flushing_caps() handle_cap_flush_ack()
> ----------------------- -----------------------
> holds i_ceph_lock <---
> iterates to cf (tid=10)
> prepares FLUSH message
> drops i_ceph_lock <---
> __send_cap() ── FLUSH(tid=10)
> MDS sends FLUSH_ACK(tid=10)
> ---> acquires i_ceph_lock
> cf->tid(10) <= flush_tid(10),
> detaches cf from
> i_cap_flush_list
> drops i_ceph_lock
> ceph_free_cap_flush(cf) <-
> frees it!
> acquires i_ceph_lock <---
> for-loop advances:
> cf = list_next_entry(cf, i_list)
> -- UAF on freed cf->i_list.next
>
> The cf was just sent by __kick_flushing_caps itself via __send_cap().
> The MDS may respond with FLUSH_ACK quickly enough that
> handle_cap_flush_ack() frees cf before __kick_flushing_caps can
> finish the iteration.
>
> Fix by converting to a manual while loop: save the next pointer
> under i_ceph_lock before dropping it, then use the saved pointer
> after reacquiring, so the potentially-freed cf is never accessed
> again.
>
> Signed-off-by: Xiubo Li <xiubo.li@xxxxxxxxx>
> ---
> fs/ceph/caps.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c
> index f8d898ad091e..358ee4f14524 100644
> --- a/fs/ceph/caps.c
> +++ b/fs/ceph/caps.c
> @@ -2626,9 +2626,14 @@ static void __kick_flushing_caps(struct
> ceph_mds_client *mdsc,
> }
> }
>
> - list_for_each_entry(cf, &ci->i_cap_flush_list, i_list) {
> - if (cf->tid < first_tid)
> + cf = list_first_entry(&ci->i_cap_flush_list, struct
> ceph_cap_flush, i_list);
> + while (&cf->i_list != &ci->i_cap_flush_list) {
> + struct ceph_cap_flush *next;
> +
> + if (cf->tid < first_tid) {
> + cf = list_next_entry(cf, i_list);
> continue;
> + }
>
> cap = ci->i_auth_cap;
> if (!(cap && cap->session == session)) {
> @@ -2638,6 +2643,7 @@ static void __kick_flushing_caps(struct
> ceph_mds_client *mdsc,
> }
>
> first_tid = cf->tid + 1;
> + next = list_next_entry(cf, i_list);
>
> if (!cf->is_capsnap) {
> struct cap_msg_args arg;
> @@ -2678,6 +2684,7 @@ static void __kick_flushing_caps(struct
> ceph_mds_client *mdsc,
> }
>
> spin_lock(&ci->i_ceph_lock);
> + cf = next;
> }
> }
>
I don't see any issues with the patch.
Reviewed-by: Viacheslav Dubeyko <slava@xxxxxxxxxxx>
Thanks,
Slava.