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

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: [email protected]
> Closes: https://syzkaller.appspot.com/bug?extid=de610eeef174bd59a8a3
> Signed-off-by: Yun Zhou <[email protected]>
> ---
>  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