* [PATCH v3] tty: ldisc: fix deadlock between ldisc_sem and rtnl_mutex
@ 2026-07-24 8:46 Yun Zhou
2026-07-30 14:50 ` Greg KH
0 siblings, 1 reply; 2+ messages in thread
From: Yun Zhou @ 2026-07-24 8:46 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. tty_ldisc_ref_wait()
waits for the transition to complete. tty_ldisc_ref() returns NULL
which callers already handle.
- 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>
---
v3:
- Use a while loop in tty_ldisc_ref_wait() to handle consecutive ldisc
switches where a reader could see NULL again after being woken up.
v2:
- Keep user-visible behavior unchanged: tty_ldisc_ref_wait() now waits
for the ldisc transition to complete instead of returning NULL (which
would cause spurious EOF/-EIO to concurrent readers).
- Fix a race between tty_ldisc_ref_wait() and __tty_hangup() where a
reader could block forever if it observed ldisc==NULL before
TTY_HUPPED was set. Add wake_up() after set_bit(TTY_HUPPED).
drivers/tty/tty_io.c | 7 +++++++
drivers/tty/tty_ldisc.c | 30 ++++++++++++++++++++++++++++--
2 files changed, 35 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index 6b283fd03ff8..e2f82e80f397 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -649,6 +649,13 @@ static void __tty_hangup(struct tty_struct *tty, int exit_session)
*/
set_bit(TTY_HUPPED, &tty->flags);
clear_bit(TTY_HUPPING, &tty->flags);
+
+ /*
+ * Wake up readers blocked in tty_ldisc_ref_wait() that may have
+ * seen ldisc == NULL but not yet TTY_HUPPED.
+ */
+ wake_up(&tty->read_wait);
+
tty_unlock(tty);
if (f)
diff --git a/drivers/tty/tty_ldisc.c b/drivers/tty/tty_ldisc.c
index 27fe8236f662..6ec93e6b8498 100644
--- a/drivers/tty/tty_ldisc.c
+++ b/drivers/tty/tty_ldisc.c
@@ -242,6 +242,16 @@ struct tty_ldisc *tty_ldisc_ref_wait(struct tty_struct *tty)
ldsem_down_read(&tty->ldisc_sem, MAX_SCHEDULE_TIMEOUT);
ld = tty->ldisc;
+ while (!ld && !test_bit(TTY_HUPPED, &tty->flags)) {
+ ldsem_up_read(&tty->ldisc_sem);
+
+ /* ldisc may be NULL during a discipline switch; wait and retry */
+ wait_event(tty->read_wait,
+ READ_ONCE(tty->ldisc) != NULL ||
+ test_bit(TTY_HUPPED, &tty->flags));
+ ldsem_down_read(&tty->ldisc_sem, MAX_SCHEDULE_TIMEOUT);
+ ld = tty->ldisc;
+ }
if (!ld)
ldsem_up_read(&tty->ldisc_sem);
return ld;
@@ -556,15 +566,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) {
@@ -584,6 +607,9 @@ int tty_set_ldisc(struct tty_struct *tty, int disc)
out:
tty_ldisc_unlock(tty);
+ /* Wake up readers waiting for the ldisc transition to complete */
+ wake_up(&tty->read_wait);
+
/*
* Restart the work queue in case no characters kick it off. Safe if
* already running
--
2.43.0
^ permalink raw reply related [flat|nested] 2+ messages in thread* Re: [PATCH v3] tty: ldisc: fix deadlock between ldisc_sem and rtnl_mutex
2026-07-24 8:46 [PATCH v3] tty: ldisc: fix deadlock between ldisc_sem and rtnl_mutex Yun Zhou
@ 2026-07-30 14:50 ` Greg KH
0 siblings, 0 replies; 2+ messages in thread
From: Greg KH @ 2026-07-30 14:50 UTC (permalink / raw)
To: Yun Zhou
Cc: jirislaby, socketcan, linux-serial, mkl, linux-can, davem,
edumazet, kuba, pabeni, horms, netdev, linux-kernel
On Fri, Jul 24, 2026 at 04:46:48PM +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. tty_ldisc_ref_wait()
> waits for the transition to complete. tty_ldisc_ref() returns NULL
> which callers already handle.
> - 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>
> ---
Sashiko has some comments:
https://sashiko.dev/#/patchset/20260724084648.3879356-1-yun.zhou@windriver.com
are they correct?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-07-30 16:20 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-24 8:46 [PATCH v3] tty: ldisc: fix deadlock between ldisc_sem and rtnl_mutex Yun Zhou
2026-07-30 14:50 ` Greg KH
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox