RE: [PATCH] tty: ldisc: fix deadlock between ldisc_sem and rtnl_mutex
"Jagielski, Jedrzej" <[email protected]> Thu, 16 Jul 2026 07:59:33 +0000
| Newsgroups | org.kernel.vger.linux-can,org.kernel.vger.linux-kernel,org.kernel.vger.linux-serial,org.kernel.vger.netdev |
|---|---|
| Message-ID | <PH0PR11MB5902C0DC3D8407F2701948C1F0C72@PH0PR11MB5902.namprd11.prod.outlook.com> |
From: Yun Zhou <[email protected]> Sent: Thursday, July 16, 2026 8:47 AM >syzbot reported a circular lock dependency involving tty ldisc_sem and >the networking rtnl_mutex. The full chain is: > > rtnl_mutex --> nft_commit_mutex --> ... --> ep->mtx --> ldisc_sem --> rtnl_mutex Hi Yun still unclear where the first mutex may come from > >The last edge (ldisc_sem -> rtnl_mutex) is created because tty line >discipline .open() callbacks (slcan, slip) call register_netdev() which >acquires rtnl_mutex, and .open() runs under ldisc_sem write lock in >tty_set_ldisc(). > >Fix by moving the .open() call outside the ldisc_sem write lock. The >ldisc .open() is initialization of the NEW discipline after the old one >has been closed - there is no need for ldisc_sem protection at this >point since: > > - tty_lock is held throughout, preventing concurrent tty_set_ldisc, > hangup, or close > - tty->ldisc is set to NULL during the window, so concurrent readers > (tty_ldisc_ref, tty_ldisc_ref_wait) see NULL and return immediately, > which callers already handle as a hangup condition > - tty buffer data stays queued until the ldisc is installed > >The sequence becomes: > 1. Hold ldisc_sem(write): close old ldisc, set tty->ldisc = NULL > 2. Release ldisc_sem(write) > 3. Call new_ldisc->ops->open() without ldisc_sem > 4. Re-acquire ldisc_sem(write): install new ldisc (or restore old) > 5. Release ldisc_sem(write) > >Reported-by: [email protected] >Closes: https://syzkaller.appspot.com/bug?extid=de610eeef174bd59a8a3 please add fixes tag and consider cc'ing stable kernel please also add net tree tag to the patch title as this is fix >Signed-off-by: Yun Zhou <[email protected]> >--- > drivers/tty/tty_ldisc.c | 17 +++++++++++++++-- > 1 file changed, 15 insertions(+), 2 deletions(-) > >diff --git a/drivers/tty/tty_ldisc.c b/drivers/tty/tty_ldisc.c >index 27fe8236f662..248a6995cc53 100644 >--- a/drivers/tty/tty_ldisc.c >+++ b/drivers/tty/tty_ldisc.c >@@ -556,15 +556,28 @@ int tty_set_ldisc(struct tty_struct *tty, int disc) > /* Shutdown the old discipline. */ > tty_ldisc_close(tty, old_ldisc); > >- /* Now set up the new line discipline. */ >- tty->ldisc = new_ldisc; >+ /* Clear tty->ldisc so concurrent readers back off during transition */ >+ tty->ldisc = NULL; > tty_set_termios_ldisc(tty, disc); >+ tty_ldisc_unlock(tty); > >+ /* this blank line is redundant i believe >+ * Open the new discipline outside ldisc_sem. The ldisc .open() >+ * may acquire locks (e.g., rtnl_mutex) that would create circular >+ * dependencies if taken under ldisc_sem. tty_lock is still held, >+ * preventing concurrent ldisc changes and hangup. >+ */ > retval = tty_ldisc_open(tty, new_ldisc); >+ >+ tty_ldisc_lock(tty, MAX_SCHEDULE_TIMEOUT); >+ > if (retval < 0) { > /* Back to the old one or N_TTY if we can't */ > tty_ldisc_put(new_ldisc); > tty_ldisc_restore(tty, old_ldisc); >+ } else { >+ /* Success - install new ldisc */ rather obvious comment >+ tty->ldisc = new_ldisc; > } > > if (tty->ldisc->ops->num != old_ldisc->ops->num && tty->ops->set_ldisc) { >-- >2.43.0