* [PATCH] tty: ldisc: fix deadlock between ldisc_sem and rtnl_mutex
@ 2026-07-16 6:47 Yun Zhou
2026-07-16 7:57 ` Greg KH
2026-07-16 7:59 ` Jagielski, Jedrzej
0 siblings, 2 replies; 3+ messages in thread
From: Yun Zhou @ 2026-07-16 6:47 UTC (permalink / raw)
To: gregkh, jirislaby, socketcan
Cc: linux-serial, mkl, linux-can, davem, edumazet, kuba, pabeni,
horms, netdev, linux-kernel, yun.zhou
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
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: syzbot+de610eeef174bd59a8a3@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=de610eeef174bd59a8a3
Signed-off-by: Yun Zhou <yun.zhou@windriver.com>
---
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);
+ /*
+ * 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 */
+ tty->ldisc = new_ldisc;
}
if (tty->ldisc->ops->num != old_ldisc->ops->num && tty->ops->set_ldisc) {
--
2.43.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] tty: ldisc: fix deadlock between ldisc_sem and rtnl_mutex
2026-07-16 6:47 [PATCH] tty: ldisc: fix deadlock between ldisc_sem and rtnl_mutex Yun Zhou
@ 2026-07-16 7:57 ` Greg KH
2026-07-16 7:59 ` Jagielski, Jedrzej
1 sibling, 0 replies; 3+ messages in thread
From: Greg KH @ 2026-07-16 7:57 UTC (permalink / raw)
To: Yun Zhou
Cc: jirislaby, socketcan, linux-serial, mkl, linux-can, davem,
edumazet, kuba, pabeni, horms, netdev, linux-kernel
On Thu, Jul 16, 2026 at 02:47:19PM +0800, Yun Zhou wrote:
> 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
>
> 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: syzbot+de610eeef174bd59a8a3@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=de610eeef174bd59a8a3
> Signed-off-by: Yun Zhou <yun.zhou@windriver.com>
> ---
> drivers/tty/tty_ldisc.c | 17 +++++++++++++++--
> 1 file changed, 15 insertions(+), 2 deletions(-)
What commit caused this to be a problem and why have we not seen this in
any real-world usages?
>
> 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);
>
> + /*
> + * 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);
Now you are calling open when previously we were not, are you sure this
isn't going to cause problems?
> +
> + tty_ldisc_lock(tty, MAX_SCHEDULE_TIMEOUT);
Why that 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 */
> + tty->ldisc = new_ldisc;
> }
Does open cause anything else to be incremented that you have to clean
up when done that you aren't doing here?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 3+ messages in thread
* RE: [PATCH] tty: ldisc: fix deadlock between ldisc_sem and rtnl_mutex
2026-07-16 6:47 [PATCH] tty: ldisc: fix deadlock between ldisc_sem and rtnl_mutex Yun Zhou
2026-07-16 7:57 ` Greg KH
@ 2026-07-16 7:59 ` Jagielski, Jedrzej
1 sibling, 0 replies; 3+ messages in thread
From: Jagielski, Jedrzej @ 2026-07-16 7:59 UTC (permalink / raw)
To: Yun Zhou, gregkh@linuxfoundation.org, jirislaby@kernel.org,
socketcan@hartkopp.net
Cc: linux-serial@vger.kernel.org, mkl@pengutronix.de,
linux-can@vger.kernel.org, davem@davemloft.net,
edumazet@google.com, kuba@kernel.org, pabeni@redhat.com,
horms@kernel.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
From: Yun Zhou <yun.zhou@windriver.com>
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: syzbot+de610eeef174bd59a8a3@syzkaller.appspotmail.com
>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 <yun.zhou@windriver.com>
>---
> 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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-07-16 7:59 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-16 6:47 [PATCH] tty: ldisc: fix deadlock between ldisc_sem and rtnl_mutex Yun Zhou
2026-07-16 7:57 ` Greg KH
2026-07-16 7:59 ` Jagielski, Jedrzej
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox