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

[Zhou, Yun]

On 7/16/26 15:57, Greg KH wrote:
> CAUTION: This email comes from a non Wind River email account!
> Do not click links or open attachments unless you recognize the sender and know the content is safe.
>
> 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@xxxxxxxxxxxxxxxxxxxxxxxxx
> > Closes: https://syzkaller.appspot.com/bug?extid=de610eeef174bd59a8a3
> > Signed-off-by: Yun Zhou <yun.zhou@xxxxxxxxxxxxx>
> > ---
> >   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?

The circular dependency has existed for a long time - it just requires
ldisc_sem -> rtnl_mutex (from slcan/slip registering a netdev in
.open()) and the reverse path through nft_commit_mutex, epoll, and
tty_poll back to ldisc_sem.

The recent dev_instance_lock series (5326fefb9fe8 "net: hold instance
lock around NETDEV_DOWN/GOING_DOWN") increased lockdep's observability
by adding lock acquisitions in more notifier paths, making it easier for
lockdep to collect all edges in a single run. It did not create the
cycle.

We have not seen this in real-world usage because triggering the actual
deadlock requires 6 unrelated subsystems to contend simultaneously -
something only a fuzzer like syzkaller would construct.

> > 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?

This is not a new .open() call - it is the same tty_ldisc_open() that
was always called here. The change only moves it outside ldisc_sem.
tty_lock is still held throughout, so .open() sees the same environment
as before.

> > +
> > +     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?

tty_ldisc_open() only sets the TTY_LDISC_OPEN flag bit and calls
ld->ops->open(). On failure it clears the flag itself.

No refcounts or other state are incremented by tty_ldisc_open() that
would need additional cleanup.

BR,
Yun