[PATCH] ceph: fix d_lock leak in debugfs mdsc_show()

[PATCH] ceph: fix d_lock leak in debugfs mdsc_show()

[Max Kellermann]

mdsc_show() walks the request tree under `mdsc->mutex` and prints the
request's dentries like this:

  spin_lock(&req->r_old_dentry->d_lock);
  seq_printf(...);
  spin_unlock(&req->r_old_dentry->d_lock);

(same for `r_dentry`).  The pointer is loaded again for the
unlock, but ceph_fill_trace() can change it in the meantime:
ceph_fill_trace() runs with req->r_fill_mutex held, but not
mdsc->mutex, and:

- after d_move() for a rename, it swaps `r_dentry` and `r_old_dentry`
- splice_dentry() replaces `r_dentry` with the spliced alias and drops
  the request's reference to the old dentry
- a lookup without a locked parent assigns a new `r_dentry`

If an unsafe rename reply is being processed while mdsc_show() is
between spin_lock() and spin_unlock(), mdsc_show() unlocks the other
dentry, and the `d_lock` of the one it locked is never released.  The
next attempt to lock it spins forever.  I have seen this on a
production server, where our monitoring reads the debugfs "mdsc" file
periodically:

 watchdog: BUG: soft lockup - CPU#67 stuck for 48s! [kworker/67:2:3746175]
 Workqueue: ceph-msgr ceph_con_workfn
 RIP: 0010:queued_spin_lock_slowpath+0x4a/0x240
 Call Trace:
  <TASK>
  ceph_fill_trace+0x7a4/0x910
  mds_dispatch+0x9f4/0x1ac0
  ceph_con_process_message+0x66/0x80
  ceph_con_v1_try_read+0x392/0x17e0
  ceph_con_workfn+0x329/0x590
  process_one_work+0x146/0x2b0
  worker_thread+0x182/0x2f0
  kthread+0xe4/0x1d0

 watchdog: BUG: soft lockup - CPU#55 stuck for 49s! [cm4all-kernel-e:16346]
 RIP: 0010:queued_spin_lock_slowpath+0x1a2/0x240
 Call Trace:
  <TASK>
  lockref_get+0x47/0x60
  ceph_mdsc_build_path.part.0+0x85/0x2a0
  mdsc_show+0x239/0x3e0
  seq_read_iter+0x11f/0x480
  seq_read+0xfd/0x130
  full_proxy_read+0x50/0x80
  vfs_read+0xa4/0x340

Both CPUs were waiting for the `d_lock` of the same dentry, which
nobody is locked but nobody will unlock it.  The first CPU was in
update_dentry_lease(), called after the rename swap.  The second one
was the next read of the debugfs file.  Since the ceph-msgr worker was
blocked, all requests to that MDS hung.

Replacing `r_dentry` with a spliced alias also means mdsc_show() can
call dget() on a dentry whose last reference has just been dropped.

Fix this by holding `r_fill_mutex` while mdsc_show() uses `r_dentry`
and `r_old_dentry`.  Taking r_fill_mutex with mdsc->mutex held is the
established lock order; __do_request(), handle_forward(),
ceph_mdsc_wait_request() and ceph_lock_wait_for_completion() do the
same.

Fixes: 76aa844d5b2f ("ceph: debugfs")
Cc: stable@vger.kernel.org
Signed-off-by: Max Kellermann <max.kellermann@ionos.com>
---
 fs/ceph/debugfs.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/fs/ceph/debugfs.c b/fs/ceph/debugfs.c
index 6a9a5551d9d7..83506d9207fa 100644
--- a/fs/ceph/debugfs.c
+++ b/fs/ceph/debugfs.c
@@ -109,6 +109,14 @@ static int mdsc_show(struct seq_file *s, void *p)
 		else
 			seq_puts(s, "\t");
 
+		/*
+		 * ceph_fill_trace() may replace r_dentry (dropping the
+		 * request's reference to the old one) or swap r_dentry
+		 * and r_old_dentry; it does so while holding
+		 * r_fill_mutex, so hold it while using them.
+		 */
+		mutex_lock(&req->r_fill_mutex);
+
 		if (req->r_inode) {
 			seq_printf(s, " #%llx", ceph_ino(req->r_inode));
 		} else if (req->r_dentry) {
@@ -151,6 +159,8 @@ static int mdsc_show(struct seq_file *s, void *p)
 				seq_printf(s, " %s", req->r_path2);
 		}
 
+		mutex_unlock(&req->r_fill_mutex);
+
 		seq_puts(s, "\n");
 	}
 	mutex_unlock(&mdsc->mutex);
-- 
2.53.0

Re: [PATCH] ceph: fix d_lock leak in debugfs mdsc_show()

[Xiubo Li]

Hi Max,

Good catch.

I reproduced this on a KASAN + lockdep 7.4-rc1 kernel against a vstart
cluster.  The real window is only a few instructions wide, because
ceph_invalidate_dentry_lease() takes dn->d_lock right after d_move(),
so I widened it with a test-only udelay(2000) in ceph_fill_trace()
between the lease invalidation and the r_dentry/r_old_dentry swap,
plus a 50us delay inside the d_lock sections of mdsc_show().  The load
was 4 tasks doing rename() back and forth while 2 tasks read the
debugfs "mdsc" file in a loop.

Without the patch it locks up within seconds.  The check in mdsc_show()
fires first:

  mdsc_show tid 229 op 4612 r_dentry changed ffff888209d8acb0 ->
ffff8882939b92e8 while d_lock held

and then:

  watchdog: BUG: soft lockup - CPU#4 stuck for 26s! [cat:30609]
   _raw_spin_lock+0x73/0x80
   lockref_get+0xd/0x50
   ceph_mdsc_build_path+0x23e/0x1560 [ceph]
   mdsc_show+0x242/0xab0 [ceph]
   seq_read_iter+0x2e9/0x12d0

The ceph-msgr worker is stuck in handle_reply() waiting for
mdsc->mutex, which the reader holds while it spins.  So this is the
same leaked d_lock as in your report, with the two CPUs arriving in the
other order.

With the patch and the same delays, 120s and ~6300 renames went through
ceph_fill_trace() with no warnings.  bpftrace showed mdsc_show() taking
1-8ms, so the readers really did wait out the window on r_fill_mutex.
A further 180s of renames plus fsstress without the test delays was
also clean.  lockdep stayed enabled throughout, recorded the new
&mdsc->mutex -> &req->r_fill_mutex dependency and reported nothing.

One side effect, not a problem in my view: a debugfs reader now waits
for any reply that is being filled (including readdir_prepopulate on a
large directory) while it holds mdsc->mutex, so it can stall request
handling on that client for that long.  The abort paths you listed
already do the same, and this is debugfs, so I think it is fine.  If
anyone cares, mutex_trylock() with a placeholder in the output would
avoid it.

Reviewed-by: Xiubo Li <xiubo.li@clyso.com>
Tested-by: Xiubo Li <xiubo.li@clyso.com>

Thanks
- Xiubo

On Wed, 11 Nov 2026 at 12:30, Max Kellermann <max.kellermann@ionos.com> wrote:
>
> mdsc_show() walks the request tree under `mdsc->mutex` and prints the
> request's dentries like this:
>
>   spin_lock(&req->r_old_dentry->d_lock);
>   seq_printf(...);
>   spin_unlock(&req->r_old_dentry->d_lock);
>
> (same for `r_dentry`).  The pointer is loaded again for the
> unlock, but ceph_fill_trace() can change it in the meantime:
> ceph_fill_trace() runs with req->r_fill_mutex held, but not
> mdsc->mutex, and:
>
> - after d_move() for a rename, it swaps `r_dentry` and `r_old_dentry`
> - splice_dentry() replaces `r_dentry` with the spliced alias and drops
>   the request's reference to the old dentry
> - a lookup without a locked parent assigns a new `r_dentry`
>
> If an unsafe rename reply is being processed while mdsc_show() is
> between spin_lock() and spin_unlock(), mdsc_show() unlocks the other
> dentry, and the `d_lock` of the one it locked is never released.  The
> next attempt to lock it spins forever.  I have seen this on a
> production server, where our monitoring reads the debugfs "mdsc" file
> periodically:
>
>  watchdog: BUG: soft lockup - CPU#67 stuck for 48s! [kworker/67:2:3746175]
>  Workqueue: ceph-msgr ceph_con_workfn
>  RIP: 0010:queued_spin_lock_slowpath+0x4a/0x240
>  Call Trace:
>   <TASK>
>   ceph_fill_trace+0x7a4/0x910
>   mds_dispatch+0x9f4/0x1ac0
>   ceph_con_process_message+0x66/0x80
>   ceph_con_v1_try_read+0x392/0x17e0
>   ceph_con_workfn+0x329/0x590
>   process_one_work+0x146/0x2b0
>   worker_thread+0x182/0x2f0
>   kthread+0xe4/0x1d0
>
>  watchdog: BUG: soft lockup - CPU#55 stuck for 49s! [cm4all-kernel-e:16346]
>  RIP: 0010:queued_spin_lock_slowpath+0x1a2/0x240
>  Call Trace:
>   <TASK>
>   lockref_get+0x47/0x60
>   ceph_mdsc_build_path.part.0+0x85/0x2a0
>   mdsc_show+0x239/0x3e0
>   seq_read_iter+0x11f/0x480
>   seq_read+0xfd/0x130
>   full_proxy_read+0x50/0x80
>   vfs_read+0xa4/0x340
>
> Both CPUs were waiting for the `d_lock` of the same dentry, which
> nobody is locked but nobody will unlock it.  The first CPU was in
> update_dentry_lease(), called after the rename swap.  The second one
> was the next read of the debugfs file.  Since the ceph-msgr worker was
> blocked, all requests to that MDS hung.
>
> Replacing `r_dentry` with a spliced alias also means mdsc_show() can
> call dget() on a dentry whose last reference has just been dropped.
>
> Fix this by holding `r_fill_mutex` while mdsc_show() uses `r_dentry`
> and `r_old_dentry`.  Taking r_fill_mutex with mdsc->mutex held is the
> established lock order; __do_request(), handle_forward(),
> ceph_mdsc_wait_request() and ceph_lock_wait_for_completion() do the
> same.
>
> Fixes: 76aa844d5b2f ("ceph: debugfs")
> Cc: stable@vger.kernel.org
> Signed-off-by: Max Kellermann <max.kellermann@ionos.com>
> ---
>  fs/ceph/debugfs.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/fs/ceph/debugfs.c b/fs/ceph/debugfs.c
> index 6a9a5551d9d7..83506d9207fa 100644
> --- a/fs/ceph/debugfs.c
> +++ b/fs/ceph/debugfs.c
> @@ -109,6 +109,14 @@ static int mdsc_show(struct seq_file *s, void *p)
>                 else
>                         seq_puts(s, "\t");
>
> +               /*
> +                * ceph_fill_trace() may replace r_dentry (dropping the
> +                * request's reference to the old one) or swap r_dentry
> +                * and r_old_dentry; it does so while holding
> +                * r_fill_mutex, so hold it while using them.
> +                */
> +               mutex_lock(&req->r_fill_mutex);
> +
>                 if (req->r_inode) {
>                         seq_printf(s, " #%llx", ceph_ino(req->r_inode));
>                 } else if (req->r_dentry) {
> @@ -151,6 +159,8 @@ static int mdsc_show(struct seq_file *s, void *p)
>                                 seq_printf(s, " %s", req->r_path2);
>                 }
>
> +               mutex_unlock(&req->r_fill_mutex);
> +
>                 seq_puts(s, "\n");
>         }
>         mutex_unlock(&mdsc->mutex);
> --
> 2.53.0
>