RE: [PATCH] tty: ldisc: fix deadlock between ldisc_sem and rtnl_mutex

"Jagielski, Jedrzej" <[email protected]>
Newsgroups org.kernel.vger.linux-serial,org.kernel.vger.linux-can,org.kernel.vger.linux-kernel,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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.