Linux CAN drivers development
 help / color / mirror / Atom feed
From: "Zhou, Yun" <yun.zhou@windriver.com>
To: sashiko-reviews@lists.linux.dev
Cc: Marc Kleine-Budde <mkl@pengutronix.de>,
	Oliver Hartkopp <socketcan@hartkopp.net>,
	linux-can@vger.kernel.org,
	Oleksij Rempel <o.rempel@pengutronix.de>,
	Vincent Mailhol <mailhol@kernel.org>
Subject: Re: [PATCH v3] tty: ldisc: fix deadlock between ldisc_sem and rtnl_mutex
Date: Sun, 26 Jul 2026 11:25:41 +0800	[thread overview]
Message-ID: <3894bbdd-1930-4ad4-8b28-431a162aece0@windriver.com> (raw)
In-Reply-To: <20260725084748.C82601F00A3A@smtp.kernel.org>

On 7/25/2026 4:47 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> 
> New issues:
> - [High] The patch replaces a lockdep splat with a silent waitqueue deadlock on the success path.
> 
> Pre-existing issues:
> - [High] Incomplete fix for circular locking between ldisc_sem and rtnl_mutex, leaving deadlocks on teardown, error, and hangup paths.
> --
> 
> --- Patch [1]: [PATCH v3] tty: ldisc: fix deadlock between ldisc_sem and rtnl_mutex ---
> commit 98cde28b47f83f4a2bf71bb1e35f53d9a7a36294
> Author: Yun Zhou <yun.zhou@windriver.com>
> 
> tty: ldisc: fix deadlock between ldisc_sem and rtnl_mutex
> 
> This commit attempts to fix a circular lock dependency between ldisc_sem
> and rtnl_mutex reported by syzbot. It moves the tty line discipline .open()
> callback outside the ldisc_sem write lock in tty_set_ldisc(). It also adds
> logic in tty_ldisc_ref_wait() to wait for the ldisc transition to complete.
> 
> Closes: https://syzkaller.appspot.com/bug?extid=de610eeef174bd59a8a3
> 
>> diff --git a/drivers/tty/tty_ldisc.c b/drivers/tty/tty_ldisc.c
>> index 27fe8236f6623..6ec93e6b84988 100644
>> --- a/drivers/tty/tty_ldisc.c
>> +++ b/drivers/tty/tty_ldisc.c
>> @@ -242,6 +242,16 @@ struct tty_ldisc *tty_ldisc_ref_wait(struct tty_struct *tty)
>>
>>        ldsem_down_read(&tty->ldisc_sem, MAX_SCHEDULE_TIMEOUT);
>>        ld = tty->ldisc;
>> +     while (!ld && !test_bit(TTY_HUPPED, &tty->flags)) {
>> +             ldsem_up_read(&tty->ldisc_sem);
>> +
>> +             /* ldisc may be NULL during a discipline switch; wait and retry */
>> +             wait_event(tty->read_wait,
>> +                        READ_ONCE(tty->ldisc) != NULL ||
>> +                        test_bit(TTY_HUPPED, &tty->flags));
> 
> [Severity: High]
> Does this wait_event() create a silent waitqueue deadlock that replaces the
> lockdep splat?
> 
> If thread A switches to a network ldisc like slcan, it will set tty->ldisc
> to NULL in tty_set_ldisc() and drop ldisc_sem, but then block waiting for
> rtnl_mutex in the .open() callback.
> 
> Concurrently, if thread B already holds rtnl_mutex and calls tty_poll(),
> it enters tty_ldisc_ref_wait() here. Thread B will observe tty->ldisc as NULL
> and block uninterruptibly in this wait_event(), waiting for thread A to
> re-assign tty->ldisc.
> 

No, this scenario does not exist. tty_poll() is only called from 
userspace poll/epoll/select paths, which never hold rtnl_mutex. And All 
callers of tty_ldisc_ref_wait() are userspace entry points: 
tty_read/tty_write/tty_poll/tty_ioctl/tty_open/tty_show_fdinfo/paste_selection/tty_perform_flush. 
None of these are reachable from any code path that holds rtnl_mutex.

> Since thread A is blocked waiting for thread B to release rtnl_mutex,
> will both threads hang forever without a lockdep warning?
> 


  reply	other threads:[~2026-07-26  3:26 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-24  8:46 [PATCH v3] tty: ldisc: fix deadlock between ldisc_sem and rtnl_mutex Yun Zhou
2026-07-25  8:47 ` sashiko-bot
2026-07-26  3:25   ` Zhou, Yun [this message]
2026-07-30 14:50 ` Greg KH

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3894bbdd-1930-4ad4-8b28-431a162aece0@windriver.com \
    --to=yun.zhou@windriver.com \
    --cc=linux-can@vger.kernel.org \
    --cc=mailhol@kernel.org \
    --cc=mkl@pengutronix.de \
    --cc=o.rempel@pengutronix.de \
    --cc=sashiko-reviews@lists.linux.dev \
    --cc=socketcan@hartkopp.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox