Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1393824 > unrolled thread

Re: [PATCH] usb: hub: fix panic caused by NULL bos pointer during reset device

Started byGreg KH <gregkh@linuxfoundation.org>
First post2016-05-03 23:20 +0200
Last post2016-05-04 04:30 +0200
Articles 2 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: [PATCH] usb: hub: fix panic caused by NULL bos pointer during  reset device Greg KH <gregkh@linuxfoundation.org> - 2016-05-03 23:20 +0200
    RE: [PATCH] usb: hub: fix panic caused by NULL bos pointer during  reset device "Du, Changbin" <changbin.du@intel.com> - 2016-05-04 04:30 +0200

#1393824 — Re: [PATCH] usb: hub: fix panic caused by NULL bos pointer during reset device

FromGreg KH <gregkh@linuxfoundation.org>
Date2016-05-03 23:20 +0200
SubjectRe: [PATCH] usb: hub: fix panic caused by NULL bos pointer during reset device
Message-ID<ruPTA-7jM-13@gated-at.bofh.it>
On Wed, Apr 27, 2016 at 09:35:57AM -0400, Tony Battersby wrote:
> On 04/26/2016 10:53 PM, Du, Changbin wrote:
> >> On Tue, Mar 08, 2016 at 05:15:17PM +0800, changbin.du@intel.com wrote:
> >>> From: "Du, Changbin" <changbin.du@intel.com>
> >>>
> >>> This is a reworked patch based on reverted commit d8f00cd685f5 ("usb:
> >>> hub: do not clear BOS field during reset device").
> >>>
> >>> The privious one caused double mem-free if run to re_enumerate label.
> >>> New patch title changed to distinguish from old one. And I have tested
> >>> it with memory debugging options.
> >>>
> >>> In function usb_reset_and_verify_device, the old BOS descriptor may
> >>> still be used before allocating a new one. (usb_disable_lpm function
> >>> uses it under the situation that it fails at usb_disable_link_state.)
> >>> So we cannot set the udev->bos to NULL before that, just keep what it
> >>> was. It will be overwrite when allocating a new one.
> >>>
> >>> How to reproduce:
> >>> 1. connect one usb3 hub to xhci port.
> >>> 2. connect several lpm-capable super-speed usb disk to the hub.
> >>> 3. copy big files to the usb disks.
> >>> 4. disconnect the hub and repeat step 1-4.
> >>>
> >>> Crash log:
> >>> BUG: unable to handle kernel NULL pointer dereference at
> >>> 0000000000000010
> >>> IP: [<ffffffff8171f98d>] usb_enable_link_state+0x2d/0x2f0
> >>> Call Trace:
> >>> [<ffffffff8171ed5b>] ? usb_set_lpm_timeout+0x12b/0x140
> >>> [<ffffffff8171fcd1>] usb_enable_lpm+0x81/0xa0
> >>> [<ffffffff8171fdd8>] usb_disable_lpm+0xa8/0xc0
> >>> [<ffffffff8171fe1c>] usb_unlocked_disable_lpm+0x2c/0x50
> >>> [<ffffffff81723933>] usb_reset_and_verify_device+0xc3/0x710
> >>> [<ffffffff8172c4ed>] ? usb_sg_wait+0x13d/0x190
> >>> [<ffffffff81724743>] usb_reset_device+0x133/0x280
> >>> [<ffffffff8179ccd1>] usb_stor_port_reset+0x61/0x70
> >>> [<ffffffff8179cd68>] usb_stor_invoke_transport+0x88/0x520
> >>>
> >>> Signed-off-by: Du, Changbin <changbin.du@intel.com>
> >>> ---
> >>>  drivers/usb/core/hub.c | 14 +++++++++-----
> >>>  1 file changed, 9 insertions(+), 5 deletions(-)
> >> Is this patch still needed?  I thought we had some other fix in this
> >> area...
> >>
> >> confused,
> >>
> >> greg k-h
> >>
> > Hi, Greg k-h,
> > Sorry for it confused you. This patch still need. This is same fix with
> > previous commit d8f00cd685f5 ("usb: hub: do not clear BOS field
> > during reset device"). But d8f00cd685f5 is buggy and reverted. This
> > new patch should be the final fix.
> >
> > Best Regards,
> > Du, Changbin
> >
> 
> I think Greg is referring to commit 464ad8c43a9e ("usb: core : hub: Fix
> BOS 'NULL pointer' kernel panic"), which has already been applied
> upstream.  It looks to me like that patch might have fixed the same
> problem in a different way, in which case Changbin's patch is not
> needed.  But I haven't been involved in developing or testing that
> patch, so I can't say for sure.  At the very least, 464ad8c43a9e
> conflicts with Changbin's patch.
> 
> Changbin, can you take a look at 464ad8c43a9e and see if that fixes the
> same problem that your patch did?

Given the lack of response here, I've dropped this from my queue.  If
it's still needed, someone must resend it.

thanks,

greg k-h

[toc] | [next] | [standalone]


#1393898

From"Du, Changbin" <changbin.du@intel.com>
Date2016-05-04 04:30 +0200
Message-ID<ruUJz-3iX-1@gated-at.bofh.it>
In reply to#1393824
> > I think Greg is referring to commit 464ad8c43a9e ("usb: core : hub: Fix
> > BOS 'NULL pointer' kernel panic"), which has already been applied
> > upstream.  It looks to me like that patch might have fixed the same
> > problem in a different way, in which case Changbin's patch is not
> > needed.  But I haven't been involved in developing or testing that
> > patch, so I can't say for sure.  At the very least, 464ad8c43a9e
> > conflicts with Changbin's patch.
> >
> > Changbin, can you take a look at 464ad8c43a9e and see if that fixes the
> > same problem that your patch did?
> 
> Given the lack of response here, I've dropped this from my queue.  If
> it's still needed, someone must resend it.
> 
> thanks,
> 
> greg k-h

Hi,
I missed this email because it was junked by my email client. Just checked
patch 464ad8c43a9e, it fix the same issue. So my patch no longer need now.
Thanks for the patch.

Best Regards,
Du, Changbin

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web