[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

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Concurrent read/write/poll system calls spuriously return EOF or EIO during line discipline transitions, breaking UAPI.

Pre-existing issues:
- [High] The circular lock dependency between ldisc_sem and rtnl_mutex is not fully resolved, remaining on the error handling and hangup paths.
--

--- Patch [1]: [PATCH] tty: ldisc: fix deadlock between ldisc_sem and rtnl_mutex ---
commit bb627a1c685e7cab3f8500d0d87506f028ebe9fe
Author: Yun Zhou <yun.zhou@windriver.com>

tty: ldisc: fix deadlock between ldisc_sem and rtnl_mutex

This commit aims to fix a circular lock dependency between the tty ldisc_sem
and the networking rtnl_mutex. It does this by modifying tty_set_ldisc() to
call the line discipline open() callback outside of the ldisc_sem write lock.

Closes: https://syzkaller.appspot.com/bug?extid=de610eeef174bd59a8a3

> diff --git a/drivers/tty/tty_ldisc.c b/drivers/tty/tty_ldisc.c
> index 27fe8236f6623..248a6995cc53a 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);

[Severity: High]
Does unlocking the semaphore while the ldisc pointer is NULL introduce a UAPI
break for concurrent operations?

If a concurrent process calls read(), write(), or poll() during this unlocked
window, it can enter tty_ldisc_ref_wait() in drivers/tty/tty_io.c. Because the
semaphore was unlocked here, tty_ldisc_ref_wait() will successfully acquire
the read lock but observe tty->ldisc as NULL.

This causes the reader to immediately return EOF or -EIO, potentially
aborting userspace applications unexpectedly during a line discipline
transition.

> +
> +	/*
> +	 * 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);

[Severity: High]
This is a pre-existing issue, but does this fully resolve the circular lock
dependency?

If tty_ldisc_open() fails above, the error path calls tty_ldisc_restore(),
which eventually calls tty_ldisc_failto(). Inside tty_ldisc_failto(),
tty_ldisc_open() is called while the ldisc_sem write lock is held.

Similarly, during hangup processing, tty_ldisc_reinit() calls tty_ldisc_open()
while holding the ldisc_sem write lock.

If the restored or reinitialized discipline acquires rtnl_mutex on these paths,
it appears the lock inversion and potential deadlock would still occur.

> +	} else {
> +		/* Success - install new ldisc */
> +		tty->ldisc = new_ldisc;
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260716064719.1401892-1-yun.zhou@windriver.com?part=1

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

[Oliver Hartkopp]

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

You are right.

The non-blank first comment line is only common for networking but you 
posted a patch for the tty subsystem and not for networking.

Documentation/Codingstyle explains that:

     The preferred style for long (multi-line) comments is:

         /*
          * This is the preferred style for multi-line
          * comments in the Linux kernel source code.
          * Please use it consistently.
          *
          * Description:  A column of asterisks on the left side,
          * with beginning and ending almost-blank lines.
          */

     For files in net/ and drivers/net/ the preferred style for long 
(multi-line)
     comments is a little different.

         /* The preferred comment style for files in net/ and drivers/net
          * looks like this.
          *
          * It is nearly the same as the generally preferred comment style,
          * but there is no initial almost-blank line.
          */

Source: https://lwn.net/Articles/694755/

Best regards,
Oliver

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

[Jagielski, Jedrzej]

From: Zhou, Yun <yun.zhou@windriver.com> 
Sent: Friday, July 17, 2026 5:42 AM

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

First line of the comment unlike the last don't have to be necessarily
blank and indeed usually it is not for new code, it’s rather obsolete practice.
But that's only suggestion.