public inbox for linux-media@vger.kernel.org
 help / color / mirror / Atom feed
From: Sean Young <sean@mess.org>
To: Alan Stern <stern@rowland.harvard.edu>
Cc: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>,
	Jarod Wilson <jarod@redhat.com>,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	syzbot <syzbot+c558267ad910fc494497@syzkaller.appspotmail.com>,
	andreyknvl@google.com, linux-media@vger.kernel.org,
	linux-usb@vger.kernel.org, syzkaller-bugs@googlegroups.com
Subject: Re: possible deadlock in display_open
Date: Mon, 25 Apr 2022 17:21:28 +0100	[thread overview]
Message-ID: <YmbKiPna01aMQhJw@gofer.mess.org> (raw)
In-Reply-To: <YmbF071fSKUff6R2@rowland.harvard.edu>

Hi Alan, Tetsuo,

On Mon, Apr 25, 2022 at 12:01:23PM -0400, Alan Stern wrote:
> On Mon, Apr 25, 2022 at 12:56:19PM +0100, Sean Young wrote:
> > On Mon, Apr 25, 2022 at 08:14:12PM +0900, Tetsuo Handa wrote:
> > > On 2022/04/25 18:20, Sean Young wrote:
> > > > The problem is there are imon devices which have two usb interfaces, even
> > > > though it is one device. The probe and disconnect function of both usb
> > > > interfaces can run concurrently.
> > > > 
> > > > Of course, this depends on probe/disconnect functions being allowed to run
> > > > concurrently on different interfaces of the same usb device.
> > > 
> > > I don't have real hardware to confirm. If you have an imon device which has
> > > two usb interfaces, please try below debug printk() patch in order to see
> > > whether the caller is already holding locks for serialization.
> > 
> > I am afraid calling debug_show_held_locks() is not really going to tell us
> > this information. This should be figured out from understanding the usb
> > stack, not from seeing if the probe happens to be concurrent.
> > 
> > Just because the locks were not held, does not mean that the usb interface
> > initialization is not concurrent.
> 
> The driver and USB cores guarantee that when an interface is probed, 
> both the interface and its USB device are locked.  Ditto for when the 
> disconnect callback gets run.  So concurrent probing/disconnection of 
> multiple interfaces on the same device is not possible.

That is very helpful, thank you Alan. I think the original patch as proposed
by Tetsuo Handa is correct.

However, the commit message does need to say that the lock is not needed
because usb core already guarantees this. Please submit a v2 for this.

Thank you both,

Sean

  reply	other threads:[~2022-04-25 16:21 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-09 13:18 possible deadlock in display_open syzbot
2022-04-25  5:29 ` Tetsuo Handa
2022-04-25  9:20   ` Sean Young
2022-04-25 11:14     ` Tetsuo Handa
2022-04-25 11:56       ` Sean Young
2022-04-25 16:01         ` Alan Stern
2022-04-25 16:21           ` Sean Young [this message]
2022-04-26  0:54             ` [PATCH] media: imon: remove redundant serialization Tetsuo Handa
2022-04-26  7:57               ` Sean Young
2022-04-26 10:32                 ` [PATCH v2] media: imon: reorganize serialization Tetsuo Handa
2022-05-02  3:49                   ` [PATCH v2 (resend)] " Tetsuo Handa
2022-05-02  9:39                     ` Sean Young
2022-05-02 10:26                       ` Tetsuo Handa
2022-05-02 10:40                         ` Tetsuo Handa
2022-05-02 10:46                     ` Oliver Neukum
2022-05-02 11:05                       ` Tetsuo Handa
2022-05-03  7:41                         ` Oliver Neukum
2022-05-02 12:53                     ` Greg KH
2022-05-02 13:04                       ` Tetsuo Handa
2022-05-02 13:06                         ` Greg KH
2022-05-02 13:40                     ` Tetsuo Handa

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=YmbKiPna01aMQhJw@gofer.mess.org \
    --to=sean@mess.org \
    --cc=andreyknvl@google.com \
    --cc=jarod@redhat.com \
    --cc=linux-media@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=mchehab@kernel.org \
    --cc=penguin-kernel@i-love.sakura.ne.jp \
    --cc=stern@rowland.harvard.edu \
    --cc=syzbot+c558267ad910fc494497@syzkaller.appspotmail.com \
    --cc=syzkaller-bugs@googlegroups.com \
    /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