Skip to contentSkip to conversation

ceph: fix d_lock leak in debugfs mdsc_show()

PATCH New Max Kellermann 1 participant 1 message Nov 11

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 ceph: fix d_lock leak in debugfs mdsc_show()

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")
---
 fs/ceph/debugfs.c | 10 ++++++++++
 1 file changed, 10 insertions(+)
fs/ceph/debugfs.c +10 -0
index 6a9a5551d9d7..83506d9207fa 100644
@@ -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);
Signature · 1 line
2.53.0