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

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

Ah, but look at the review at:
	https://sashiko.dev/#/patchset/20260716064719.1401892-1-yun.zhou@windriver.com
which says:
	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.

Is that not true?

thanks,

greg k-h

On 7/17/26 18:53, 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
> 
> Ah, but look at the review at:
>          https://sashiko.dev/#/patchset/20260716064719.1401892-1-yun.zhou@windriver.com
> which says:
>          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.
> 
> Is that not true?
> 

No, the issue reported by Sashiko does indeed exist. I will fix it in 
the v2.

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

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

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.

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

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

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

On Fri, Jul 17, 2026 at 11:03:18AM +0800, Zhou, Yun wrote:
> 
> 
> 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.

Ah, you are right, my bad read of the patch, sorry.

thanks,

greg k-h