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