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

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

[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

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

[Greg KH]

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

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

[Jagielski, Jedrzej]

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

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

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

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

[Zhou, Yun]

Hi Jagielski,

On 7/16/26 15:59, Jagielski, Jedrzej 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.
> 
> 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
> 
> still unclear where the first mutex may come from

   tty_set_ldisc()
     tty_ldisc_lock()        <- hold ldisc_sem
       tty_ldisc_open()
         slcan_open()
           register_candev()
             register_netdev()
               rtnl_net_lock_killable()  <- hold rtnl_mutex

> 
>>
>> +      tty_ldisc_unlock(tty);
>>
>> +      /*
> 
> this blank line is redundant i believe
> 

Isn't it common practice to leave the first line of a multi-line comment 
blank? Does it violate the coding style guidelines?

BR,
Yun