Linux-USB Archive on lore.kernel.org
 help / color / Atom feed
From: Alan Stern <stern@rowland.harvard.edu>
To: David Heinzelmann <heinzelmann.david@gmail.com>
Cc: linux-usb@vger.kernel.org
Subject: Re: [PATCH] Check for changed device descriptors when a connection-change occurs before validating the connection.
Date: Fri, 4 Oct 2019 10:17:52 -0400 (EDT)
Message-ID: <Pine.LNX.4.44L0.1910041008420.1615-100000@iolanthe.rowland.org> (raw)
In-Reply-To: <20191004132341.GA22292@dhe-pc>

On Fri, 4 Oct 2019, David Heinzelmann wrote:

> Hi,
> 
> thank you for the feedback! I hope this time everything is in order.
> However, the correct error description is somewhat difficult for me.

You should follow the advice I gave you last time: Explain what the 
problem is and how the patch fixes it.

> Here is the third version:
> 
> From 58634d035508b621025da1d866179b59ed0ae37a Mon Sep 17 00:00:00 2001
> From: David Heinzelmann <heinzelmann.david@gmail.com>
> Date: Fri, 4 Oct 2019 12:28:36 +0200
> Subject: [PATCH v3] usb: hub: Check device descriptor before resusciation
> 
> If a port connection-change occurs, the connection should not be
> resusciated without a prior check if the port connection is enabled.
> Signed-off-by: David Heinzelmann <heinzelmann.david@gmail.com>

(Insert a blank line before the Signed-off-by:.)

See, this doesn't say what the problem is.  Someone reading your 
description won't know _why_ a check is needed.

The problem shows up when a device goes through a firmware update.  It
will disconnect from the USB bus and reconnect, and if it is attached
to an xHCI host controller then the controller hardware will
automatically initialize the connection and enable the port.  But
hub_port_connect_change() assumes that if the port is enabled then
nothing has changed; it doesn't check to see if the device's
descriptors have been updated.  As a result, the kernel's internal copy
of the descriptors ends up being incorrect and the device doesn't work
properly.

The solution to the problem is for hub_port_connect_change() always to
check whether the device's descriptors have changed before 
resuscitating an enabled port.

Alan Stern


  reply index

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-20 10:36 David Heinzelmann
2019-09-20  8:55 ` Greg KH
2019-09-20 13:17   ` David Heinzelmann
2019-09-20 12:15     ` Greg KH
2019-09-20 15:33       ` David Heinzelmann
2019-09-23 14:49         ` Alan Stern
2019-09-24 10:01           ` David Heinzelmann
2019-09-25 14:20             ` Alan Stern
2019-09-30  7:26               ` David Heinzelmann
2019-09-30 14:25                 ` Alan Stern
2019-10-04 13:23                   ` David Heinzelmann
2019-10-04 14:17                     ` Alan Stern [this message]
2019-10-07  8:47                       ` David Heinzelmann
2019-10-07 14:01                         ` Alan Stern
2019-10-07 15:35                           ` Greg KH
2019-10-08  8:09                             ` [PATCH v4] usb: hub: Check device descriptor before resusciation David Heinzelmann
2019-10-08 12:55                               ` Greg KH
2019-10-08 16:10                                 ` David Heinzelmann
2019-10-08 15:17                                   ` Greg KH
2019-10-09  4:46                             ` [PATCH v5] " David Heinzelmann

Reply instructions:

You may reply publically 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=Pine.LNX.4.44L0.1910041008420.1615-100000@iolanthe.rowland.org \
    --to=stern@rowland.harvard.edu \
    --cc=heinzelmann.david@gmail.com \
    --cc=linux-usb@vger.kernel.org \
    /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

Linux-USB Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-usb/0 linux-usb/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-usb linux-usb/ https://lore.kernel.org/linux-usb \
		linux-usb@vger.kernel.org
	public-inbox-index linux-usb

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-usb


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git