[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