ceph: fix d_lock leak in debugfs mdsc_show()
Subsystems (2)
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.
--- fs/ceph/debugfs.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
fs/ceph/debugfs.c +10 -0
@@ -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