All of lore.kernel.org
 help / color / mirror / Atom feed
From: Olivier Sobrie <olivier@sobrie.be>
To: Alan Stern <stern@rowland.harvard.edu>
Cc: Jan Dumon <j.dumon@option.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org,
	netdev@vger.kernel.org
Subject: Re: [PATCH 11/11] usb: core: fix a race with usb_queue_reset_device()
Date: Wed, 21 Jan 2015 14:55:12 +0100	[thread overview]
Message-ID: <20150121135512.GA9889@hposo> (raw)
In-Reply-To: <Pine.LNX.4.44L0.1501201022360.1150-100000@iolanthe.rowland.org>

Hello Alan,

On Tue, Jan 20, 2015 at 10:26:30AM -0500, Alan Stern wrote:
> On Tue, 20 Jan 2015, Olivier Sobrie wrote:
> 
> > When usb_queue_reset() is called it schedules a work in view of
> > resetting the usb interface. When the reset work is running, it
> > can be scheduled again (e.g. by the usb disconnect method of
> > the driver).
> > 
> > Consider that the reset work is queued again while the reset work
> > is running and that this work leads to a forced unbinding of the
> > usb interface (e.g. because a driver is bound to the interface
> > and has no pre/post_reset methods - see usb_reset_device()).
> > In such condition, usb_unbind_interface() gets called and this
> > function calls usb_cancel_queued_reset() which does nothing
> > because the flag "reset_running" is set to 1. The second reset
> > work that has been scheduled is therefore not cancelled.
> > Later, the usb_reset_device() tries to rebind the interface.
> > If it fails, then the usb interface context which contain the
> > reset work struct is freed and it most likely crash when the
> > second reset work tries to be run.
> 
> There was an earlier patch posted for testing (no results yet)  
> affecting this same region of code:
> 
> 	http://marc.info/?l=linux-usb&m=142064533924019&w=2
> 
> It should fix the problem described here, because (among other things) 
> it adds usb_get/put_intf calls to the delayed-reset routines.

I tested your patch. It also fixes the problem I observed.
You can drop mine.

For your info:

My test consists in powering down a usb hso modem while one of its
serial port is opened. It leads to two URB failures, each urb callback
queues a reset.
Without your fix (or without the one I sent), a crash happens after
less than ~20 power up/down sequences. With your fix, after more than
1000 power up/down I don't see any crash.

Thanks,

-- 
Olivier

  parent reply	other threads:[~2015-01-21 13:55 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-01-20 12:29 [PATCH 00/11] hso: fix some problems with reset/disconnect path Olivier Sobrie
2015-01-20 12:29 ` [PATCH 01/11] hso: remove useless header file timer.h Olivier Sobrie
2015-01-20 12:29   ` [PATCH 02/11] hso: fix crash when device disappears while serial port is open Olivier Sobrie
2015-01-20 12:29     ` [PATCH 03/11] hso: fix memory leak when device disconnects Olivier Sobrie
2015-01-20 12:29       ` [PATCH 04/11] hso: fix memory leak in hso_create_rfkill() Olivier Sobrie
2015-01-20 12:29         ` [PATCH 05/11] hso: fix small indentation error Olivier Sobrie
2015-01-20 12:29           ` [PATCH 06/11] hso: rename hso_dev into serial in hso_free_interface() Olivier Sobrie
2015-01-20 12:29             ` Olivier Sobrie
2015-01-20 12:29             ` [PATCH 07/11] hso: replace reset_device work by usb_queue_reset_device() Olivier Sobrie
2015-01-20 12:29               ` [PATCH 08/11] hso: move tty_unregister outside hso_serial_common_free() Olivier Sobrie
2015-01-20 12:29                 ` [PATCH 09/11] hso: update serial_table in usb disconnect method Olivier Sobrie
2015-01-20 12:29                   ` [PATCH 10/11] hso: add missing cancel_work_sync in disconnect() Olivier Sobrie
2015-01-20 12:29                     ` [PATCH 11/11] usb: core: fix a race with usb_queue_reset_device() Olivier Sobrie
2015-01-20 13:48                       ` Oliver Neukum
2015-01-20 15:10                         ` Olivier Sobrie
2015-01-20 15:26                       ` Alan Stern
2015-01-20 16:30                         ` Olivier Sobrie
2015-01-21 13:55                         ` Olivier Sobrie [this message]
2015-01-21 18:52                           ` Alan Stern
2015-01-21 18:52                             ` Alan Stern
2015-01-20 13:13         ` [PATCH 04/11] hso: fix memory leak in hso_create_rfkill() Oliver Neukum
2015-01-20 15:10           ` Olivier Sobrie
2015-01-20 18:41           ` Dan Williams
2015-01-30 12:21 ` [PATCH v2 00/11] hso: fix some problems in the disconnect path Olivier Sobrie
2015-01-30 12:21   ` [PATCH v2 01/11] hso: remove useless header file timer.h Olivier Sobrie
2015-01-30 12:21     ` [PATCH v2 02/11] hso: fix crash when device disappears while serial port is open Olivier Sobrie
2015-01-30 12:21       ` [PATCH v2 03/11] hso: fix memory leak when device disconnects Olivier Sobrie
2015-01-30 12:21         ` [PATCH v2 04/11] hso: fix memory leak in hso_create_rfkill() Olivier Sobrie
2015-01-30 12:21           ` [PATCH v2 05/11] hso: fix small indentation error Olivier Sobrie
2015-01-30 12:21             ` [PATCH v2 06/11] hso: rename hso_dev into serial in hso_free_interface() Olivier Sobrie
2015-01-30 12:21               ` [PATCH v2 07/11] hso: replace reset_device work by usb_queue_reset_device() Olivier Sobrie
2015-01-30 12:22                 ` [PATCH v2 08/11] hso: move tty_unregister outside hso_serial_common_free() Olivier Sobrie
2015-01-30 12:22                   ` [PATCH v2 09/11] hso: update serial_table in usb disconnect method Olivier Sobrie
2015-01-30 12:22                     ` [PATCH v2 10/11] hso: add missing cancel_work_sync in disconnect() Olivier Sobrie
2015-01-30 12:22                       ` [PATCH v2 11/11] hso: fix rfkill name conflicts Olivier Sobrie
2015-01-30 15:47                         ` Dan Williams
2015-01-30 16:15                           ` Olivier Sobrie
2015-01-30 16:59                             ` Dan Williams
2015-02-01 20:33   ` [PATCH v2 00/11] hso: fix some problems in the disconnect path David Miller
2015-02-01 20:33     ` David Miller

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=20150121135512.GA9889@hposo \
    --to=olivier@sobrie.be \
    --cc=gregkh@linuxfoundation.org \
    --cc=j.dumon@option.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=stern@rowland.harvard.edu \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.