Skip to content

Commit 111f7d4

Browse files
name2965NipaLocal
authored and
NipaLocal
committed
ptp: remove ptp->n_vclocks check logic in ptp_vclock_in_use()
There is no disagreement that we should check both ptp->is_virtual_clock and ptp->n_vclocks to check if the ptp virtual clock is in use. However, when we acquire ptp->n_vclocks_mux to read ptp->n_vclocks in ptp_vclock_in_use(), we observe a recursive lock in the call trace starting from n_vclocks_store(). ============================================ WARNING: possible recursive locking detected 6.15.0-rc6 kernel-patches#1 Not tainted -------------------------------------------- syz.0.1540/13807 is trying to acquire lock: ffff888035a24868 (&ptp->n_vclocks_mux){+.+.}-{4:4}, at: ptp_vclock_in_use drivers/ptp/ptp_private.h:103 [inline] ffff888035a24868 (&ptp->n_vclocks_mux){+.+.}-{4:4}, at: ptp_clock_unregister+0x21/0x250 drivers/ptp/ptp_clock.c:415 but task is already holding lock: ffff888030704868 (&ptp->n_vclocks_mux){+.+.}-{4:4}, at: n_vclocks_store+0xf1/0x6d0 drivers/ptp/ptp_sysfs.c:215 other info that might help us debug this: Possible unsafe locking scenario: CPU0 ---- lock(&ptp->n_vclocks_mux); lock(&ptp->n_vclocks_mux); *** DEADLOCK *** .... ============================================ The best way to solve this is to remove the logic that checks ptp->n_vclocks in ptp_vclock_in_use(). The reason why this is appropriate is that any path that uses ptp->n_vclocks must unconditionally check if ptp->n_vclocks is greater than 0 before unregistering vclocks, and all functions are already written this way. And in the function that uses ptp->n_vclocks, we already get ptp->n_vclocks_mux before unregistering vclocks. Therefore, we need to remove the redundant check for ptp->n_vclocks in ptp_vclock_in_use() to prevent recursive locking. Fixes: 73f3706 ("ptp: support ptp physical/virtual clocks conversion") Signed-off-by: Jeongjun Park <[email protected]> Signed-off-by: NipaLocal <nipa@local>
1 parent 3c058ca commit 111f7d4

File tree

2 files changed

+2
-13
lines changed

2 files changed

+2
-13
lines changed

drivers/ptp/ptp_clock.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -412,9 +412,8 @@ static int unregister_vclock(struct device *dev, void *data)
412412

413413
int ptp_clock_unregister(struct ptp_clock *ptp)
414414
{
415-
if (ptp_vclock_in_use(ptp)) {
415+
if (ptp_vclock_in_use(ptp))
416416
device_for_each_child(&ptp->dev, NULL, unregister_vclock);
417-
}
418417

419418
ptp->defunct = 1;
420419
wake_up_interruptible(&ptp->tsev_wq);

drivers/ptp/ptp_private.h

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -98,17 +98,7 @@ static inline int queue_cnt(const struct timestamp_event_queue *q)
9898
/* Check if ptp virtual clock is in use */
9999
static inline bool ptp_vclock_in_use(struct ptp_clock *ptp)
100100
{
101-
bool in_use = false;
102-
103-
if (mutex_lock_interruptible(&ptp->n_vclocks_mux))
104-
return true;
105-
106-
if (!ptp->is_virtual_clock && ptp->n_vclocks)
107-
in_use = true;
108-
109-
mutex_unlock(&ptp->n_vclocks_mux);
110-
111-
return in_use;
101+
return !ptp->is_virtual_clock;
112102
}
113103

114104
/* Check if ptp clock shall be free running */

0 commit comments

Comments
 (0)