RE: [PATCH v2 2/2] usb: chipidea: add role switch class support
> -Original Message-
> From: Peter Chen
> Sent: 2019年8月8日 17:31
> To: Jun Li
> Cc: gre...@linuxfoundation.org; Jun Li ; dl-linux-imx
> ; linux-usb@vger.kernel.org
> Subject: RE: [PATCH v2 2/2] usb: chipidea: add role switch class support
>
>
> > USB role is fully controlled by usb role switch consumer(e.g. typec),
> > usb port can be at host mode(USB_ROLE_HOST), device mode connected to
> > host(USB_ROLE_DEVICE), or not connecting any parter(USB_ROLE_NONE).
> >
>
> %s/parter/partner ?
Yes, I will update.
>
> Are there any ways you could get external cable status from usb-switch or
> type-c like
> external connector? If there are, you could update otgsc value for
> otgsc_read, or change
> cable status, and avoid changing common handling, like ci_hdrc_probe and
> ci_otg_work.
> And it could benefit for other use cases, like booting with cable connected
> and switch role
> through /sys.
The new role switch class has nothing to do with extcon, it's using graph
bindings
to link the connection, so there is no extcon_dev, change for ci_hdrc_probe() is
required as property usb-role-switch has to be specified, there may be some code
reuse if still touch the external cable even no extcon dev, but that will make
existing
external cable complicated and not clean.
On the other hand, a new GPIO based role switch driver is on review:
https://patchwork.kernel.org/patch/11056379/
Seems this is the direction to move out from extcon.
Li Jun
>
> Peter
>
> > Signed-off-by: Li Jun
> > ---
> >
> > Change for v2:
> > - Support USB_ROLE_NONE, which for usb port not connecting any
> > device or host, and will be in low power mode.
> >
> > drivers/usb/chipidea/ci.h | 3 ++
> > drivers/usb/chipidea/core.c | 120
> > +---
> >
> > drivers/usb/chipidea/otg.c | 20
> > 3 files changed, 114 insertions(+), 29 deletions(-)
> >
> > diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
> > index 82b86cd..f0aec1d 100644
> > --- a/drivers/usb/chipidea/ci.h
> > +++ b/drivers/usb/chipidea/ci.h
> > @@ -205,6 +205,7 @@ struct ci_hdrc {
> > int irq;
> > struct ci_role_driver *roles[USB_ROLE_DEVICE + 1];
> > enum usb_role role;
> > + enum usb_role target_role;
> > boolis_otg;
> > struct usb_otg otg;
> > struct otg_fsm fsm;
> > @@ -212,6 +213,7 @@ struct ci_hdrc {
> > ktime_t
> > hr_timeouts[NUM_OTG_FSM_TIMERS];
> > unsignedenabled_otg_timer_bits;
> > enum otg_fsm_timer next_otg_timer;
> > + struct usb_role_switch *role_switch;
> > struct work_struct work;
> > struct workqueue_struct *wq;
> >
> > @@ -244,6 +246,7 @@ struct ci_hdrc {
> > struct dentry *debugfs;
> > boolid_event;
> > boolb_sess_valid_event;
> > + boolrole_switch_event;
> > boolimx28_write_fix;
> > boolsupports_runtime_pm;
> > boolin_lpm;
> > diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> > index
> > bc24c5b..965ce17 100644
> > --- a/drivers/usb/chipidea/core.c
> > +++ b/drivers/usb/chipidea/core.c
> > @@ -587,6 +587,42 @@ static irqreturn_t ci_irq(int irq, void *data)
> > return ret;
> > }
> >
> > +static int ci_usb_role_switch_set(struct device *dev, enum usb_role
> > +role) {
> > + struct ci_hdrc *ci = dev_get_drvdata(dev);
> > + unsigned long flags;
> > +
> > + if (ci->role == role)
> > + return 0;
> > +
> > + spin_lock_irqsave(&ci->lock, flags);
> > + ci->role_switch_event = true;
> > + ci->target_role = role;
> > + spin_unlock_irqrestore(&ci->lock, flags);
> > +
> > + ci_otg_queue_work(ci);
> > +
> > + return 0;
> > +}
> > +
> > +static enum usb_role ci_usb_role_switch_get(struct device *dev) {
> > + struct ci_hdrc *ci = dev_get_drvdata(dev);
> > + unsigned long flags;
> > + enum usb_role role;
> > +
> > + spin_lock_irqsave(&ci->lock, flags);
> > + role = ci->role;
> > + spin_unlock_irqrestore(&ci->lock, flags);
> > +
> > + return role;
> > +}
> > +
> > +static struct usb_role_switch_desc ci_role_switch = {
> > + .set = ci_usb_role_switch_set,
> > + .get = ci_usb_role_switch_get,
> > +};
> > +
> > static int ci_cable_notifier(struct notifier_block *nb, unsigned long
> > event,
> > void *ptr)
> > {
> > @@ -689,6 +725,9 @@ static int ci_get_platdata(struct device *dev,
> > if (of_find_property(dev->of_node, "non-zero-ttctrl-ttha", NULL))
> > platdata->flags |= CI_HDRC_SET_NON_ZERO_TTHA;
> >
> > + if (device_property_read_bool(dev, "usb-role-switch"))
> > + ci_role_switch.fwnode = dev->fwnode;
> >
AW: KASAN: use-after-free Read in usbhid_power
Hi all having use-after-free issues in USB shutdowns:
I hunted for a similar case in the intel_xhci_usb_sw driver.
What i have found and proposed is (from yesterday):
---
[PATCH] kernel/resource.c: invalidate parent when freed resource has childs
When a resource is freed and has children, the childrens are
left without any hint that their parent is no more valid.
This caused at least one use-after-free in the xhci-hcd using
ext-caps driver when platform code released platform devices.
Fix this by setting child's parent to zero and warn.
Signed-off-by: Carsten Schmid
---
Rationale:
When hunting for the root cause of a crash on a 4.14.86 kernel, i
have found the root cause and checked it being still present
upstream. Our case:
Having xhci-hcd and intel_xhci_usb_sw active we can see in
/proc/meminfo: (exceirpt)
b3c0-b3c0 : :00:15.0
b3c0-b3c0 : xhci-hcd
b3c08070-b3c0846f : intel_xhci_usb_sw
intel_xhci_usb_sw being a child of xhci-hcd.
Doing an unbind command
echo :00:15.0 > /sys/bus/pci/drivers/xhci_hcd/unbind
leads to xhci-hcd being freed in __release_region.
The intel_xhci_usb_sw resource is accessed in platform code
in platform_device_del with
for (i = 0; i < pdev->num_resources; i++) {
struct resource *r = &pdev->resource[i];
if (r->parent)
release_resource(r);
}
as the resource's parent has not been updated, the release_resource
uses the parent:
p = &old->parent->child;
which is now invalid.
Fix this by marking the parent invalid in the child and give a warning
in dmesg.
---
kernel/resource.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/kernel/resource.c b/kernel/resource.c
index 158f04ec1d4f..95340cb0b1c2 100644
--- a/kernel/resource.c
+++ b/kernel/resource.c
@@ -1200,6 +1200,15 @@ void __release_region(struct resource *parent,
resource_size_t start,
write_unlock(&resource_lock);
if (res->flags & IORESOURCE_MUXED)
wake_up(&muxed_resource_wait);
+
+ write_lock(&resource_lock);
+ if (res->child) {
+ printk(KERN_WARNING "__release_region: %s has
child %s,"
+ "invalidating childs parent\n",
+ res->name, res->child->name);
+ res->child->parent = NULL;
+ }
+ write_unlock(&resource_lock);
free_resource(res);
return;
}
--
2.17.1
> -Ursprüngliche Nachricht-
> Von: linux-usb-ow...@vger.kernel.org [mailto:linux-usb-
> ow...@vger.kernel.org] Im Auftrag von Alan Stern
> Gesendet: Donnerstag, 8. August 2019 21:37
> An: Andrey Konovalov
> Cc: Oliver Neukum ; syzkaller-bugs b...@googlegroups.com>; syzbot
> ; USB list
> ; Hillf Danton
> Betreff: Re: KASAN: use-after-free Read in usbhid_power
>
> On Thu, 8 Aug 2019, Andrey Konovalov wrote:
>
> > On Thu, Jul 25, 2019 at 5:09 PM Alan Stern
> wrote:
> > >
> > > On Thu, 25 Jul 2019, Oliver Neukum wrote:
> > >
> > > > Am Mittwoch, den 24.07.2019, 17:02 -0400 schrieb Alan Stern:
> > > > > On Wed, 24 Jul 2019, Oliver Neukum wrote:
> > > > >
> > > > > > drivers/hid/usbhid/hid-core.c | 13 +
> > > > > > 1 file changed, 13 insertions(+)
> > > > > >
> > > > > > diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-
> core.c
> > > > > > index c7bc9db5b192..98b996ecf4d3 100644
> > > > > > --- a/drivers/hid/usbhid/hid-core.c
> > > > > > +++ b/drivers/hid/usbhid/hid-core.c
> > > > > > @@ -1229,6 +1229,17 @@ static int usbhid_power(struct hid_device
> *hid, int lvl)
> > > > > > struct usbhid_device *usbhid = hid->driver_data;
> > > > > > int r = 0;
> > > > > >
> > > > > > + spin_lock_irq(&usbhid->lock);
> > > > > > + if (test_bit(HID_DISCONNECTED, &usbhid->iofl)) {
> > > > > > + r = -ENODEV;
> > > > > > + spin_unlock_irq(&usbhid->lock);
> > > > > > + goto bail_out;
> > > > > > + } else {
> > > > > > + /* protect against disconnect */
> > > > > > + usb_get_dev(interface_to_usbdev(usbhid->intf));
> > > > > > + spin_unlock_irq(&usbhid->lock);
> > > > > > + }
> > > > > > +
> > > > > > switch (lvl) {
> > > > > > case PM_HINT_FULLON:
> > > > > > r = usb_autopm_get_interface(usbhid->intf);
> > > > > > @@ -1238,7 +1249,9 @@ static int usbhid_power(struct hid_device
> *hid, int lvl)
> > > > > > usb_autopm_put_interface(usbhid->intf);
> > > > > > break;
> > > > > > }
> > > > > > + usb_put_dev(interface_to_usbdev(usbhid->intf));
> > > > > >
> > > > > > +bail_out:
> > > > > > return r;
> > > > > > }
>
> > This report looks like very similar to these two:
> >
> > https://syzkaller.appspot.com/bug?extid=b156665cf4d1b
Re: [PATCH] [RFC] usb: gadget: hid: Add "single_ep" option
Hi, Benjamin Herrenschmidt writes: > On Fri, 2019-08-09 at 08:31 +0300, Felipe Balbi wrote: >> Hi, >> >> Benjamin Herrenschmidt writes: >> >> > Some host drivers really do not like keyboards having an OUT >> > endpoint. >> > >> > For example, most UEFI forked from EDK2 before 2006 (or was it 2008 >> > ?) >> > have a bug, they'll try to use the *last* interrupt EP in the >> > descriptor list and just assume it's an IN endpoint. Newer UEFIs >> > use the *first* interrupt endpoint instead. None of them checks the >> > direction :-( >> > >> > This adds a "single_ep" option to f_hid which allows to specify >> > that >> > only the IN path should be created. This should be used for >> > keyboards >> > if they are ever to be used with such systems as host. >> > >> > Signed-off-by: Benjamin Herrenschmidt >> >> Could you come up with a slightly more descriptive name? single_ep >> doesn't give me any hint of which endpoint will be left around. >> >> Perhaps call it 'disable_output_report'? > > Sure. Or more concice "input_only" maybe ? that works too. Another option would to introduce two options, has_input_report and has_output_report and have them true by default. Then user can even produce an output-only HID device, like these odd USB-controlled relays. -- balbi
Re: [PATCH] [RFC] usb: gadget: hid: Add "single_ep" option
On Fri, 2019-08-09 at 11:08 +0300, Felipe Balbi wrote: > > that works too. Another option would to introduce two options, > has_input_report and has_output_report and have them true by default. > > Then user can even produce an output-only HID device, like these odd > USB-controlled relays. Ideally we could just parse the descriptor :-) But that's a bit trickier. One thing I've been meaning to look at is a way to properly handle keyboards (and other "on/off" buttons) by having the kernel latch the state so it can properly respond to things like get report, or resend all the "down" keys after a reset (so that things like "keep some key pressed during boot for magic to happen") actually works... But one thing at a time :-) Cheers, Ben.
KMSAN: uninit-value in smsc75xx_bind
Hello, syzbot found the following crash on: HEAD commit:beaab8a3 fix KASAN build git tree: kmsan console output: https://syzkaller.appspot.com/x/log.txt?x=13d7b65c60 kernel config: https://syzkaller.appspot.com/x/.config?x=4db781fe35a84ef5 dashboard link: https://syzkaller.appspot.com/bug?extid=6966546b78d050bb0b5d compiler: clang version 9.0.0 (/home/glider/llvm/clang 80fee25776c2fb61e74c1ecb1a523375c2500b69) syz repro: https://syzkaller.appspot.com/x/repro.syz?x=14ab9ef060 C reproducer: https://syzkaller.appspot.com/x/repro.c?x=11be2b3460 IMPORTANT: if you fix the bug, please add the following tag to the commit: Reported-by: syzbot+6966546b78d050bb0...@syzkaller.appspotmail.com == BUG: KMSAN: uninit-value in smsc75xx_wait_ready drivers/net/usb/smsc75xx.c:976 [inline] BUG: KMSAN: uninit-value in smsc75xx_bind+0x541/0x12d0 drivers/net/usb/smsc75xx.c:1483 CPU: 0 PID: 2892 Comm: kworker/0:2 Not tainted 5.2.0+ #15 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 Workqueue: usb_hub_wq hub_event Call Trace: __dump_stack lib/dump_stack.c:77 [inline] dump_stack+0x191/0x1f0 lib/dump_stack.c:113 kmsan_report+0x162/0x2d0 mm/kmsan/kmsan_report.c:109 __msan_warning+0x75/0xe0 mm/kmsan/kmsan_instr.c:294 smsc75xx_wait_ready drivers/net/usb/smsc75xx.c:976 [inline] smsc75xx_bind+0x541/0x12d0 drivers/net/usb/smsc75xx.c:1483 usbnet_probe+0x10d3/0x3950 drivers/net/usb/usbnet.c:1722 usb_probe_interface+0xd19/0x1310 drivers/usb/core/driver.c:361 really_probe+0x1344/0x1d90 drivers/base/dd.c:513 driver_probe_device+0x1ba/0x510 drivers/base/dd.c:670 __device_attach_driver+0x5b8/0x790 drivers/base/dd.c:777 bus_for_each_drv+0x28e/0x3b0 drivers/base/bus.c:454 __device_attach+0x489/0x750 drivers/base/dd.c:843 device_initial_probe+0x4a/0x60 drivers/base/dd.c:890 bus_probe_device+0x131/0x390 drivers/base/bus.c:514 device_add+0x25b5/0x2df0 drivers/base/core.c:2111 usb_set_configuration+0x309f/0x3710 drivers/usb/core/message.c:2027 generic_probe+0xe7/0x280 drivers/usb/core/generic.c:210 usb_probe_device+0x146/0x200 drivers/usb/core/driver.c:266 really_probe+0x1344/0x1d90 drivers/base/dd.c:513 driver_probe_device+0x1ba/0x510 drivers/base/dd.c:670 __device_attach_driver+0x5b8/0x790 drivers/base/dd.c:777 bus_for_each_drv+0x28e/0x3b0 drivers/base/bus.c:454 __device_attach+0x489/0x750 drivers/base/dd.c:843 device_initial_probe+0x4a/0x60 drivers/base/dd.c:890 bus_probe_device+0x131/0x390 drivers/base/bus.c:514 device_add+0x25b5/0x2df0 drivers/base/core.c:2111 usb_new_device+0x23e5/0x2fb0 drivers/usb/core/hub.c:2534 hub_port_connect drivers/usb/core/hub.c:5089 [inline] hub_port_connect_change drivers/usb/core/hub.c:5204 [inline] port_event drivers/usb/core/hub.c:5350 [inline] hub_event+0x5853/0x7320 drivers/usb/core/hub.c:5432 process_one_work+0x1572/0x1f00 kernel/workqueue.c:2269 process_scheduled_works kernel/workqueue.c:2331 [inline] worker_thread+0x189c/0x2460 kernel/workqueue.c:2417 kthread+0x4b5/0x4f0 kernel/kthread.c:256 ret_from_fork+0x35/0x40 arch/x86/entry/entry_64.S:355 Local variable description: buf.i93@smsc75xx_bind Variable was created at: __smsc75xx_read_reg drivers/net/usb/smsc75xx.c:83 [inline] smsc75xx_wait_ready drivers/net/usb/smsc75xx.c:969 [inline] smsc75xx_bind+0x44c/0x12d0 drivers/net/usb/smsc75xx.c:1483 usbnet_probe+0x10d3/0x3950 drivers/net/usb/usbnet.c:1722 == --- This bug is generated by a bot. It may contain errors. See https://goo.gl/tpsmEJ for more information about syzbot. syzbot engineers can be reached at syzkal...@googlegroups.com. syzbot will keep track of this bug report. See: https://goo.gl/tpsmEJ#status for how to communicate with syzbot. syzbot can test patches for this bug, for details see: https://goo.gl/tpsmEJ#testing-patches
Re: [PATCH 10/22] usb: omap: avoid mach/*.h headers
On Thu, Aug 08, 2019 at 11:22:19PM +0200, Arnd Bergmann wrote: > The omap usb drivers still rely on mach/*.h headers that > are explicitly or implicitly included, but all the required > definitions are now in include/linux/soc/ti/, so use those > instead and allow compile-testing on other architectures. > > Signed-off-by: Arnd Bergmann > --- > drivers/usb/gadget/udc/Kconfig | 2 +- > drivers/usb/gadget/udc/omap_udc.c | 2 ++ > drivers/usb/host/Kconfig | 2 +- > drivers/usb/host/ohci-omap.c | 7 +++ > drivers/usb/phy/Kconfig| 3 ++- > drivers/usb/phy/phy-isp1301-omap.c | 4 ++-- > 6 files changed, 11 insertions(+), 9 deletions(-) Acked-by: Greg Kroah-Hartman
Re: KASAN: use-after-free Read in usbhid_power
On Fri, Aug 09, 2019 at 07:35:32AM +, Schmid, Carsten wrote:
> Hi all having use-after-free issues in USB shutdowns:
> I hunted for a similar case in the intel_xhci_usb_sw driver.
> What i have found and proposed is (from yesterday):
> ---
> [PATCH] kernel/resource.c: invalidate parent when freed resource has childs
>
> When a resource is freed and has children, the childrens are
> left without any hint that their parent is no more valid.
> This caused at least one use-after-free in the xhci-hcd using
> ext-caps driver when platform code released platform devices.
>
> Fix this by setting child's parent to zero and warn.
>
> Signed-off-by: Carsten Schmid
> ---
> Rationale:
> When hunting for the root cause of a crash on a 4.14.86 kernel, i
> have found the root cause and checked it being still present
> upstream. Our case:
> Having xhci-hcd and intel_xhci_usb_sw active we can see in
> /proc/meminfo: (exceirpt)
> b3c0-b3c0 : :00:15.0
> b3c0-b3c0 : xhci-hcd
> b3c08070-b3c0846f : intel_xhci_usb_sw
> intel_xhci_usb_sw being a child of xhci-hcd.
>
> Doing an unbind command
> echo :00:15.0 > /sys/bus/pci/drivers/xhci_hcd/unbind
> leads to xhci-hcd being freed in __release_region.
> The intel_xhci_usb_sw resource is accessed in platform code
> in platform_device_del with
> for (i = 0; i < pdev->num_resources; i++) {
> struct resource *r = &pdev->resource[i];
> if (r->parent)
> release_resource(r);
> }
> as the resource's parent has not been updated, the release_resource
> uses the parent:
> p = &old->parent->child;
> which is now invalid.
> Fix this by marking the parent invalid in the child and give a warning
> in dmesg.
> ---
> kernel/resource.c | 9 +
> 1 file changed, 9 insertions(+)
>
> diff --git a/kernel/resource.c b/kernel/resource.c
> index 158f04ec1d4f..95340cb0b1c2 100644
> --- a/kernel/resource.c
> +++ b/kernel/resource.c
> @@ -1200,6 +1200,15 @@ void __release_region(struct resource *parent,
> resource_size_t start,
> write_unlock(&resource_lock);
> if (res->flags & IORESOURCE_MUXED)
> wake_up(&muxed_resource_wait);
> +
> + write_lock(&resource_lock);
Nit, should't this be written so that you only drop/get the lock if the
above if statement is true?
> + if (res->child) {
> + printk(KERN_WARNING "__release_region: %s has
> child %s,"
> + "invalidating childs
> parent\n",
> + res->name, res->child->name);
What can userspace/anyone do about this if it triggers?
Can't we fix the root problem in the xhci driver to properly order how
it tears things down? If it has intel_xhci_usb_driver as a "child"
shouldn't that be unbound/freed before the parent is?
thanks,
greg k-h
Re: [balbi-usb:testing/next 2/13] drivers/usb/phy/phy-tahvo.c:434:4: error: 'struct device_driver' has no member named 'dev_groups'; did you mean 'groups'?
On Fri, Aug 09, 2019 at 08:27:52AM +0300, Felipe Balbi wrote: > > Hi, > > kbuild test robot writes: > > > tree: > > https://kernel.googlesource.com/pub/scm/linux/kernel/git/balbi/usb.git > > testing/next > > head: d06a2c3f683a591efce9d02b2b60ef346df5ae02 > > commit: 2a714ea6d90d9d1b510ba424652a2e3dfd547267 [2/13] USB: phy: tahvo: > > convert platform driver to use dev_groups > > config: sh-allmodconfig (attached as .config) > > compiler: sh4-linux-gcc (GCC) 7.4.0 > > reproduce: > > wget > > https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O > > ~/bin/make.cross > > chmod +x ~/bin/make.cross > > git checkout 2a714ea6d90d9d1b510ba424652a2e3dfd547267 > > # save the attached .config to linux build tree > > GCC_VERSION=7.4.0 make.cross ARCH=sh > > > > If you fix the issue, kindly add following tag > > Reported-by: kbuild test robot > > > > All errors (new ones prefixed by >>): > > > >>> drivers/usb/phy/phy-tahvo.c:434:4: error: 'struct device_driver' has no > >>> member named 'dev_groups'; did you mean 'groups'? > > .dev_groups = tahvo_groups, > >^~ > > > looks like these patches depend on something else that's not upstream > yet. I'll drop the patches from my queue. Greg,if you'd like to add my > ack: > > Acked-by: Felipe Balbi Thanks, they depend on a patch that is in linux-next and in my tree (and a few other developer's trees) already. greg k-h
Re: BUG: bad usercopy in ld_usb_read
On Thu, Aug 08, 2019 at 04:06:32PM -0700, Kees Cook wrote:
> On Thu, Aug 08, 2019 at 02:46:54PM +0200, Greg KH wrote:
> > On Thu, Aug 08, 2019 at 05:38:06AM -0700, syzbot wrote:
> > > Hello,
> > >
> > > syzbot found the following crash on:
> > >
> > > HEAD commit:e96407b4 usb-fuzzer: main usb gadget fuzzer driver
> > > git tree: https://github.com/google/kasan.git usb-fuzzer
> > > console output: https://syzkaller.appspot.com/x/log.txt?x=13aeaece60
> > > kernel config: https://syzkaller.appspot.com/x/.config?x=cfa2c18fb6a8068e
> > > dashboard link:
> > > https://syzkaller.appspot.com/bug?extid=45b2f40f0778cfa7634e
> > > compiler: gcc (GCC) 9.0.0 20181231 (experimental)
> > >
> > > Unfortunately, I don't have any reproducer for this crash yet.
> > >
> > > IMPORTANT: if you fix the bug, please add the following tag to the commit:
> > > Reported-by: syzbot+45b2f40f0778cfa76...@syzkaller.appspotmail.com
> > >
> > > ldusb 6-1:0.124: Read buffer overflow, -131383996186150 bytes dropped
> >
> > That's a funny number :)
> >
> > Nice overflow found, I see you are now starting to fuzz the char device
> > nodes of usb drivers...
> >
> > Michael, care to fix this up?
>
> This looks like the length in the read-from-device buffer is unchecked:
>
> /* actual_buffer contains actual_length + interrupt_in_buffer */
> actual_buffer = (size_t *)(dev->ring_buffer + dev->ring_tail *
> (sizeof(size_t)+dev->interrupt_in_endpoint_size));
> bytes_to_read = min(count, *actual_buffer);
> if (bytes_to_read < *actual_buffer)
> dev_warn(&dev->intf->dev, "Read buffer overflow, %zd bytes
> dropped\n",
> *actual_buffer-bytes_to_read);
>
> /* copy one interrupt_in_buffer from ring_buffer into userspace */
> if (copy_to_user(buffer, actual_buffer+1, bytes_to_read)) {
> retval = -EFAULT;
> goto unlock_exit;
> }
>
> I assume what's stored at actual_buffer is bogus and needs validation
> somewhere before it's actually used. (If not here, maybe where ever the
> write into the buffer originally happens?)
I think it should be verified here, as that's when it is parsed. The
data is written to the buffer in ld_usb_interrupt_in_callback() but it
does not "know" how to parse it at that location.
thanks,
greg k-h
Re: KASAN: use-after-free Read in wdm_int_callback
syzbot has found a reproducer for the following crash on: HEAD commit:e96407b4 usb-fuzzer: main usb gadget fuzzer driver git tree: https://github.com/google/kasan.git usb-fuzzer console output: https://syzkaller.appspot.com/x/log.txt?x=15b5018c60 kernel config: https://syzkaller.appspot.com/x/.config?x=cfa2c18fb6a8068e dashboard link: https://syzkaller.appspot.com/bug?extid=1a3765ef3c0d49d36a75 compiler: gcc (GCC) 9.0.0 20181231 (experimental) syz repro: https://syzkaller.appspot.com/x/repro.syz?x=14d9a9c260 C reproducer: https://syzkaller.appspot.com/x/repro.c?x=16f1a18c60 IMPORTANT: if you fix the bug, please add the following tag to the commit: Reported-by: syzbot+1a3765ef3c0d49d36...@syzkaller.appspotmail.com == BUG: KASAN: use-after-free in wdm_int_callback+0x459/0x4c0 drivers/usb/class/cdc-wdm.c:239 Read of size 8 at addr 8881cf909690 by task kworker/1:2/1742 CPU: 1 PID: 1742 Comm: kworker/1:2 Not tainted 5.3.0-rc2+ #25 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 Workqueue: usb_hub_wq hub_event Call Trace: __dump_stack lib/dump_stack.c:77 [inline] dump_stack+0xca/0x13e lib/dump_stack.c:113 print_address_description+0x6a/0x32c mm/kasan/report.c:351 __kasan_report.cold+0x1a/0x33 mm/kasan/report.c:482 kasan_report+0xe/0x12 mm/kasan/common.c:612 wdm_int_callback+0x459/0x4c0 drivers/usb/class/cdc-wdm.c:239 __usb_hcd_giveback_urb+0x1f2/0x470 drivers/usb/core/hcd.c:1757 usb_hcd_giveback_urb+0x368/0x420 drivers/usb/core/hcd.c:1822 dummy_timer+0x120f/0x2fa2 drivers/usb/gadget/udc/dummy_hcd.c:1965 call_timer_fn+0x179/0x650 kernel/time/timer.c:1322 expire_timers kernel/time/timer.c:1366 [inline] __run_timers kernel/time/timer.c:1685 [inline] __run_timers kernel/time/timer.c:1653 [inline] run_timer_softirq+0x5cc/0x14b0 kernel/time/timer.c:1698 __do_softirq+0x221/0x912 kernel/softirq.c:292 invoke_softirq kernel/softirq.c:373 [inline] irq_exit+0x178/0x1a0 kernel/softirq.c:413 exiting_irq arch/x86/include/asm/apic.h:537 [inline] smp_apic_timer_interrupt+0x12f/0x500 arch/x86/kernel/apic/apic.c:1095 apic_timer_interrupt+0xf/0x20 arch/x86/entry/entry_64.S:828 RIP: 0010:arch_local_irq_restore arch/x86/include/asm/irqflags.h:85 [inline] RIP: 0010:console_unlock+0xa2a/0xc40 kernel/printk/printk.c:2471 Code: 00 89 ee 48 c7 c7 20 88 d3 86 e8 81 ad 03 00 65 ff 0d 72 a1 d9 7e e9 db f9 ff ff e8 70 a1 15 00 e8 1b cb 1a 00 ff 74 24 30 9d 18 fe ff ff e8 5c a1 15 00 48 8d 7d 08 48 89 f8 48 c1 e8 03 42 RSP: 0018:8881d29feea8 EFLAGS: 0293 ORIG_RAX: ff13 RAX: 0007 RBX: 0200 RCX: 0006 RDX: RSI: 0008 RDI: 8881d3478844 RBP: R08: 8881d3478000 R09: fbfff11acd91 R10: fbfff11acd90 R11: 88d66c87 R12: 0061 R13: dc00 R14: 82909100 R15: 87077190 vprintk_emit+0x171/0x3e0 kernel/printk/printk.c:1986 dev_vprintk_emit+0x4fc/0x541 drivers/base/core.c:3232 dev_printk_emit+0xba/0xf1 drivers/base/core.c:3243 __dev_printk+0x1db/0x203 drivers/base/core.c:3255 _dev_err+0xd7/0x109 drivers/base/core.c:3298 usbvision_probe.cold+0x1c1/0x1e57 drivers/media/usb/usbvision/usbvision-video.c:1442 usb_probe_interface+0x305/0x7a0 drivers/usb/core/driver.c:361 really_probe+0x281/0x650 drivers/base/dd.c:548 driver_probe_device+0x101/0x1b0 drivers/base/dd.c:709 __device_attach_driver+0x1c2/0x220 drivers/base/dd.c:816 bus_for_each_drv+0x15c/0x1e0 drivers/base/bus.c:454 __device_attach+0x217/0x360 drivers/base/dd.c:882 bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514 device_add+0xae6/0x16f0 drivers/base/core.c:2114 usb_set_configuration+0xdf6/0x1670 drivers/usb/core/message.c:2023 generic_probe+0x9d/0xd5 drivers/usb/core/generic.c:210 usb_probe_device+0x99/0x100 drivers/usb/core/driver.c:266 really_probe+0x281/0x650 drivers/base/dd.c:548 driver_probe_device+0x101/0x1b0 drivers/base/dd.c:709 __device_attach_driver+0x1c2/0x220 drivers/base/dd.c:816 bus_for_each_drv+0x15c/0x1e0 drivers/base/bus.c:454 __device_attach+0x217/0x360 drivers/base/dd.c:882 bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514 device_add+0xae6/0x16f0 drivers/base/core.c:2114 usb_new_device.cold+0x6a4/0xe79 drivers/usb/core/hub.c:2536 hub_port_connect drivers/usb/core/hub.c:5098 [inline] hub_port_connect_change drivers/usb/core/hub.c:5213 [inline] port_event drivers/usb/core/hub.c:5359 [inline] hub_event+0x1b5c/0x3640 drivers/usb/core/hub.c:5441 process_one_work+0x92b/0x1530 kernel/workqueue.c:2269 worker_thread+0x96/0xe20 kernel/workqueue.c:2415 kthread+0x318/0x420 kernel/kthread.c:255 ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352 Allocated by task 1742: save_stack+0x1b/0x80 mm/kasan/common.c:69 set_track mm/kasan/common.c:77 [inline] __kasan_kmalloc mm/kasan/common.c:487 [inline] __kasan_kmalloc.c
Re: [PATCH] [RFC] usb: gadget: hid: Add "single_ep" option
Hi, Benjamin Herrenschmidt writes: > On Fri, 2019-08-09 at 11:08 +0300, Felipe Balbi wrote: >> >> that works too. Another option would to introduce two options, >> has_input_report and has_output_report and have them true by default. >> >> Then user can even produce an output-only HID device, like these odd >> USB-controlled relays. > > Ideally we could just parse the descriptor :-) But that's a bit > trickier. > > One thing I've been meaning to look at is a way to properly handle > keyboards (and other "on/off" buttons) by having the kernel latch the > state so it can properly respond to things like get report, or resend > all the "down" keys after a reset (so that things like "keep some key > pressed during boot for magic to happen") actually works... cool idea > But one thing at a time :-) yup -- balbi
AW: KASAN: use-after-free Read in usbhid_power
> -Ursprüngliche Nachricht-
> Von: Greg KH [mailto:gre...@linuxfoundation.org]
> Gesendet: Freitag, 9. August 2019 09:56
> An: Schmid, Carsten
> Cc: Alan Stern ; Andrey Konovalov
> ; Oliver Neukum ;
> syzkaller-bugs ; syzbot
> ; USB list
> ; Hillf Danton
> Betreff: Re: KASAN: use-after-free Read in usbhid_power
>
> On Fri, Aug 09, 2019 at 07:35:32AM +, Schmid, Carsten wrote:
> > Hi all having use-after-free issues in USB shutdowns:
> > I hunted for a similar case in the intel_xhci_usb_sw driver.
> > What i have found and proposed is (from yesterday):
> > ---
> > [PATCH] kernel/resource.c: invalidate parent when freed resource has
> childs
> >
> > When a resource is freed and has children, the childrens are
> > left without any hint that their parent is no more valid.
> > This caused at least one use-after-free in the xhci-hcd using
> > ext-caps driver when platform code released platform devices.
> >
> > Fix this by setting child's parent to zero and warn.
> >
> > Signed-off-by: Carsten Schmid
> > ---
> > Rationale:
> > When hunting for the root cause of a crash on a 4.14.86 kernel, i
> > have found the root cause and checked it being still present
> > upstream. Our case:
> > Having xhci-hcd and intel_xhci_usb_sw active we can see in
> > /proc/meminfo: (exceirpt)
> > b3c0-b3c0 : :00:15.0
> > b3c0-b3c0 : xhci-hcd
> > b3c08070-b3c0846f : intel_xhci_usb_sw
> > intel_xhci_usb_sw being a child of xhci-hcd.
> >
> > Doing an unbind command
> > echo :00:15.0 > /sys/bus/pci/drivers/xhci_hcd/unbind
> > leads to xhci-hcd being freed in __release_region.
> > The intel_xhci_usb_sw resource is accessed in platform code
> > in platform_device_del with
> > for (i = 0; i < pdev->num_resources; i++) {
> > struct resource *r = &pdev->resource[i];
> > if (r->parent)
> > release_resource(r);
> > }
> > as the resource's parent has not been updated, the release_resource
> > uses the parent:
> > p = &old->parent->child;
> > which is now invalid.
> > Fix this by marking the parent invalid in the child and give a warning
> > in dmesg.
> > ---
> > kernel/resource.c | 9 +
> > 1 file changed, 9 insertions(+)
> >
> > diff --git a/kernel/resource.c b/kernel/resource.c
> > index 158f04ec1d4f..95340cb0b1c2 100644
> > --- a/kernel/resource.c
> > +++ b/kernel/resource.c
> > @@ -1200,6 +1200,15 @@ void __release_region(struct resource *parent,
> resource_size_t start,
> > write_unlock(&resource_lock);
> > if (res->flags & IORESOURCE_MUXED)
> > wake_up(&muxed_resource_wait);
> > +
> > + write_lock(&resource_lock);
>
> Nit, should't this be written so that you only drop/get the lock if the
> above if statement is true?
>
What if some other async part invalidates the child while accessing it?
I wanted to make sure that the res->child is valid through the time it is
accessed.
> > + if (res->child) {
> > + printk(KERN_WARNING "__release_region: %s
> > has child
> %s,"
> > + "invalidating childs
> > parent\n",
> > + res->name,
> > res->child->name);
>
> What can userspace/anyone do about this if it triggers?
>
At least a platform driver developer can see he did something wrong.
I had a look at the code of Hans and did not see anything weird.
(platform driver is in drivers/usb/host/xhci-ext-caps.c)
The issue is very racy - what happens when the platform code accesses the
resource mainly depends on if the freed resource memory already has been
reused by someone.
It was hard to find that, and only a single core dump enabled me to find it.
A first attempt was to set resource count to 0 in Hans' driver in the unregister
pdev before calling platform_device_unregister. That worked.
But this is a dirty hack in my eyes. The framework should detect such issues,
so i decided to find the best place where to insert the check - and
found it is the place where the resource is freed and still has childrens.
> Can't we fix the root problem in the xhci driver to properly order how
> it tears things down? If it has intel_xhci_usb_driver as a "child"
> shouldn't that be unbound/freed before the parent is?
>
Hans, any idea?
> thanks,
>
> greg k-h
RE: [PATCH v2 2/2] usb: chipidea: add role switch class support
> > > USB role is fully controlled by usb role switch consumer(e.g.
> > > typec), usb port can be at host mode(USB_ROLE_HOST), device mode
> > > connected to host(USB_ROLE_DEVICE), or not connecting any
> parter(USB_ROLE_NONE).
> > >
> >
> > %s/parter/partner ?
>
> Yes, I will update.
>
> >
> > Are there any ways you could get external cable status from usb-switch
> > or type-c like external connector? If there are, you could update
> > otgsc value for otgsc_read, or change cable status, and avoid changing
> > common
> handling, like ci_hdrc_probe and ci_otg_work.
> > And it could benefit for other use cases, like booting with cable
> > connected and switch role through /sys.
>
> The new role switch class has nothing to do with extcon, it's using graph
> bindings to
> link the connection, so there is no extcon_dev, change for ci_hdrc_probe() is
> required as property usb-role-switch has to be specified, there may be some
> code
> reuse if still touch the external cable even no extcon dev, but that will
> make existing
> external cable complicated and not clean.
>
I don't mean using extcon directly.
At ci_usb_role_switch_set, you could know current "id" and "vbus" status.
as well as if "id" and "vbus" has changed. So, you could update ci->id_event
or ci->vbus_event. And at otgsc_read, you could return correct vbus and id
value.
So, you may not need to change main function for ci_otg_work, for ci_hdrc_probe,
you could only need to register usb-switch class.
Surely, you may need to introduce some structures or variables for usb switch,
but
it could keep main structure not changing, and just differentiate the way to get
id and vbus.
> On the other hand, a new GPIO based role switch driver is on review:
> https://patchwork.kernel.org/patch/11056379/
> Seems this is the direction to move out from extcon.
>
If external connector is given up, we need to use that.
When you update the v3, please fix the build error reported by kbuild test
robot.
Peter
> Li Jun
> >
> > Peter
> >
> > > Signed-off-by: Li Jun
> > > ---
> > >
> > > Change for v2:
> > > - Support USB_ROLE_NONE, which for usb port not connecting any
> > > device or host, and will be in low power mode.
> > >
> > > drivers/usb/chipidea/ci.h | 3 ++
> > > drivers/usb/chipidea/core.c | 120
> > > +---
> > >
> > > drivers/usb/chipidea/otg.c | 20
> > > 3 files changed, 114 insertions(+), 29 deletions(-)
> > >
> > > diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
> > > index 82b86cd..f0aec1d 100644
> > > --- a/drivers/usb/chipidea/ci.h
> > > +++ b/drivers/usb/chipidea/ci.h
> > > @@ -205,6 +205,7 @@ struct ci_hdrc {
> > > int irq;
> > > struct ci_role_driver *roles[USB_ROLE_DEVICE + 1];
> > > enum usb_role role;
> > > + enum usb_role target_role;
> > > boolis_otg;
> > > struct usb_otg otg;
> > > struct otg_fsm fsm;
> > > @@ -212,6 +213,7 @@ struct ci_hdrc {
> > > ktime_t
> > > hr_timeouts[NUM_OTG_FSM_TIMERS];
> > > unsignedenabled_otg_timer_bits;
> > > enum otg_fsm_timer next_otg_timer;
> > > + struct usb_role_switch *role_switch;
> > > struct work_struct work;
> > > struct workqueue_struct *wq;
> > >
> > > @@ -244,6 +246,7 @@ struct ci_hdrc {
> > > struct dentry *debugfs;
> > > boolid_event;
> > > boolb_sess_valid_event;
> > > + boolrole_switch_event;
> > > boolimx28_write_fix;
> > > boolsupports_runtime_pm;
> > > boolin_lpm;
> > > diff --git a/drivers/usb/chipidea/core.c
> > > b/drivers/usb/chipidea/core.c index
> > > bc24c5b..965ce17 100644
> > > --- a/drivers/usb/chipidea/core.c
> > > +++ b/drivers/usb/chipidea/core.c
> > > @@ -587,6 +587,42 @@ static irqreturn_t ci_irq(int irq, void *data)
> > > return ret;
> > > }
> > >
> > > +static int ci_usb_role_switch_set(struct device *dev, enum usb_role
> > > +role) {
> > > + struct ci_hdrc *ci = dev_get_drvdata(dev);
> > > + unsigned long flags;
> > > +
> > > + if (ci->role == role)
> > > + return 0;
> > > +
> > > + spin_lock_irqsave(&ci->lock, flags);
> > > + ci->role_switch_event = true;
> > > + ci->target_role = role;
> > > + spin_unlock_irqrestore(&ci->lock, flags);
> > > +
> > > + ci_otg_queue_work(ci);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static enum usb_role ci_usb_role_switch_get(struct device *dev) {
> > > + struct ci_hdrc *ci = dev_get_drvdata(dev);
> > > + unsigned long flags;
> > > + enum usb_role role;
> > > +
> > > + spin_lock_irqsave(&ci->lock, flags);
> > > + role = ci->role;
> > > + spin_unlock_irqrestore(&ci->lock, flags);
> > > +
> > > + return role;
>
Re: AW: KASAN: use-after-free Read in usbhid_power
Hi,
On 8/9/19 11:34 AM, Schmid, Carsten wrote:
-Ursprüngliche Nachricht-
Von: Greg KH [mailto:gre...@linuxfoundation.org]
Gesendet: Freitag, 9. August 2019 09:56
An: Schmid, Carsten
Cc: Alan Stern ; Andrey Konovalov
; Oliver Neukum ;
syzkaller-bugs ; syzbot
; USB list
; Hillf Danton
Betreff: Re: KASAN: use-after-free Read in usbhid_power
On Fri, Aug 09, 2019 at 07:35:32AM +, Schmid, Carsten wrote:
Hi all having use-after-free issues in USB shutdowns:
I hunted for a similar case in the intel_xhci_usb_sw driver.
What i have found and proposed is (from yesterday):
---
[PATCH] kernel/resource.c: invalidate parent when freed resource has
childs
When a resource is freed and has children, the childrens are
left without any hint that their parent is no more valid.
This caused at least one use-after-free in the xhci-hcd using
ext-caps driver when platform code released platform devices.
Fix this by setting child's parent to zero and warn.
Signed-off-by: Carsten Schmid
---
Rationale:
When hunting for the root cause of a crash on a 4.14.86 kernel, i
have found the root cause and checked it being still present
upstream. Our case:
Having xhci-hcd and intel_xhci_usb_sw active we can see in
/proc/meminfo: (exceirpt)
b3c0-b3c0 : :00:15.0
b3c0-b3c0 : xhci-hcd
b3c08070-b3c0846f : intel_xhci_usb_sw
intel_xhci_usb_sw being a child of xhci-hcd.
Doing an unbind command
echo :00:15.0 > /sys/bus/pci/drivers/xhci_hcd/unbind
leads to xhci-hcd being freed in __release_region.
The intel_xhci_usb_sw resource is accessed in platform code
in platform_device_del with
for (i = 0; i < pdev->num_resources; i++) {
struct resource *r = &pdev->resource[i];
if (r->parent)
release_resource(r);
}
as the resource's parent has not been updated, the release_resource
uses the parent:
p = &old->parent->child;
which is now invalid.
Fix this by marking the parent invalid in the child and give a warning
in dmesg.
---
kernel/resource.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/kernel/resource.c b/kernel/resource.c
index 158f04ec1d4f..95340cb0b1c2 100644
--- a/kernel/resource.c
+++ b/kernel/resource.c
@@ -1200,6 +1200,15 @@ void __release_region(struct resource *parent,
resource_size_t start,
write_unlock(&resource_lock);
if (res->flags & IORESOURCE_MUXED)
wake_up(&muxed_resource_wait);
+
+ write_lock(&resource_lock);
Nit, should't this be written so that you only drop/get the lock if the
above if statement is true?
What if some other async part invalidates the child while accessing it?
I wanted to make sure that the res->child is valid through the time it is
accessed.
+ if (res->child) {
+ printk(KERN_WARNING "__release_region: %s has
child
%s,"
+ "invalidating childs parent\n",
+ res->name, res->child->name);
What can userspace/anyone do about this if it triggers?
At least a platform driver developer can see he did something wrong.
I had a look at the code of Hans and did not see anything weird.
(platform driver is in drivers/usb/host/xhci-ext-caps.c)
The issue is very racy - what happens when the platform code accesses the
resource mainly depends on if the freed resource memory already has been
reused by someone.
We are talking memory-mapped io here, so it cannot just be "re-used", it
is wat it is. I guess the PCI BAR could be released and then the physical
address the resource was at could be re-used for another piece of MMIo,
but AFAIK outside of PI=CI hotplug we never release BARs.
Maybe we need to ref-count resources and have the aprent free only be
a deref and not release the resource until the child resource also
is free-ed doing another deref?
I must say that to me it sometimes just seems like always allowing unbind
is a bad idea. Another example of this is things like virtio, where
we can have a filesystem based on virtio-block, but the virtio interface
between the hypervisor and the guest-kernel is a PCI-device and in theory
the user could unbind the virtio driver from that PCI-device, after which
the whole house comes crashing down.
I also know that the extcon framework in its current incarnaton
does not deal with unbind properly...
Maybe it is time that we allow drivers to block unbind instead of
trying to support unbind in really complex situations where normal
use-cases will never need it ?
I do realize unbind is very useful for driver developent without
rebooting.
It was hard to find that, and only a single core dump enabled me to find it.
A first attempt was to set resource count to 0 in Hans' driver in the unregister
pdev before calling platform_device_unregis
Re: [PATCH] usb: dwc3: remove generic PHYs forwarding for XHCI device
Hi,
On 08/08/2019 15:47, Felipe Balbi wrote:
>
> Hi,
>
> Marek Szyprowski writes:
>
>> Commit 08f871a3aca2 ("usb: dwc3: host: convey the PHYs to xhci") added
>> forwarding of the generic PHYs from DWC3 core to the instantiated XHCI-plat
>> device. However XHCI(-plat) driver never gained support for generic PHYs,
>> thus the lookup added by that commit is never used. In meantime the commit
>> d64ff406e51e ("usb: dwc3: use bus->sysdev for DMA configuration")
>> incorrectly changed the device used for creating lookup, making the lookup
>> useless and generic PHYs inaccessible from XHCI device.
>>
>> However since commit 178a0bce05cb ("usb: core: hcd: integrate the PHY
>> wrapper into the HCD core") USB HCD already handles generic PHYs acquired
>> from the HCD's 'sysdev', which in this case is DWC3 core device. This means
>> that creating any custom lookup entries for XHCI driver is no longer needed
>> and can be simply removed.
>>
>> Signed-off-by: Marek Szyprowski
>> ---
>> drivers/usb/dwc3/host.c | 22 --
>> 1 file changed, 4 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/usb/dwc3/host.c b/drivers/usb/dwc3/host.c
>> index f55947294f7c..8deea8c91e03 100644
>> --- a/drivers/usb/dwc3/host.c
>> +++ b/drivers/usb/dwc3/host.c
>> @@ -85,7 +85,7 @@ int dwc3_host_init(struct dwc3 *dwc)
>> DWC3_XHCI_RESOURCES_NUM);
>> if (ret) {
>> dev_err(dwc->dev, "couldn't add resources to xHCI device\n");
>> -goto err1;
>> +goto err;
>> }
>>
>> memset(props, 0, sizeof(struct property_entry) * ARRAY_SIZE(props));
>> @@ -112,37 +112,23 @@ int dwc3_host_init(struct dwc3 *dwc)
>> ret = platform_device_add_properties(xhci, props);
>> if (ret) {
>> dev_err(dwc->dev, "failed to add properties to xHCI\n");
>> -goto err1;
>> +goto err;
>> }
>> }
>>
>> -phy_create_lookup(dwc->usb2_generic_phy, "usb2-phy",
>> - dev_name(dwc->dev));
>> -phy_create_lookup(dwc->usb3_generic_phy, "usb3-phy",
>> - dev_name(dwc->dev));
>> -
>> ret = platform_device_add(xhci);
>> if (ret) {
>> dev_err(dwc->dev, "failed to register xHCI device\n");
>> -goto err2;
>> +goto err;
>> }
>>
>> return 0;
>> -err2:
>> -phy_remove_lookup(dwc->usb2_generic_phy, "usb2-phy",
>> - dev_name(dwc->dev));
>> -phy_remove_lookup(dwc->usb3_generic_phy, "usb3-phy",
>> - dev_name(dwc->dev));
>> -err1:
>> +err:
>> platform_device_put(xhci);
>> return ret;
>> }
>>
>> void dwc3_host_exit(struct dwc3 *dwc)
>> {
>> -phy_remove_lookup(dwc->usb2_generic_phy, "usb2-phy",
>> - dev_name(dwc->dev));
>> -phy_remove_lookup(dwc->usb3_generic_phy, "usb3-phy",
>> - dev_name(dwc->dev));
>> platform_device_unregister(dwc->xhci);
>> }
>
> Roger, could you verify that this doesn't regress any of your platforms?
Patch is fine with our platforms.
--
cheers,
-roger
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
Re: [PATCH v10 0/6] Introduced new Cadence USBSS DRD Driver.
Pawel, On 08/08/2019 07:12, Pawel Laszczak wrote: > Hi Roger, > >> >> >> On 23/07/2019 07:32, Pawel Laszczak wrote: >> >>> Hi, >> >>> >> On Mon 2019-07-22 13:56:44, Pavel Machek wrote: >> > Hi! >> > >> > This patch introduce new Cadence USBSS DRD driver to linux kernel. >> > >> > The Cadence USBSS DRD Controller is a highly configurable IP Core > which >> > can be instantiated as Dual-Role Device (DRD), Peripheral Only and >> > Host Only (XHCI)configurations. >> >> I see you are using debugfs to select between DRD, peripheral-onlyh and XHCI... >> >> Is that good idea? >> >>> >> >>> Yes driver allows selecting dr_mode by debugfs. Controller also support >>> such functionality >> >>> so I don't understand why would it not be a good idea. >> >>> >> >>> I personally use this for testing but it can be used to limit >>> controller functionality without >> >>> recompiling kernel. >> >> >> >> debugfs is ONLY for debugging, never rely on it being enabled, or >> >> mounted, on a system in order to have any normal operation happen. >> >> >> >> So for testing, yes, this is fine. If this is going to be the normal >> >> api/interface for how to control this driver, no, that is not acceptable >> >> at all. >> > >> > It makes a lot of sense for end-user to toggle this... for example >> > when he is lacking right cable for proper otg detection. As it is >> > third driver offering this functionality, I believe we should stop >> > treating it as debugging. >> >> At least renesas usb controller seems to have variables in sysfs: >> drivers/phy/renesas/phy-rcar-gen3-usb2.c : functions role_show and >> role_store. See also >> Documentation/ABI/testing/sysfs-platform-phy-rcar-gen3-usb2 . >> >> I believe this driver should do same. >> >> >>> >> >>> CDNS3 driver use the role framework and also has such variable defined >> >>> in role switch framework. >> >>> >> >>> https://urldefense.proofpoint.com/v2/url?u=https- >> 3A__elixir.bootlin.com_linux_latest_source_drivers_usb_roles_class.c&d=DwICaQ&c=aUq983L2pue2FqKFoP6PGHMJQyoJ7kl3s3GZ- >> _haXqY&r=e1OgxfvkL0qo9XO6fX1gscva-w03uSYC1nIyxl89-rI&m=_jBsEOB3gtoQVvsVk8k2Pz8dp9zhzZbbL4M0tINJLR8&s=mq5ce-d4Td- >> lc3OvcvektfSHhXPAL2Go2gWP-q9QwTY&e= >> > > The meaning is little different. Role switch framework allow to changing role > [ host -> device, device -> host ] > > The debugfs.c allows to limit dr_mode. > >> >> Can we get rid of the debugfs interface for user initiated role change and >> just >> >> rely on role switch framework via sysfs? >> >> >> >> We do need user initiated role changes in production systems. So we can't >> >> rely on debugfs for this. > > But I assume that in production systems this will be disabled. > cdns3-$(CONFIG_DEBUG_FS) += debugfs.o > But we do want user space based role switching in production systems. > I think that I understand your concerns. My idea was not to expand the > supported > dr_mode. Rather I wanted to have possibility to limit this (only for > testing). > > Eg. > If cdns->dr_mode = USB_DR_MODE_OTG > then we can limit mode to HOST or DEVICE or DRD In this case we register with role switch framework. > if cdns->dr_mode == USB_DR_MODE_HOST || >cdns->dr_mode == USB_DR_MODE_PERIPHERAL) > then driver can't change anything In this case we don't register to role switch framework. > > It allows me for testing some functionality using only single board > and even with lacking right cable for proper otg detection. > > So, removing this can cause that testing some functionality > will be limited on my boards. > > If you rely want to remove this, maybe we could do this > after putting this driver to kernel ?. I don't want you to remove the user based role change functionality. I'm just asking to rely on role switch framework for that and not debugfs. > > Maintaining this as my internal code before putting this driver > to kernel will be problematic. > > Regards, > Pawell > > -- cheers, -roger Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
Re: [PATCH v10 0/6] Introduced new Cadence USBSS DRD Driver.
Hi, Roger Quadros writes: >> It allows me for testing some functionality using only single board >> and even with lacking right cable for proper otg detection. >> >> So, removing this can cause that testing some functionality >> will be limited on my boards. >> >> If you rely want to remove this, maybe we could do this >> after putting this driver to kernel ?. > > I don't want you to remove the user based role change functionality. > I'm just asking to rely on role switch framework for that and not debugfs. I agree with Roger. Use role switch framework for production, not debugfs. -- balbi
Re: AW: AW: KASAN: use-after-free Read in usbhid_power
Hi, On 8/9/19 12:47 PM, Schmid, Carsten wrote: We are talking memory-mapped io here, so it cannot just be "re-used", it is wat it is. I guess the PCI BAR could be released and then the physical address the resource was at could be re-used for another piece of MMIo, but AFAIK outside of PI=CI hotplug we never release BARs. Maybe we need to ref-count resources and have the aprent free only be a deref and not release the resource until the child resource also is free-ed doing another deref? I must say that to me it sometimes just seems like always allowing unbind is a bad idea. Another example of this is things like virtio, where we can have a filesystem based on virtio-block, but the virtio interface between the hypervisor and the guest-kernel is a PCI-device and in theory the user could unbind the virtio driver from that PCI-device, after which the whole house comes crashing down. I also know that the extcon framework in its current incarnaton does not deal with unbind properly... Maybe it is time that we allow drivers to block unbind instead of trying to support unbind in really complex situations where normal use-cases will never need it ? I do realize unbind is very useful for driver developent without rebooting. Hey, i did not want to trigger an eartquake in the basement of the kernel ;-) My intention was to prevent some crashes, and help developers to find their bugs. I think my patch exactly does this. Hehe, actually drivers not being able to block unbind has been bugging me for a while now, because there are cases where this would be really helpful. 1) make resources refcounted, have child resources take a ref on the parent 2) Disallow unbind on devices with bound child-devices? Exactly what i was thinking of in first attempts. But i fear that would break even more use cases. Hans, directly regarding the driver: The problem i see is that the xhci_intel_unregister_pdev which is added as an action with devm_add_action_or_reset() is called late by the framework, later than the usb_hcd_pci_remove() in xhci_pci_remove. Is there any chance to trigger this before? This is what Greg meant with "right order". Ah, I missed that part, sure that should be easy, just stop using devm_add_action_or_reset() and do the xhci_intel_unregister_pdev() manually at the right time. The downside of this is that you also need to make sure it happens at the right time from probe error-paths but given the bug you are hitting, I guess that is probably already a problem. Regards, Hans
AW: AW: KASAN: use-after-free Read in usbhid_power
> > We are talking memory-mapped io here, so it cannot just be "re-used", it > is wat it is. I guess the PCI BAR could be released and then the physical > address the resource was at could be re-used for another piece of MMIo, > but AFAIK outside of PI=CI hotplug we never release BARs. > > Maybe we need to ref-count resources and have the aprent free only be > a deref and not release the resource until the child resource also > is free-ed doing another deref? > > I must say that to me it sometimes just seems like always allowing unbind > is a bad idea. Another example of this is things like virtio, where > we can have a filesystem based on virtio-block, but the virtio interface > between the hypervisor and the guest-kernel is a PCI-device and in theory > the user could unbind the virtio driver from that PCI-device, after which > the whole house comes crashing down. > > I also know that the extcon framework in its current incarnaton > does not deal with unbind properly... > > Maybe it is time that we allow drivers to block unbind instead of > trying to support unbind in really complex situations where normal > use-cases will never need it ? > > I do realize unbind is very useful for driver developent without > rebooting. > Hey, i did not want to trigger an eartquake in the basement of the kernel ;-) My intention was to prevent some crashes, and help developers to find their bugs. I think my patch exactly does this. > 1) make resources refcounted, have child resources take a ref on the parent > 2) Disallow unbind on devices with bound child-devices? > Exactly what i was thinking of in first attempts. But i fear that would break even more use cases. Hans, directly regarding the driver: The problem i see is that the xhci_intel_unregister_pdev which is added as an action with devm_add_action_or_reset() is called late by the framework, later than the usb_hcd_pci_remove() in xhci_pci_remove. Is there any chance to trigger this before? This is what Greg meant with "right order". Anyway, i really appreciate these discussions, thanks for all your patience. Best Regards Carsten
Re: [Patch V6 7/8] usb: gadget: Add UDC driver for tegra XUSB device mode controller
Hi, Nagarjuna Kristam writes: > This patch adds UDC driver for tegra XUSB 3.0 device mode controller. > XUSB device mode controller supports SS, HS and FS modes > > Based on work by: > Mark Kuo > Hui Fu > Andrew Bresticker > > Signed-off-by: Nagarjuna Kristam > Acked-by: Thierry Reding > --- > drivers/usb/gadget/udc/Kconfig | 11 + > drivers/usb/gadget/udc/Makefile |1 + > drivers/usb/gadget/udc/tegra_xudc.c | 3808 > +++ > 3 files changed, 3820 insertions(+) > create mode 100644 drivers/usb/gadget/udc/tegra_xudc.c > > diff --git a/drivers/usb/gadget/udc/Kconfig b/drivers/usb/gadget/udc/Kconfig > index ef0259a..fe6028e 100644 > --- a/drivers/usb/gadget/udc/Kconfig > +++ b/drivers/usb/gadget/udc/Kconfig > @@ -440,6 +440,17 @@ config USB_GADGET_XILINX > dynamically linked module called "udc-xilinx" and force all > gadget drivers to also be dynamically linked. > > +config USB_TEGRA_XUDC > + tristate "NVIDIA Tegra Superspeed USB 3.0 Device Controller" > + depends on ARCH_TEGRA I need at least a COMPILE_TEST here. -- balbi
Re: [PATCH v5 2/6] usb: gadget: u_serial: reimplement console support
Hi, Michał Mirosław writes: > Rewrite console support to fix a few shortcomings of the old code > preventing its use with multiple ports. This removes some duplicated > code and replaces a custom kthread with simpler workqueue item. > > Only port ttyGS0 gets to be a console for now. > > Signed-off-by: Michał Mirosław > Reviewed-by: Greg Kroah-Hartman > Tested-by: Ladislav Michl checking file drivers/usb/gadget/function/u_serial.c Hunk #7 FAILED at 1206. Hunk #8 succeeded at 1302 (offset -2 lines). Hunk #9 succeeded at 1334 (offset -2 lines). Hunk #10 succeeded at 1363 (offset -2 lines). 1 out of 10 hunks FAILED Could you rebase on my testing/next? -- balbi
AW: AW: AW: KASAN: use-after-free Read in usbhid_power
Hi again, >> >> Hey, i did not want to trigger an eartquake in the basement of the kernel ;-) >> My intention was to prevent some crashes, and help developers to find their >> bugs. >> I think my patch exactly does this. > > Hehe, actually drivers not being able to block unbind has been bugging me > for > a while now, because there are cases where this would be really helpful. >>> 1) make resources refcounted, have child resources take a ref on the parent >>> 2) Disallow unbind on devices with bound child-devices? >>> >> Exactly what i was thinking of in first attempts. >> But i fear that would break even more use cases. >> >> Hans, directly regarding the driver: >> The problem i see is that the xhci_intel_unregister_pdev which is added >> as an action with devm_add_action_or_reset() is called late by the framework, >> later than the usb_hcd_pci_remove() in xhci_pci_remove. >> Is there any chance to trigger this before? >> This is what Greg meant with "right order". > > Ah, I missed that part, sure that should be easy, just stop using > devm_add_action_or_reset() and do the xhci_intel_unregister_pdev() > manually at the right time. The downside of this is that you also > need to make sure it happens at the right time from probe error-paths > but given the bug you are hitting, I guess that is probably > already a problem. > @Hans: Sure, will have a look at this. I think i have found where to do that, but need to check how to get the pdev pointer there @Greg: I am still confident that my patch in __release_region should be taken in. Situation now without my patch: If we have a device driver (or whatever) releasing a resource, the owner of the child will have no notification that the parent is gone. Accessing the parent (at least this will happen when trying to free the resource) might have changed memory at the parent location, and what happens might be an access to unmapped memory, whatever - an oops and we don't know why. That's what i experienced and hunting. Situation with the patch applied: The owner gets a notification (parent=NULL) and we have an indication in the kernel log. If an owner of the resource where the parent is gone checks for the parent, we are fine. If he doesn't check: we have a NULL pointer deref with a warning message pointing to the root cause. Isn't it better to have a pointer to a crash rather than having unreliable racy crashes in such a case? Have a nice weekend. Best regards Carsten
Re: AW: AW: KASAN: use-after-free Read in usbhid_power
On Fri, Aug 09, 2019 at 12:38:35PM +, Schmid, Carsten wrote: > Hi again, > > >> > >> Hey, i did not want to trigger an eartquake in the basement of the kernel > >> ;-) > >> My intention was to prevent some crashes, and help developers to find > >> their bugs. > >> I think my patch exactly does this. > > > > Hehe, actually drivers not being able to block unbind has been bugging me > > for > > a while now, because there are cases where this would be really helpful. > >>> 1) make resources refcounted, have child resources take a ref on the > >>> parent > >>> 2) Disallow unbind on devices with bound child-devices? > >>> > >> Exactly what i was thinking of in first attempts. > >> But i fear that would break even more use cases. > >> > >> Hans, directly regarding the driver: > >> The problem i see is that the xhci_intel_unregister_pdev which is added > >> as an action with devm_add_action_or_reset() is called late by the > >> framework, > >> later than the usb_hcd_pci_remove() in xhci_pci_remove. > >> Is there any chance to trigger this before? > >> This is what Greg meant with "right order". > > > > Ah, I missed that part, sure that should be easy, just stop using > > devm_add_action_or_reset() and do the xhci_intel_unregister_pdev() > > manually at the right time. The downside of this is that you also > > need to make sure it happens at the right time from probe error-paths > > but given the bug you are hitting, I guess that is probably > > already a problem. > > > @Hans: > Sure, will have a look at this. I think i have found where to do that, > but need to check how to get the pdev pointer there > > @Greg: > I am still confident that my patch in __release_region should be taken in. Ok, submit it in a "real" way and we can consider it :) thanks, greg k-h
AW: AW: AW: KASAN: use-after-free Read in usbhid_power
>> >> @Greg: >> I am still confident that my patch in __release_region should be taken in. > > Ok, submit it in a "real" way and we can consider it :) > > thanks, > > greg k-h Already done, linux-ker...@vger.kernel.org, see https://www.spinics.net/lists/kernel/msg3218180.html Thanks, and have a nice weekend. Best regards Carsten
WARNING in em28xx_usb_disconnect
Hello, syzbot found the following crash on: HEAD commit:e96407b4 usb-fuzzer: main usb gadget fuzzer driver git tree: https://github.com/google/kasan.git usb-fuzzer console output: https://syzkaller.appspot.com/x/log.txt?x=15ef3cba60 kernel config: https://syzkaller.appspot.com/x/.config?x=cfa2c18fb6a8068e dashboard link: https://syzkaller.appspot.com/bug?extid=b7f57261c521087d89bb compiler: gcc (GCC) 9.0.0 20181231 (experimental) syz repro: https://syzkaller.appspot.com/x/repro.syz?x=16ceae3660 C reproducer: https://syzkaller.appspot.com/x/repro.c?x=12816f2660 IMPORTANT: if you fix the bug, please add the following tag to the commit: Reported-by: syzbot+b7f57261c521087d8...@syzkaller.appspotmail.com usb 1-1: USB disconnect, device number 2 em28xx 1-1:1.153: Disconnecting em28xx #1 [ cut here ] WARNING: CPU: 0 PID: 12 at kernel/workqueue.c:3031 __flush_work.cold+0x2c/0x36 kernel/workqueue.c:3031 Kernel panic - not syncing: panic_on_warn set ... CPU: 0 PID: 12 Comm: kworker/0:1 Not tainted 5.3.0-rc2+ #25 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 Workqueue: usb_hub_wq hub_event Call Trace: __dump_stack lib/dump_stack.c:77 [inline] dump_stack+0xca/0x13e lib/dump_stack.c:113 panic+0x2a3/0x6da kernel/panic.c:219 __warn.cold+0x20/0x4a kernel/panic.c:576 report_bug+0x262/0x2a0 lib/bug.c:186 fixup_bug arch/x86/kernel/traps.c:179 [inline] fixup_bug arch/x86/kernel/traps.c:174 [inline] do_error_trap+0x12b/0x1e0 arch/x86/kernel/traps.c:272 do_invalid_op+0x32/0x40 arch/x86/kernel/traps.c:291 invalid_op+0x23/0x30 arch/x86/entry/entry_64.S:1026 RIP: 0010:__flush_work.cold+0x2c/0x36 kernel/workqueue.c:3031 Code: 9a 22 00 48 c7 c7 20 e4 c5 85 e8 d9 3a 0d 00 0f 0b 45 31 e4 e9 98 86 ff ff e8 51 9a 22 00 48 c7 c7 20 e4 c5 85 e8 be 3a 0d 00 <0f> 0b 45 31 e4 e9 7d 86 ff ff e8 36 9a 22 00 48 c7 c7 20 e4 c5 85 RSP: 0018:8881da20f720 EFLAGS: 00010286 RAX: 0024 RBX: dc00 RCX: RDX: RSI: 8128a0fd RDI: ed103b441ed6 RBP: 8881da20f888 R08: 0024 R09: fbfff11acd9a R10: fbfff11acd99 R11: 88d66ccf R12: R13: 0001 R14: 8881c6685df8 R15: 8881d2a85b78 flush_request_modules drivers/media/usb/em28xx/em28xx-cards.c:3325 [inline] em28xx_usb_disconnect.cold+0x280/0x2a6 drivers/media/usb/em28xx/em28xx-cards.c:4023 usb_unbind_interface+0x1bd/0x8a0 drivers/usb/core/driver.c:423 __device_release_driver drivers/base/dd.c:1120 [inline] device_release_driver_internal+0x404/0x4c0 drivers/base/dd.c:1151 bus_remove_device+0x2dc/0x4a0 drivers/base/bus.c:556 device_del+0x420/0xb10 drivers/base/core.c:2288 usb_disable_device+0x211/0x690 drivers/usb/core/message.c:1237 usb_disconnect+0x284/0x8d0 drivers/usb/core/hub.c:2199 hub_port_connect drivers/usb/core/hub.c:4949 [inline] hub_port_connect_change drivers/usb/core/hub.c:5213 [inline] port_event drivers/usb/core/hub.c:5359 [inline] hub_event+0x1454/0x3640 drivers/usb/core/hub.c:5441 process_one_work+0x92b/0x1530 kernel/workqueue.c:2269 process_scheduled_works kernel/workqueue.c:2331 [inline] worker_thread+0x7ab/0xe20 kernel/workqueue.c:2417 kthread+0x318/0x420 kernel/kthread.c:255 ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352 Kernel Offset: disabled Rebooting in 86400 seconds.. --- This bug is generated by a bot. It may contain errors. See https://goo.gl/tpsmEJ for more information about syzbot. syzbot engineers can be reached at syzkal...@googlegroups.com. syzbot will keep track of this bug report. See: https://goo.gl/tpsmEJ#status for how to communicate with syzbot. syzbot can test patches for this bug, for details see: https://goo.gl/tpsmEJ#testing-patches
INFO: rcu detected stall in dummy_timer
Hello, syzbot found the following crash on: HEAD commit:e96407b4 usb-fuzzer: main usb gadget fuzzer driver git tree: https://github.com/google/kasan.git usb-fuzzer console output: https://syzkaller.appspot.com/x/log.txt?x=102b8c4a60 kernel config: https://syzkaller.appspot.com/x/.config?x=cfa2c18fb6a8068e dashboard link: https://syzkaller.appspot.com/bug?extid=b24d736f18a1541ad550 compiler: gcc (GCC) 9.0.0 20181231 (experimental) syz repro: https://syzkaller.appspot.com/x/repro.syz?x=12edd63660 IMPORTANT: if you fix the bug, please add the following tag to the commit: Reported-by: syzbot+b24d736f18a1541ad...@syzkaller.appspotmail.com imon 5-1:0.0: imon usb_rx_callback_intf0: status(-71): ignored imon 1-1:0.0: imon usb_rx_callback_intf0: status(-71): ignored imon 2-1:0.0: imon usb_rx_callback_intf0: status(-71): ignored imon 5-1:0.0: imon usb_rx_callback_intf0: status(-71): ignored imon 2-1:0.0: imon usb_rx_callback_intf0: status(-71): ignored rcu: INFO: rcu_sched self-detected stall on CPU rcu: 0-...!: (1 GPs behind) idle=36e/1/0x4004 softirq=16230/16230 fqs=0 (t=10500 jiffies g=22005 q=99) rcu: rcu_sched kthread starved for 10500 jiffies! g22005 f0x0 RCU_GP_WAIT_FQS(5) ->state=0x0 ->cpu=1 rcu: RCU grace-period kthread stack dump: rcu_sched R running task2942410 2 0x80004000 Call Trace: schedule+0x9a/0x250 kernel/sched/core.c:3944 schedule_timeout+0x440/0xb20 kernel/time/timer.c:1807 rcu_gp_fqs_loop kernel/rcu/tree.c:1611 [inline] rcu_gp_kthread+0xb01/0x27f0 kernel/rcu/tree.c:1768 kthread+0x318/0x420 kernel/kthread.c:255 ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352 NMI backtrace for cpu 0 CPU: 0 PID: 2795 Comm: kworker/0:4 Not tainted 5.3.0-rc2+ #25 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 Workqueue: usb_hub_wq hub_event Call Trace: __dump_stack lib/dump_stack.c:77 [inline] dump_stack+0xca/0x13e lib/dump_stack.c:113 nmi_cpu_backtrace.cold+0x55/0x96 lib/nmi_backtrace.c:101 nmi_trigger_cpumask_backtrace+0x1b0/0x1c7 lib/nmi_backtrace.c:62 trigger_single_cpu_backtrace include/linux/nmi.h:164 [inline] rcu_dump_cpu_stacks+0x169/0x1b3 kernel/rcu/tree_stall.h:254 print_cpu_stall kernel/rcu/tree_stall.h:455 [inline] check_cpu_stall kernel/rcu/tree_stall.h:529 [inline] rcu_pending kernel/rcu/tree.c:2736 [inline] rcu_sched_clock_irq.cold+0x4a4/0x8d8 kernel/rcu/tree.c:2183 update_process_times+0x2a/0x70 kernel/time/timer.c:1639 tick_sched_handle+0x9b/0x180 kernel/time/tick-sched.c:167 tick_sched_timer+0x42/0x130 kernel/time/tick-sched.c:1296 __run_hrtimer kernel/time/hrtimer.c:1389 [inline] __hrtimer_run_queues+0x303/0xc50 kernel/time/hrtimer.c:1451 hrtimer_interrupt+0x2e8/0x730 kernel/time/hrtimer.c:1509 local_apic_timer_interrupt arch/x86/kernel/apic/apic.c:1068 [inline] smp_apic_timer_interrupt+0xf5/0x500 arch/x86/kernel/apic/apic.c:1093 apic_timer_interrupt+0xf/0x20 arch/x86/entry/entry_64.S:828 RIP: 0010:arch_local_irq_restore arch/x86/include/asm/irqflags.h:85 [inline] RIP: 0010:__raw_spin_unlock_irqrestore include/linux/spinlock_api_smp.h:160 [inline] RIP: 0010:_raw_spin_unlock_irqrestore+0x40/0x50 kernel/locking/spinlock.c:191 Code: e8 a5 d6 b7 fb 48 89 ef e8 4d b7 b8 fb f6 c7 02 75 11 53 9d e8 21 6b d5 fb 65 ff 0d 82 3f 94 7a 5b 5d c3 e8 32 69 d5 fb 53 9d ed 0f 1f 40 00 66 2e 0f 1f 84 00 00 00 00 00 55 48 89 fd 65 ff RSP: 0018:8881db209b08 EFLAGS: 0206 ORIG_RAX: ff13 RAX: 0007 RBX: 0206 RCX: 0002 RDX: RSI: 0008 RDI: 8881c7023844 RBP: 8881da16c200 R08: 8881c7023000 R09: fbfff11acda2 R10: fbfff11acda1 R11: 88d66d0f R12: 0080 R13: R14: dc00 R15: 8881c9263d00 spin_unlock_irqrestore include/linux/spinlock.h:393 [inline] dummy_timer+0x131b/0x2fa2 drivers/usb/gadget/udc/dummy_hcd.c:1979 call_timer_fn+0x179/0x650 kernel/time/timer.c:1322 expire_timers kernel/time/timer.c:1366 [inline] __run_timers kernel/time/timer.c:1685 [inline] __run_timers kernel/time/timer.c:1653 [inline] run_timer_softirq+0x5cc/0x14b0 kernel/time/timer.c:1698 __do_softirq+0x221/0x912 kernel/softirq.c:292 invoke_softirq kernel/softirq.c:373 [inline] irq_exit+0x178/0x1a0 kernel/softirq.c:413 exiting_irq arch/x86/include/asm/apic.h:537 [inline] smp_apic_timer_interrupt+0x12f/0x500 arch/x86/kernel/apic/apic.c:1095 apic_timer_interrupt+0xf/0x20 arch/x86/entry/entry_64.S:828 RIP: 0010:__kernfs_new_node+0x164/0x640 fs/kernfs/dir.c:639 Code: 00 00 00 4c 89 e6 4c 89 ff 89 44 24 08 e8 a4 fd df 03 31 ff 89 c5 89 c6 e8 a9 30 b6 ff 85 ed 0f 88 5a 03 00 00 e8 2c 2f b6 ff <49> 8d 7d 60 48 b8 00 00 00 00 00 fc ff df 48 89 fa 48 c1 ea 03 0f RSP: 0018:8881c169ede8 EFLAGS: 0293 ORIG_RAX: ff13 RAX: 8881c7023000 RBX: 1110382d3dc0 RCX: 8187bf97 RDX: RSI:
KASAN: use-after-free Read in ld_usb_release
Hello, syzbot found the following crash on: HEAD commit:e96407b4 usb-fuzzer: main usb gadget fuzzer driver git tree: https://github.com/google/kasan.git usb-fuzzer console output: https://syzkaller.appspot.com/x/log.txt?x=15a16f2660 kernel config: https://syzkaller.appspot.com/x/.config?x=cfa2c18fb6a8068e dashboard link: https://syzkaller.appspot.com/bug?extid=30cf45ebfe0b0c4847a1 compiler: gcc (GCC) 9.0.0 20181231 (experimental) syz repro: https://syzkaller.appspot.com/x/repro.syz?x=1416df2660 C reproducer: https://syzkaller.appspot.com/x/repro.c?x=11ce511c60 IMPORTANT: if you fix the bug, please add the following tag to the commit: Reported-by: syzbot+30cf45ebfe0b0c484...@syzkaller.appspotmail.com == BUG: KASAN: use-after-free in __mutex_lock_common kernel/locking/mutex.c:912 [inline] BUG: KASAN: use-after-free in __mutex_lock+0xf23/0x1360 kernel/locking/mutex.c:1077 Read of size 8 at addr 8881d21fc2d8 by task syz-executor834/1878 CPU: 0 PID: 1878 Comm: syz-executor834 Not tainted 5.3.0-rc2+ #25 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 Call Trace: __dump_stack lib/dump_stack.c:77 [inline] dump_stack+0xca/0x13e lib/dump_stack.c:113 print_address_description+0x6a/0x32c mm/kasan/report.c:351 __kasan_report.cold+0x1a/0x33 mm/kasan/report.c:482 kasan_report+0xe/0x12 mm/kasan/common.c:612 __mutex_lock_common kernel/locking/mutex.c:912 [inline] __mutex_lock+0xf23/0x1360 kernel/locking/mutex.c:1077 ld_usb_release+0xb1/0x400 drivers/usb/misc/ldusb.c:386 __fput+0x2d7/0x840 fs/file_table.c:280 task_work_run+0x13f/0x1c0 kernel/task_work.c:113 tracehook_notify_resume include/linux/tracehook.h:188 [inline] exit_to_usermode_loop+0x1d2/0x200 arch/x86/entry/common.c:163 prepare_exit_to_usermode arch/x86/entry/common.c:194 [inline] syscall_return_slowpath arch/x86/entry/common.c:274 [inline] do_syscall_64+0x45f/0x580 arch/x86/entry/common.c:299 entry_SYSCALL_64_after_hwframe+0x49/0xbe RIP: 0033:0x406b31 Code: 75 14 b8 03 00 00 00 0f 05 48 3d 01 f0 ff ff 0f 83 04 19 00 00 c3 48 83 ec 08 e8 6a fc ff ff 48 89 04 24 b8 03 00 00 00 0f 05 <48> 8b 3c 24 48 89 c2 e8 b3 fc ff ff 48 89 d0 48 83 c4 08 48 3d 01 RSP: 002b:7ffcf13bd080 EFLAGS: 0293 ORIG_RAX: 0003 RAX: RBX: 0005 RCX: 00406b31 RDX: fff7 RSI: 0080 RDI: 0004 RBP: 0159 R08: 0020 R09: 0020 R10: 7ffcf13bd0b0 R11: 0293 R12: 0001d884 R13: 0004 R14: 006e39ec R15: 0064 Allocated by task 1775: save_stack+0x1b/0x80 mm/kasan/common.c:69 set_track mm/kasan/common.c:77 [inline] __kasan_kmalloc mm/kasan/common.c:487 [inline] __kasan_kmalloc.constprop.0+0xbf/0xd0 mm/kasan/common.c:460 kmalloc include/linux/slab.h:552 [inline] kzalloc include/linux/slab.h:748 [inline] ld_usb_probe+0x6e/0xa65 drivers/usb/misc/ldusb.c:661 usb_probe_interface+0x305/0x7a0 drivers/usb/core/driver.c:361 really_probe+0x281/0x650 drivers/base/dd.c:548 driver_probe_device+0x101/0x1b0 drivers/base/dd.c:709 __device_attach_driver+0x1c2/0x220 drivers/base/dd.c:816 bus_for_each_drv+0x15c/0x1e0 drivers/base/bus.c:454 __device_attach+0x217/0x360 drivers/base/dd.c:882 bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514 device_add+0xae6/0x16f0 drivers/base/core.c:2114 usb_set_configuration+0xdf6/0x1670 drivers/usb/core/message.c:2023 generic_probe+0x9d/0xd5 drivers/usb/core/generic.c:210 usb_probe_device+0x99/0x100 drivers/usb/core/driver.c:266 really_probe+0x281/0x650 drivers/base/dd.c:548 driver_probe_device+0x101/0x1b0 drivers/base/dd.c:709 __device_attach_driver+0x1c2/0x220 drivers/base/dd.c:816 bus_for_each_drv+0x15c/0x1e0 drivers/base/bus.c:454 __device_attach+0x217/0x360 drivers/base/dd.c:882 bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514 device_add+0xae6/0x16f0 drivers/base/core.c:2114 usb_new_device.cold+0x6a4/0xe79 drivers/usb/core/hub.c:2536 hub_port_connect drivers/usb/core/hub.c:5098 [inline] hub_port_connect_change drivers/usb/core/hub.c:5213 [inline] port_event drivers/usb/core/hub.c:5359 [inline] hub_event+0x1b5c/0x3640 drivers/usb/core/hub.c:5441 process_one_work+0x92b/0x1530 kernel/workqueue.c:2269 worker_thread+0x96/0xe20 kernel/workqueue.c:2415 kthread+0x318/0x420 kernel/kthread.c:255 ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352 Freed by task 1775: save_stack+0x1b/0x80 mm/kasan/common.c:69 set_track mm/kasan/common.c:77 [inline] __kasan_slab_free+0x130/0x180 mm/kasan/common.c:449 slab_free_hook mm/slub.c:1423 [inline] slab_free_freelist_hook mm/slub.c:1470 [inline] slab_free mm/slub.c:3012 [inline] kfree+0xe4/0x2f0 mm/slub.c:3953 ld_usb_probe+0x728/0xa65 drivers/usb/misc/ldusb.c:744 usb_probe_interface+0x305/0x7a0 drivers/usb/core/driver.c:361 really_probe+0x281/0x650 drivers/
possible deadlock in display_open
Hello,
syzbot found the following crash on:
HEAD commit:e96407b4 usb-fuzzer: main usb gadget fuzzer driver
git tree: https://github.com/google/kasan.git usb-fuzzer
console output: https://syzkaller.appspot.com/x/log.txt?x=13b29b2660
kernel config: https://syzkaller.appspot.com/x/.config?x=cfa2c18fb6a8068e
dashboard link: https://syzkaller.appspot.com/bug?extid=c558267ad910fc494497
compiler: gcc (GCC) 9.0.0 20181231 (experimental)
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=1542700260
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=111cb61c60
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+c558267ad910fc494...@syzkaller.appspotmail.com
==
WARNING: possible circular locking dependency detected
5.3.0-rc2+ #25 Not tainted
--
syz-executor626/1723 is trying to acquire lock:
1a0d74d7 (driver_lock#2){+.+.}, at: display_open+0x1f/0x1d0
drivers/media/rc/imon.c:475
but task is already holding lock:
076a0058 (minor_rwsem){}, at: usb_open+0x23/0x270
drivers/usb/core/file.c:39
which lock already depends on the new lock.
the existing dependency chain (in reverse order) is:
-> #2 (minor_rwsem){}:
down_write+0x92/0x150 kernel/locking/rwsem.c:1500
usb_register_dev drivers/usb/core/file.c:187 [inline]
usb_register_dev+0x131/0x6a0 drivers/usb/core/file.c:156
imon_init_display drivers/media/rc/imon.c:2343 [inline]
imon_probe+0x244d/0x2af0 drivers/media/rc/imon.c:2426
usb_probe_interface+0x305/0x7a0 drivers/usb/core/driver.c:361
really_probe+0x281/0x650 drivers/base/dd.c:548
driver_probe_device+0x101/0x1b0 drivers/base/dd.c:709
__device_attach_driver+0x1c2/0x220 drivers/base/dd.c:816
bus_for_each_drv+0x15c/0x1e0 drivers/base/bus.c:454
__device_attach+0x217/0x360 drivers/base/dd.c:882
bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514
device_add+0xae6/0x16f0 drivers/base/core.c:2114
usb_set_configuration+0xdf6/0x1670 drivers/usb/core/message.c:2023
generic_probe+0x9d/0xd5 drivers/usb/core/generic.c:210
usb_probe_device+0x99/0x100 drivers/usb/core/driver.c:266
really_probe+0x281/0x650 drivers/base/dd.c:548
driver_probe_device+0x101/0x1b0 drivers/base/dd.c:709
__device_attach_driver+0x1c2/0x220 drivers/base/dd.c:816
bus_for_each_drv+0x15c/0x1e0 drivers/base/bus.c:454
__device_attach+0x217/0x360 drivers/base/dd.c:882
bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514
device_add+0xae6/0x16f0 drivers/base/core.c:2114
usb_new_device.cold+0x6a4/0xe79 drivers/usb/core/hub.c:2536
hub_port_connect drivers/usb/core/hub.c:5098 [inline]
hub_port_connect_change drivers/usb/core/hub.c:5213 [inline]
port_event drivers/usb/core/hub.c:5359 [inline]
hub_event+0x1b5c/0x3640 drivers/usb/core/hub.c:5441
process_one_work+0x92b/0x1530 kernel/workqueue.c:2269
worker_thread+0x96/0xe20 kernel/workqueue.c:2415
kthread+0x318/0x420 kernel/kthread.c:255
ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352
-> #1 (&ictx->lock){+.+.}:
__mutex_lock_common kernel/locking/mutex.c:930 [inline]
__mutex_lock+0x158/0x1360 kernel/locking/mutex.c:1077
imon_init_intf0 drivers/media/rc/imon.c:2188 [inline]
imon_probe+0xf0c/0x2af0 drivers/media/rc/imon.c:2387
usb_probe_interface+0x305/0x7a0 drivers/usb/core/driver.c:361
really_probe+0x281/0x650 drivers/base/dd.c:548
driver_probe_device+0x101/0x1b0 drivers/base/dd.c:709
__device_attach_driver+0x1c2/0x220 drivers/base/dd.c:816
bus_for_each_drv+0x15c/0x1e0 drivers/base/bus.c:454
__device_attach+0x217/0x360 drivers/base/dd.c:882
bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514
device_add+0xae6/0x16f0 drivers/base/core.c:2114
usb_set_configuration+0xdf6/0x1670 drivers/usb/core/message.c:2023
generic_probe+0x9d/0xd5 drivers/usb/core/generic.c:210
usb_probe_device+0x99/0x100 drivers/usb/core/driver.c:266
really_probe+0x281/0x650 drivers/base/dd.c:548
driver_probe_device+0x101/0x1b0 drivers/base/dd.c:709
__device_attach_driver+0x1c2/0x220 drivers/base/dd.c:816
bus_for_each_drv+0x15c/0x1e0 drivers/base/bus.c:454
__device_attach+0x217/0x360 drivers/base/dd.c:882
bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514
device_add+0xae6/0x16f0 drivers/base/core.c:2114
usb_new_device.cold+0x6a4/0xe79 drivers/usb/core/hub.c:2536
hub_port_connect drivers/usb/core/hub.c:5098 [inline]
hub_port_connect_change drivers/usb/core/hub.c:5213 [inline]
port_event drivers/usb/core/hub.c:5359 [inline]
hub_event+0x1b5c/0x3640 drivers/usb/core/hub.c:5441
process_one_work+0x92b/0x1530 kernel/workqu
KASAN: use-after-free Read in prepare_to_wait_event
Hello, syzbot found the following crash on: HEAD commit:e96407b4 usb-fuzzer: main usb gadget fuzzer driver git tree: https://github.com/google/kasan.git usb-fuzzer console output: https://syzkaller.appspot.com/x/log.txt?x=10fbde8c60 kernel config: https://syzkaller.appspot.com/x/.config?x=cfa2c18fb6a8068e dashboard link: https://syzkaller.appspot.com/bug?extid=332cbcbd8be3e03c62eb compiler: gcc (GCC) 9.0.0 20181231 (experimental) syz repro: https://syzkaller.appspot.com/x/repro.syz?x=127dd63660 C reproducer: https://syzkaller.appspot.com/x/repro.c?x=171de9ce60 IMPORTANT: if you fix the bug, please add the following tag to the commit: Reported-by: syzbot+332cbcbd8be3e03c6...@syzkaller.appspotmail.com == BUG: KASAN: use-after-free in __lock_acquire+0x302a/0x3b50 kernel/locking/lockdep.c:3753 Read of size 8 at addr 8881d2a83238 by task syz-executor771/2979 CPU: 0 PID: 2979 Comm: syz-executor771 Not tainted 5.3.0-rc2+ #25 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 Call Trace: __dump_stack lib/dump_stack.c:77 [inline] dump_stack+0xca/0x13e lib/dump_stack.c:113 print_address_description+0x6a/0x32c mm/kasan/report.c:351 __kasan_report.cold+0x1a/0x33 mm/kasan/report.c:482 kasan_report+0xe/0x12 mm/kasan/common.c:612 __lock_acquire+0x302a/0x3b50 kernel/locking/lockdep.c:3753 lock_acquire+0x127/0x320 kernel/locking/lockdep.c:4412 __raw_spin_lock_irqsave include/linux/spinlock_api_smp.h:110 [inline] _raw_spin_lock_irqsave+0x32/0x50 kernel/locking/spinlock.c:159 prepare_to_wait_event+0x5b/0x650 kernel/sched/wait.c:263 ld_usb_read+0x619/0x780 drivers/usb/misc/ldusb.c:480 __vfs_read+0x76/0x100 fs/read_write.c:425 vfs_read+0x1ea/0x430 fs/read_write.c:461 ksys_read+0x1e8/0x250 fs/read_write.c:587 do_syscall_64+0xb7/0x580 arch/x86/entry/common.c:296 entry_SYSCALL_64_after_hwframe+0x49/0xbe RIP: 0033:0x448859 Code: e8 ac e7 ff ff 48 83 c4 18 c3 0f 1f 80 00 00 00 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 0f 83 4b 06 fc ff c3 66 2e 0f 1f 84 00 00 00 00 RSP: 002b:7f9b31740ce8 EFLAGS: 0246 ORIG_RAX: RAX: ffda RBX: 006dec48 RCX: 00448859 RDX: 0049 RSI: 2080 RDI: 0004 RBP: 006dec40 R08: R09: R10: R11: 0246 R12: 006dec4c R13: 7ffd58e161df R14: 7f9b317419c0 R15: 006dec4c Allocated by task 2705: save_stack+0x1b/0x80 mm/kasan/common.c:69 set_track mm/kasan/common.c:77 [inline] __kasan_kmalloc mm/kasan/common.c:487 [inline] __kasan_kmalloc.constprop.0+0xbf/0xd0 mm/kasan/common.c:460 kmalloc include/linux/slab.h:552 [inline] kzalloc include/linux/slab.h:748 [inline] ld_usb_probe+0x6e/0xa65 drivers/usb/misc/ldusb.c:661 usb_probe_interface+0x305/0x7a0 drivers/usb/core/driver.c:361 really_probe+0x281/0x650 drivers/base/dd.c:548 driver_probe_device+0x101/0x1b0 drivers/base/dd.c:709 __device_attach_driver+0x1c2/0x220 drivers/base/dd.c:816 bus_for_each_drv+0x15c/0x1e0 drivers/base/bus.c:454 __device_attach+0x217/0x360 drivers/base/dd.c:882 bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514 device_add+0xae6/0x16f0 drivers/base/core.c:2114 usb_set_configuration+0xdf6/0x1670 drivers/usb/core/message.c:2023 generic_probe+0x9d/0xd5 drivers/usb/core/generic.c:210 usb_probe_device+0x99/0x100 drivers/usb/core/driver.c:266 really_probe+0x281/0x650 drivers/base/dd.c:548 driver_probe_device+0x101/0x1b0 drivers/base/dd.c:709 __device_attach_driver+0x1c2/0x220 drivers/base/dd.c:816 bus_for_each_drv+0x15c/0x1e0 drivers/base/bus.c:454 __device_attach+0x217/0x360 drivers/base/dd.c:882 bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514 device_add+0xae6/0x16f0 drivers/base/core.c:2114 usb_new_device.cold+0x6a4/0xe79 drivers/usb/core/hub.c:2536 hub_port_connect drivers/usb/core/hub.c:5098 [inline] hub_port_connect_change drivers/usb/core/hub.c:5213 [inline] port_event drivers/usb/core/hub.c:5359 [inline] hub_event+0x1b5c/0x3640 drivers/usb/core/hub.c:5441 process_one_work+0x92b/0x1530 kernel/workqueue.c:2269 worker_thread+0x96/0xe20 kernel/workqueue.c:2415 kthread+0x318/0x420 kernel/kthread.c:255 ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352 Freed by task 2705: save_stack+0x1b/0x80 mm/kasan/common.c:69 set_track mm/kasan/common.c:77 [inline] __kasan_slab_free+0x130/0x180 mm/kasan/common.c:449 slab_free_hook mm/slub.c:1423 [inline] slab_free_freelist_hook mm/slub.c:1470 [inline] slab_free mm/slub.c:3012 [inline] kfree+0xe4/0x2f0 mm/slub.c:3953 ld_usb_probe+0x728/0xa65 drivers/usb/misc/ldusb.c:744 usb_probe_interface+0x305/0x7a0 drivers/usb/core/driver.c:361 really_probe+0x281/0x650 drivers/base/dd.c:548 driver_probe_device+0x101/0x1b0 drivers/base/dd.c:709 __device_attach_driver+0x1c2/0x22
Re: AW: AW: KASAN: use-after-free Read in usbhid_power
On Fri, Aug 09, 2019 at 01:00:25PM +, Schmid, Carsten wrote: > >> > >> @Greg: > >> I am still confident that my patch in __release_region should be taken in. > > > > Ok, submit it in a "real" way and we can consider it :) > > > > thanks, > > > > greg k-h > > Already done, linux-ker...@vger.kernel.org, see > https://www.spinics.net/lists/kernel/msg3218180.html You didn't cc: any developer, that's a sure way to get a patch ignored :( Try resending it with at least the people who get_maintainer.pl says has touched that file last in it. Also, Linus is the unofficial resource.c maintainer. I think he has a set of userspace testing scripts for changes somewhere, so you should cc: him too. And might as well add me :) thanks, greg k-h
Re: [PATCH 05/14] gpio: lpc32xx: allow building on non-lpc32xx targets
On Mon, Aug 5, 2019 at 10:28 AM Bartosz Golaszewski
wrote:
>
> pt., 2 sie 2019 o 13:20 Arnd Bergmann napisał(a):
> >
> > On Fri, Aug 2, 2019 at 9:10 AM Bartosz Golaszewski
> > wrote:
> > > > -#include
> > > > -#include
> > > > +#define _GPREG(x) (x)
> > >
> > > What purpose does this macro serve?
> > >
> > > >
> > > > #define LPC32XX_GPIO_P3_INP_STATE _GPREG(0x000)
> > > > #define LPC32XX_GPIO_P3_OUTP_SET _GPREG(0x004)
> >
> > In the existing code base, this macro converts a register offset to
> > an __iomem pointer for a gpio register. I changed the definition of the
> > macro here to keep the number of changes down, but I it's just
> > as easy to remove it if you prefer.
>
> Could you just add a comment so that it's clear at first glance?
I ended up removing the macro. With the change to keep the reg_base as
a struct member, this ends up being a relatively small change, and it's
more straightforward that way.
> > > > @@ -167,14 +166,26 @@ struct lpc32xx_gpio_chip {
> > > > struct gpio_regs*gpio_grp;
> > > > };
> > > >
> > > > +void __iomem *gpio_reg_base;
> > >
> > > Any reason why this can't be made part of struct lpc32xx_gpio_chip?
> >
> > It could be, but it's the same for each instance, and not known until
> > probe() time, so the same pointer would need to be copied into each
> > instance that is otherwise read-only.
> >
> > Let me know if you'd prefer me to rework these two things or leave
> > them as they are.
>
> I would prefer not to have global state in the driver, let's just
> store the pointer in the data passed to gpiochip_add_data().
Ok, done.
Arnd
Re: [PATCH 05/14] gpio: lpc32xx: allow building on non-lpc32xx targets
On Tue, Aug 6, 2019 at 10:02 PM Sylvain Lemieux wrote: > > > > + gpio_reg_base = devm_platform_ioremap_resource(pdev, 0); > > + if (gpio_reg_base) > > + return -ENXIO; > > The probe function will always return an error. > Please replace the previous 2 lines with: > if (IS_ERR(gpio_reg_base)) > return PTR_ERR(gpio_reg_base); > > You can add my acked-by and tested-by in the v2 patch. > Acked-by: Sylvain Lemieux > Tested-by: Sylvain Lemieux Ok, fixed now, along with addressing Bartosz' concerns. Arnd
Re: [Patch V6 7/8] usb: gadget: Add UDC driver for tegra XUSB device mode controller
Hi Nagarjuna,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on linus/master]
[cannot apply to v5.3-rc3 next-20190809]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Nagarjuna-Kristam/Tegra-XUSB-gadget-driver-support/20190809-042626
config: arm-allmodconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (GCC) 7.4.0
reproduce:
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.4.0 make.cross ARCH=arm
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot
All warnings (new ones prefixed by >>):
In file included from include/linux/printk.h:332:0,
from include/linux/kernel.h:15,
from include/linux/clk.h:13,
from drivers/usb/gadget/udc/tegra_xudc.c:9:
drivers/usb/gadget/udc/tegra_xudc.c: In function
'tegra_xudc_ep0_standard_req':
>> include/linux/dynamic_debug.h:122:52: warning: this statement may fall
>> through [-Wimplicit-fallthrough=]
#define __dynamic_func_call(id, fmt, func, ...) do { \
^
include/linux/dynamic_debug.h:143:2: note: in expansion of macro
'__dynamic_func_call'
__dynamic_func_call(__UNIQUE_ID(ddebug), fmt, func, ##__VA_ARGS__)
^~~
include/linux/dynamic_debug.h:157:2: note: in expansion of macro
'_dynamic_func_call'
_dynamic_func_call(fmt,__dynamic_dev_dbg, \
^~
include/linux/device.h:1509:2: note: in expansion of macro 'dynamic_dev_dbg'
dynamic_dev_dbg(dev, dev_fmt(fmt), ##__VA_ARGS__)
^~~
>> drivers/usb/gadget/udc/tegra_xudc.c:2394:3: note: in expansion of macro
>> 'dev_dbg'
dev_dbg(xudc->dev, "USB_REQ_SET_CONFIGURATION\n");
^~~
drivers/usb/gadget/udc/tegra_xudc.c:2400:2: note: here
default:
^~~
--
In file included from include/linux/printk.h:332:0,
from include/linux/kernel.h:15,
from include/linux/clk.h:13,
from drivers/usb//gadget/udc/tegra_xudc.c:9:
drivers/usb//gadget/udc/tegra_xudc.c: In function
'tegra_xudc_ep0_standard_req':
>> include/linux/dynamic_debug.h:122:52: warning: this statement may fall
>> through [-Wimplicit-fallthrough=]
#define __dynamic_func_call(id, fmt, func, ...) do { \
^
include/linux/dynamic_debug.h:143:2: note: in expansion of macro
'__dynamic_func_call'
__dynamic_func_call(__UNIQUE_ID(ddebug), fmt, func, ##__VA_ARGS__)
^~~
include/linux/dynamic_debug.h:157:2: note: in expansion of macro
'_dynamic_func_call'
_dynamic_func_call(fmt,__dynamic_dev_dbg, \
^~
include/linux/device.h:1509:2: note: in expansion of macro 'dynamic_dev_dbg'
dynamic_dev_dbg(dev, dev_fmt(fmt), ##__VA_ARGS__)
^~~
drivers/usb//gadget/udc/tegra_xudc.c:2394:3: note: in expansion of macro
'dev_dbg'
dev_dbg(xudc->dev, "USB_REQ_SET_CONFIGURATION\n");
^~~
drivers/usb//gadget/udc/tegra_xudc.c:2400:2: note: here
default:
^~~
vim +/dev_dbg +2394 drivers/usb/gadget/udc/tegra_xudc.c
2365
2366 static int tegra_xudc_ep0_standard_req(struct tegra_xudc *xudc,
2367struct usb_ctrlrequest *ctrl)
2368 {
2369 int ret;
2370
2371 switch (ctrl->bRequest) {
2372 case USB_REQ_GET_STATUS:
2373 dev_dbg(xudc->dev, "USB_REQ_GET_STATUS\n");
2374 ret = tegra_xudc_ep0_get_status(xudc, ctrl);
2375 break;
2376 case USB_REQ_SET_ADDRESS:
2377 dev_dbg(xudc->dev, "USB_REQ_SET_ADDRESS\n");
2378 ret = tegra_xudc_ep0_set_address(xudc, ctrl);
2379 break;
2380 case USB_REQ_SET_SEL:
2381 dev_dbg(xudc->dev, "USB_REQ_SET_SEL\n");
2382 ret = tegra_xudc_ep0_set_sel(xudc, ctrl);
2383 break;
2384 case USB_REQ_SET_ISOCH_DELAY:
2385 dev_dbg(xudc->dev, "USB_REQ_SET_ISOCH_DELAY\n");
2386 ret = tegra_xudc_ep0_set_isoch_delay(xudc, ctrl);
2387 break;
2388 case USB_REQ_CLEAR_FEATURE:
2389 case USB_REQ_SET_FEATURE:
2390 dev_dbg(xudc->dev, "USB_REQ_CLEAR/SET_FEATURE\n");
2391
[PATCH v2 00/13] v2: ARM: move lpc32xx to multiplatform
Version 2 contains some minor changes based on earlier feedback
and from the 0day build bot testing on other architectures. The
only patch that changed significantly is the one for the gpio driver.
I would suggest we merge this version into the soc tree directly
if there are no further concerns.
Arnd
Arnd Bergmann (12):
usb: ohci-nxp: enable compile-testing
usb: udc: lpc32xx: allow compile-testing
watchdog: pnx4008_wdt: allow compile-testing
serial: lpc32xx_hs: allow compile-testing
gpio: lpc32xx: allow building on non-lpc32xx targets
net: lpc-enet: factor out iram access
net: lpc-enet: move phy setup into platform code
net: lpc-enet: fix printk format strings
net: lpc-enet: allow compile testing
serial: lpc32xx: allow compile testing
ARM: lpc32xx: clean up header files
ARM: lpc32xx: allow multiplatform build
kbuild test robot (1):
net: lpc-enet: fix badzero.cocci warnings
arch/arm/Kconfig | 17 +--
arch/arm/configs/lpc32xx_defconfig| 2 +
arch/arm/mach-lpc32xx/Kconfig | 11 ++
arch/arm/mach-lpc32xx/common.c| 24 +++-
arch/arm/mach-lpc32xx/common.h| 1 -
arch/arm/mach-lpc32xx/include/mach/board.h| 15 ---
.../mach-lpc32xx/include/mach/entry-macro.S | 28 -
arch/arm/mach-lpc32xx/include/mach/hardware.h | 25
.../mach-lpc32xx/include/mach/uncompress.h| 50
.../{include/mach/platform.h => lpc32xx.h}| 18 ++-
arch/arm/mach-lpc32xx/pm.c| 3 +-
arch/arm/mach-lpc32xx/serial.c| 33 -
arch/arm/mach-lpc32xx/suspend.S | 3 +-
drivers/gpio/Kconfig | 7 ++
drivers/gpio/Makefile | 2 +-
drivers/gpio/gpio-lpc32xx.c | 118 ++
drivers/net/ethernet/nxp/Kconfig | 2 +-
drivers/net/ethernet/nxp/lpc_eth.c| 45 +++
drivers/tty/serial/Kconfig| 3 +-
drivers/tty/serial/lpc32xx_hs.c | 37 +-
drivers/usb/gadget/udc/Kconfig| 3 +-
drivers/usb/gadget/udc/lpc32xx_udc.c | 3 +-
drivers/usb/host/Kconfig | 3 +-
drivers/usb/host/ohci-nxp.c | 25 ++--
drivers/watchdog/Kconfig | 2 +-
drivers/watchdog/pnx4008_wdt.c| 1 -
include/linux/soc/nxp/lpc32xx-misc.h | 33 +
27 files changed, 242 insertions(+), 272 deletions(-)
create mode 100644 arch/arm/mach-lpc32xx/Kconfig
delete mode 100644 arch/arm/mach-lpc32xx/include/mach/board.h
delete mode 100644 arch/arm/mach-lpc32xx/include/mach/entry-macro.S
delete mode 100644 arch/arm/mach-lpc32xx/include/mach/hardware.h
delete mode 100644 arch/arm/mach-lpc32xx/include/mach/uncompress.h
rename arch/arm/mach-lpc32xx/{include/mach/platform.h => lpc32xx.h} (98%)
create mode 100644 include/linux/soc/nxp/lpc32xx-misc.h
--
2.20.0
Cc: s...@kernel.org
Cc: linux-arm-ker...@lists.infradead.org
Cc: Vladimir Zapolskiy
Cc: Sylvain Lemieux
Cc: Linus Walleij
Cc: "David S. Miller"
Cc: Greg Kroah-Hartman
Cc: Alan Stern
Cc: Guenter Roeck
Cc: linux-g...@vger.kernel.org
Cc: net...@vger.kernel.org
Cc: linux-ser...@vger.kernel.org
Cc: linux-usb@vger.kernel.org
Cc: linux-watch...@vger.kernel.org
[PATCH v2 01/13] usb: ohci-nxp: enable compile-testing
The driver hardcodes a hardware I/O address the way one should
generally not do, and this prevents both compile-testing, and
moving the platform to CONFIG_ARCH_MULTIPLATFORM.
Change the code to be independent of the machine headers
to allow those two. Removing the hardcoded address would
be hard and is not necessary, so leave that in place for now.
Signed-off-by: Arnd Bergmann
---
drivers/usb/host/Kconfig| 3 ++-
drivers/usb/host/ohci-nxp.c | 25 ++---
2 files changed, 20 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index 40b5de597112..73d233d3bf4d 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -441,7 +441,8 @@ config USB_OHCI_HCD_S3C2410
config USB_OHCI_HCD_LPC32XX
tristate "Support for LPC on-chip OHCI USB controller"
- depends on USB_OHCI_HCD && ARCH_LPC32XX
+ depends on USB_OHCI_HCD
+ depends on ARCH_LPC32XX || COMPILE_TEST
depends on USB_ISP1301
default y
---help---
diff --git a/drivers/usb/host/ohci-nxp.c b/drivers/usb/host/ohci-nxp.c
index f5f532601092..c561881d0e79 100644
--- a/drivers/usb/host/ohci-nxp.c
+++ b/drivers/usb/host/ohci-nxp.c
@@ -29,10 +29,7 @@
#include "ohci.h"
-#include
-
#define USB_CONFIG_BASE0x3102
-#define USB_OTG_STAT_CONTROL IO_ADDRESS(USB_CONFIG_BASE + 0x110)
/* USB_OTG_STAT_CONTROL bit defines */
#define TRANSPARENT_I2C_EN (1 << 7)
@@ -122,19 +119,33 @@ static inline void isp1301_vbus_off(void)
static void ohci_nxp_start_hc(void)
{
- unsigned long tmp = __raw_readl(USB_OTG_STAT_CONTROL) | HOST_EN;
+ void __iomem *usb_otg_stat_control = ioremap(USB_CONFIG_BASE + 0x110,
4);
+ unsigned long tmp;
+
+ if (WARN_ON(!usb_otg_stat_control))
+ return;
+
+ tmp = __raw_readl(usb_otg_stat_control) | HOST_EN;
- __raw_writel(tmp, USB_OTG_STAT_CONTROL);
+ __raw_writel(tmp, usb_otg_stat_control);
isp1301_vbus_on();
+
+ iounmap(usb_otg_stat_control);
}
static void ohci_nxp_stop_hc(void)
{
+ void __iomem *usb_otg_stat_control = ioremap(USB_CONFIG_BASE + 0x110,
4);
unsigned long tmp;
+ if (WARN_ON(!usb_otg_stat_control))
+ return;
+
isp1301_vbus_off();
- tmp = __raw_readl(USB_OTG_STAT_CONTROL) & ~HOST_EN;
- __raw_writel(tmp, USB_OTG_STAT_CONTROL);
+ tmp = __raw_readl(usb_otg_stat_control) & ~HOST_EN;
+ __raw_writel(tmp, usb_otg_stat_control);
+
+ iounmap(usb_otg_stat_control);
}
static int ohci_hcd_nxp_probe(struct platform_device *pdev)
--
2.20.0
[PATCH v2 02/13] usb: udc: lpc32xx: allow compile-testing
The only thing that prevents building this driver on other platforms is the mach/hardware.h include, which is not actually used here at all, so remove the line and allow CONFIG_COMPILE_TEST. Acked-by: Greg Kroah-Hartman Acked-by: Sylvain Lemieux Signed-off-by: Arnd Bergmann --- drivers/usb/gadget/udc/Kconfig | 3 ++- drivers/usb/gadget/udc/lpc32xx_udc.c | 3 +-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/usb/gadget/udc/Kconfig b/drivers/usb/gadget/udc/Kconfig index ef0259a950ba..d354036ff6c8 100644 --- a/drivers/usb/gadget/udc/Kconfig +++ b/drivers/usb/gadget/udc/Kconfig @@ -45,7 +45,8 @@ config USB_AT91 config USB_LPC32XX tristate "LPC32XX USB Peripheral Controller" - depends on ARCH_LPC32XX && I2C + depends on ARCH_LPC32XX || COMPILE_TEST + depends on I2C select USB_ISP1301 help This option selects the USB device controller in the LPC32xx SoC. diff --git a/drivers/usb/gadget/udc/lpc32xx_udc.c b/drivers/usb/gadget/udc/lpc32xx_udc.c index 5f1b14f3e5a0..defe04d52e6d 100644 --- a/drivers/usb/gadget/udc/lpc32xx_udc.c +++ b/drivers/usb/gadget/udc/lpc32xx_udc.c @@ -24,6 +24,7 @@ #include #include #include +#include #include #include #include @@ -35,8 +36,6 @@ #include #endif -#include - /* * USB device configuration structure */ -- 2.20.0
Re: BUG: bad usercopy in ld_usb_read
On Fri, 9 Aug 2019, Greg KH wrote:
> On Thu, Aug 08, 2019 at 04:06:32PM -0700, Kees Cook wrote:
> > On Thu, Aug 08, 2019 at 02:46:54PM +0200, Greg KH wrote:
> > > On Thu, Aug 08, 2019 at 05:38:06AM -0700, syzbot wrote:
> > > > Hello,
> > > >
> > > > syzbot found the following crash on:
> > > >
> > > > HEAD commit:e96407b4 usb-fuzzer: main usb gadget fuzzer driver
> > > > git tree: https://github.com/google/kasan.git usb-fuzzer
> > > > console output: https://syzkaller.appspot.com/x/log.txt?x=13aeaece60
> > > > kernel config:
> > > > https://syzkaller.appspot.com/x/.config?x=cfa2c18fb6a8068e
> > > > dashboard link:
> > > > https://syzkaller.appspot.com/bug?extid=45b2f40f0778cfa7634e
> > > > compiler: gcc (GCC) 9.0.0 20181231 (experimental)
> > > >
> > > > Unfortunately, I don't have any reproducer for this crash yet.
> > > >
> > > > IMPORTANT: if you fix the bug, please add the following tag to the
> > > > commit:
> > > > Reported-by: syzbot+45b2f40f0778cfa76...@syzkaller.appspotmail.com
> > > >
> > > > ldusb 6-1:0.124: Read buffer overflow, -131383996186150 bytes dropped
> > >
> > > That's a funny number :)
> > >
> > > Nice overflow found, I see you are now starting to fuzz the char device
> > > nodes of usb drivers...
> > >
> > > Michael, care to fix this up?
> >
> > This looks like the length in the read-from-device buffer is unchecked:
> >
> > /* actual_buffer contains actual_length + interrupt_in_buffer */
> > actual_buffer = (size_t *)(dev->ring_buffer + dev->ring_tail *
> > (sizeof(size_t)+dev->interrupt_in_endpoint_size));
> > bytes_to_read = min(count, *actual_buffer);
> > if (bytes_to_read < *actual_buffer)
> > dev_warn(&dev->intf->dev, "Read buffer overflow, %zd bytes
> > dropped\n",
> > *actual_buffer-bytes_to_read);
> >
> > /* copy one interrupt_in_buffer from ring_buffer into userspace */
> > if (copy_to_user(buffer, actual_buffer+1, bytes_to_read)) {
> > retval = -EFAULT;
> > goto unlock_exit;
> > }
> >
> > I assume what's stored at actual_buffer is bogus and needs validation
> > somewhere before it's actually used. (If not here, maybe where ever the
> > write into the buffer originally happens?)
>
> I think it should be verified here, as that's when it is parsed. The
> data is written to the buffer in ld_usb_interrupt_in_callback() but it
> does not "know" how to parse it at that location.
I looked at this bug report, and it is very puzzling.
Yes, the value stored in *actual_buffer is written in
ld_usb_interrupt_in_callback(), but the value is simply
urb->actual_length and therefore does not need any validation. The
URB's transfer_buffer_length is taken from
dev->interrupt_in_endpoint_size, which comes from usb_endpoint_maxp()
and therefore cannot be larger than 2048.
(On the other hand, actual_buffer might not be aligned on a 32-bit
address. For x86, of course, this doesn't matter, but it can affect
other architectures.)
Furthermore, the computation leading to the dev_warn() involves only
size_t types and therefore is carried out entirely using unsigned
arithmetic. The warning's format string uses %zd instead of %zu;
that's why the number showed up as negative but doesn't explain why it
looks so funny.
In fact, I don't see why any of the computations here should overflow
or wrap around, or even give rise to a negative value. If syzbot had a
reproducer we could get more debugging output -- but it doesn't.
Alan Stern
Re: KASAN: use-after-free Read in ld_usb_release
Greg: See below... On Fri, 9 Aug 2019, syzbot wrote: > Hello, > > syzbot found the following crash on: > > HEAD commit:e96407b4 usb-fuzzer: main usb gadget fuzzer driver > git tree: https://github.com/google/kasan.git usb-fuzzer > console output: https://syzkaller.appspot.com/x/log.txt?x=15a16f2660 > kernel config: https://syzkaller.appspot.com/x/.config?x=cfa2c18fb6a8068e > dashboard link: https://syzkaller.appspot.com/bug?extid=30cf45ebfe0b0c4847a1 > compiler: gcc (GCC) 9.0.0 20181231 (experimental) > syz repro: https://syzkaller.appspot.com/x/repro.syz?x=1416df2660 > C reproducer: https://syzkaller.appspot.com/x/repro.c?x=11ce511c60 > > IMPORTANT: if you fix the bug, please add the following tag to the commit: > Reported-by: syzbot+30cf45ebfe0b0c484...@syzkaller.appspotmail.com > > == > BUG: KASAN: use-after-free in __mutex_lock_common > kernel/locking/mutex.c:912 [inline] > BUG: KASAN: use-after-free in __mutex_lock+0xf23/0x1360 > kernel/locking/mutex.c:1077 > Read of size 8 at addr 8881d21fc2d8 by task syz-executor834/1878 > > CPU: 0 PID: 1878 Comm: syz-executor834 Not tainted 5.3.0-rc2+ #25 > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS > Google 01/01/2011 > Call Trace: > __dump_stack lib/dump_stack.c:77 [inline] > dump_stack+0xca/0x13e lib/dump_stack.c:113 > print_address_description+0x6a/0x32c mm/kasan/report.c:351 > __kasan_report.cold+0x1a/0x33 mm/kasan/report.c:482 > kasan_report+0xe/0x12 mm/kasan/common.c:612 > __mutex_lock_common kernel/locking/mutex.c:912 [inline] > __mutex_lock+0xf23/0x1360 kernel/locking/mutex.c:1077 > ld_usb_release+0xb1/0x400 drivers/usb/misc/ldusb.c:386 > __fput+0x2d7/0x840 fs/file_table.c:280 > task_work_run+0x13f/0x1c0 kernel/task_work.c:113 > tracehook_notify_resume include/linux/tracehook.h:188 [inline] > exit_to_usermode_loop+0x1d2/0x200 arch/x86/entry/common.c:163 > prepare_exit_to_usermode arch/x86/entry/common.c:194 [inline] > syscall_return_slowpath arch/x86/entry/common.c:274 [inline] > do_syscall_64+0x45f/0x580 arch/x86/entry/common.c:299 > entry_SYSCALL_64_after_hwframe+0x49/0xbe > RIP: 0033:0x406b31 > Code: 75 14 b8 03 00 00 00 0f 05 48 3d 01 f0 ff ff 0f 83 04 19 00 00 c3 48 > 83 ec 08 e8 6a fc ff ff 48 89 04 24 b8 03 00 00 00 0f 05 <48> 8b 3c 24 48 > 89 c2 e8 b3 fc ff ff 48 89 d0 48 83 c4 08 48 3d 01 > RSP: 002b:7ffcf13bd080 EFLAGS: 0293 ORIG_RAX: 0003 > RAX: RBX: 0005 RCX: 00406b31 > RDX: fff7 RSI: 0080 RDI: 0004 > RBP: 0159 R08: 0020 R09: 0020 > R10: 7ffcf13bd0b0 R11: 0293 R12: 0001d884 > R13: 0004 R14: 006e39ec R15: 0064 > > Allocated by task 1775: > save_stack+0x1b/0x80 mm/kasan/common.c:69 > set_track mm/kasan/common.c:77 [inline] > __kasan_kmalloc mm/kasan/common.c:487 [inline] > __kasan_kmalloc.constprop.0+0xbf/0xd0 mm/kasan/common.c:460 > kmalloc include/linux/slab.h:552 [inline] > kzalloc include/linux/slab.h:748 [inline] > ld_usb_probe+0x6e/0xa65 drivers/usb/misc/ldusb.c:661 > usb_probe_interface+0x305/0x7a0 drivers/usb/core/driver.c:361 > really_probe+0x281/0x650 drivers/base/dd.c:548 > driver_probe_device+0x101/0x1b0 drivers/base/dd.c:709 > __device_attach_driver+0x1c2/0x220 drivers/base/dd.c:816 > bus_for_each_drv+0x15c/0x1e0 drivers/base/bus.c:454 > __device_attach+0x217/0x360 drivers/base/dd.c:882 > bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514 > device_add+0xae6/0x16f0 drivers/base/core.c:2114 > usb_set_configuration+0xdf6/0x1670 drivers/usb/core/message.c:2023 > generic_probe+0x9d/0xd5 drivers/usb/core/generic.c:210 > usb_probe_device+0x99/0x100 drivers/usb/core/driver.c:266 > really_probe+0x281/0x650 drivers/base/dd.c:548 > driver_probe_device+0x101/0x1b0 drivers/base/dd.c:709 > __device_attach_driver+0x1c2/0x220 drivers/base/dd.c:816 > bus_for_each_drv+0x15c/0x1e0 drivers/base/bus.c:454 > __device_attach+0x217/0x360 drivers/base/dd.c:882 > bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514 > device_add+0xae6/0x16f0 drivers/base/core.c:2114 > usb_new_device.cold+0x6a4/0xe79 drivers/usb/core/hub.c:2536 > hub_port_connect drivers/usb/core/hub.c:5098 [inline] > hub_port_connect_change drivers/usb/core/hub.c:5213 [inline] > port_event drivers/usb/core/hub.c:5359 [inline] > hub_event+0x1b5c/0x3640 drivers/usb/core/hub.c:5441 > process_one_work+0x92b/0x1530 kernel/workqueue.c:2269 > worker_thread+0x96/0xe20 kernel/workqueue.c:2415 > kthread+0x318/0x420 kernel/kthread.c:255 > ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352 > > Freed by task 1775: > save_stack+0x1b/0x80 mm/kasan/common.c:69 > set_track mm/kasan/common.c:77 [inline] > __kasan_slab_free+0x130/0x180 mm/kasan/common.c:449 > slab_free_ho
Re: [PATCH] usb: host: xhci-rcar: Fix timeout in xhci_suspend()
On Fri, Aug 02, 2019 at 05:33:35PM +0900, Yoshihiro Shimoda wrote:
> When a USB device is connected to the host controller and
> the system enters suspend, the following error happens
> in xhci_suspend():
>
> xhci-hcd ee00.usb: WARN: xHC CMD_RUN timeout
>
> Since the firmware/internal CPU control the USBSTS.STS_HALT
> and the process speed is down when the roothub port enters U3,
> long delay for the handshake of STS_HALT is neeed in xhci_suspend().
> So, this patch adds to set the XHCI_SLOW_SUSPEND.
>
> Fixes: 435cc1138ec9 ("usb: host: xhci-plat: set resume_quirk() for R-Car
> controllers")
> Cc: # v4.12+
> Signed-off-by: Yoshihiro Shimoda
Reviewed-by: Simon Horman
> ---
> drivers/usb/host/xhci-rcar.c | 9 +++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/host/xhci-rcar.c b/drivers/usb/host/xhci-rcar.c
> index 671bce1..8616c52 100644
> --- a/drivers/usb/host/xhci-rcar.c
> +++ b/drivers/usb/host/xhci-rcar.c
> @@ -238,10 +238,15 @@ int xhci_rcar_init_quirk(struct usb_hcd *hcd)
>* pointers. So, this driver clears the AC64 bit of xhci->hcc_params
>* to call dma_set_coherent_mask(dev, DMA_BIT_MASK(32)) in
>* xhci_gen_setup().
> + *
> + * And, since the firmware/internal CPU control the USBSTS.STS_HALT
> + * and the process speed is down when the roothub port enters U3,
> + * long delay for the handshake of STS_HALT is neeed in xhci_suspend().
>*/
> if (xhci_rcar_is_gen2(hcd->self.controller) ||
> - xhci_rcar_is_gen3(hcd->self.controller))
> - xhci->quirks |= XHCI_NO_64BIT_SUPPORT;
> + xhci_rcar_is_gen3(hcd->self.controller)) {
> + xhci->quirks |= XHCI_NO_64BIT_SUPPORT | XHCI_SLOW_SUSPEND;
> + }
nit: As there is still only one line guarded by the conditional I don't
think that { } need to be added.
>
> if (!xhci_rcar_wait_for_pll_active(hcd))
> return -ETIMEDOUT;
> --
> 2.7.4
>
Re: KASAN: use-after-free Read in ld_usb_release
On Fri, Aug 9, 2019 at 6:51 PM Alan Stern wrote: > > Greg: > > See below... > > On Fri, 9 Aug 2019, syzbot wrote: > > > Hello, > > > > syzbot found the following crash on: > > > > HEAD commit:e96407b4 usb-fuzzer: main usb gadget fuzzer driver > > git tree: https://github.com/google/kasan.git usb-fuzzer > > console output: https://syzkaller.appspot.com/x/log.txt?x=15a16f2660 > > kernel config: https://syzkaller.appspot.com/x/.config?x=cfa2c18fb6a8068e > > dashboard link: https://syzkaller.appspot.com/bug?extid=30cf45ebfe0b0c4847a1 > > compiler: gcc (GCC) 9.0.0 20181231 (experimental) > > syz repro: https://syzkaller.appspot.com/x/repro.syz?x=1416df2660 > > C reproducer: https://syzkaller.appspot.com/x/repro.c?x=11ce511c60 > > > > IMPORTANT: if you fix the bug, please add the following tag to the commit: > > Reported-by: syzbot+30cf45ebfe0b0c484...@syzkaller.appspotmail.com > > > > == > > BUG: KASAN: use-after-free in __mutex_lock_common > > kernel/locking/mutex.c:912 [inline] > > BUG: KASAN: use-after-free in __mutex_lock+0xf23/0x1360 > > kernel/locking/mutex.c:1077 > > Read of size 8 at addr 8881d21fc2d8 by task syz-executor834/1878 > > > > CPU: 0 PID: 1878 Comm: syz-executor834 Not tainted 5.3.0-rc2+ #25 > > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS > > Google 01/01/2011 > > Call Trace: > > __dump_stack lib/dump_stack.c:77 [inline] > > dump_stack+0xca/0x13e lib/dump_stack.c:113 > > print_address_description+0x6a/0x32c mm/kasan/report.c:351 > > __kasan_report.cold+0x1a/0x33 mm/kasan/report.c:482 > > kasan_report+0xe/0x12 mm/kasan/common.c:612 > > __mutex_lock_common kernel/locking/mutex.c:912 [inline] > > __mutex_lock+0xf23/0x1360 kernel/locking/mutex.c:1077 > > ld_usb_release+0xb1/0x400 drivers/usb/misc/ldusb.c:386 > > __fput+0x2d7/0x840 fs/file_table.c:280 > > task_work_run+0x13f/0x1c0 kernel/task_work.c:113 > > tracehook_notify_resume include/linux/tracehook.h:188 [inline] > > exit_to_usermode_loop+0x1d2/0x200 arch/x86/entry/common.c:163 > > prepare_exit_to_usermode arch/x86/entry/common.c:194 [inline] > > syscall_return_slowpath arch/x86/entry/common.c:274 [inline] > > do_syscall_64+0x45f/0x580 arch/x86/entry/common.c:299 > > entry_SYSCALL_64_after_hwframe+0x49/0xbe > > RIP: 0033:0x406b31 > > Code: 75 14 b8 03 00 00 00 0f 05 48 3d 01 f0 ff ff 0f 83 04 19 00 00 c3 48 > > 83 ec 08 e8 6a fc ff ff 48 89 04 24 b8 03 00 00 00 0f 05 <48> 8b 3c 24 48 > > 89 c2 e8 b3 fc ff ff 48 89 d0 48 83 c4 08 48 3d 01 > > RSP: 002b:7ffcf13bd080 EFLAGS: 0293 ORIG_RAX: 0003 > > RAX: RBX: 0005 RCX: 00406b31 > > RDX: fff7 RSI: 0080 RDI: 0004 > > RBP: 0159 R08: 0020 R09: 0020 > > R10: 7ffcf13bd0b0 R11: 0293 R12: 0001d884 > > R13: 0004 R14: 006e39ec R15: 0064 > > > > Allocated by task 1775: > > save_stack+0x1b/0x80 mm/kasan/common.c:69 > > set_track mm/kasan/common.c:77 [inline] > > __kasan_kmalloc mm/kasan/common.c:487 [inline] > > __kasan_kmalloc.constprop.0+0xbf/0xd0 mm/kasan/common.c:460 > > kmalloc include/linux/slab.h:552 [inline] > > kzalloc include/linux/slab.h:748 [inline] > > ld_usb_probe+0x6e/0xa65 drivers/usb/misc/ldusb.c:661 > > usb_probe_interface+0x305/0x7a0 drivers/usb/core/driver.c:361 > > really_probe+0x281/0x650 drivers/base/dd.c:548 > > driver_probe_device+0x101/0x1b0 drivers/base/dd.c:709 > > __device_attach_driver+0x1c2/0x220 drivers/base/dd.c:816 > > bus_for_each_drv+0x15c/0x1e0 drivers/base/bus.c:454 > > __device_attach+0x217/0x360 drivers/base/dd.c:882 > > bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514 > > device_add+0xae6/0x16f0 drivers/base/core.c:2114 > > usb_set_configuration+0xdf6/0x1670 drivers/usb/core/message.c:2023 > > generic_probe+0x9d/0xd5 drivers/usb/core/generic.c:210 > > usb_probe_device+0x99/0x100 drivers/usb/core/driver.c:266 > > really_probe+0x281/0x650 drivers/base/dd.c:548 > > driver_probe_device+0x101/0x1b0 drivers/base/dd.c:709 > > __device_attach_driver+0x1c2/0x220 drivers/base/dd.c:816 > > bus_for_each_drv+0x15c/0x1e0 drivers/base/bus.c:454 > > __device_attach+0x217/0x360 drivers/base/dd.c:882 > > bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514 > > device_add+0xae6/0x16f0 drivers/base/core.c:2114 > > usb_new_device.cold+0x6a4/0xe79 drivers/usb/core/hub.c:2536 > > hub_port_connect drivers/usb/core/hub.c:5098 [inline] > > hub_port_connect_change drivers/usb/core/hub.c:5213 [inline] > > port_event drivers/usb/core/hub.c:5359 [inline] > > hub_event+0x1b5c/0x3640 drivers/usb/core/hub.c:5441 > > process_one_work+0x92b/0x1530 kernel/workqueue.c:2269 > > worker_thread+0x96/0xe20 kernel/workqueue.c:2415 > > kthread+0x318/0x420 kernel/kthread.c:255 > > ret_from_fork+0x24
[PATCH] USB: storage: isd200: remove redundant assignment to variable sendToTransport
From: Colin Ian King
The variable sendToTransport is being initialized with a value that is
never read and is being re-assigned a little later on. The assignment
is redundant and hence can be removed.
Addresses-Coverity: ("Unused value")
Signed-off-by: Colin Ian King
---
drivers/usb/storage/isd200.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/storage/isd200.c b/drivers/usb/storage/isd200.c
index 2b474d60b4db..28e1128d53a4 100644
--- a/drivers/usb/storage/isd200.c
+++ b/drivers/usb/storage/isd200.c
@@ -1511,7 +1511,7 @@ static int isd200_Initialization(struct us_data *us)
static void isd200_ata_command(struct scsi_cmnd *srb, struct us_data *us)
{
- int sendToTransport = 1, orig_bufflen;
+ int sendToTransport, orig_bufflen;
union ata_cdb ataCdb;
/* Make sure driver was initialized */
--
2.20.1
Re: KASAN: use-after-free Read in usbhid_power
On Thu, Aug 8, 2019 at 9:37 PM Alan Stern wrote:
>
> On Thu, 8 Aug 2019, Andrey Konovalov wrote:
>
> > On Thu, Jul 25, 2019 at 5:09 PM Alan Stern
> > wrote:
> > >
> > > On Thu, 25 Jul 2019, Oliver Neukum wrote:
> > >
> > > > Am Mittwoch, den 24.07.2019, 17:02 -0400 schrieb Alan Stern:
> > > > > On Wed, 24 Jul 2019, Oliver Neukum wrote:
> > > > >
> > > > > > drivers/hid/usbhid/hid-core.c | 13 +
> > > > > > 1 file changed, 13 insertions(+)
> > > > > >
> > > > > > diff --git a/drivers/hid/usbhid/hid-core.c
> > > > > > b/drivers/hid/usbhid/hid-core.c
> > > > > > index c7bc9db5b192..98b996ecf4d3 100644
> > > > > > --- a/drivers/hid/usbhid/hid-core.c
> > > > > > +++ b/drivers/hid/usbhid/hid-core.c
> > > > > > @@ -1229,6 +1229,17 @@ static int usbhid_power(struct hid_device
> > > > > > *hid, int lvl)
> > > > > > struct usbhid_device *usbhid = hid->driver_data;
> > > > > > int r = 0;
> > > > > >
> > > > > > + spin_lock_irq(&usbhid->lock);
> > > > > > + if (test_bit(HID_DISCONNECTED, &usbhid->iofl)) {
> > > > > > + r = -ENODEV;
> > > > > > + spin_unlock_irq(&usbhid->lock);
> > > > > > + goto bail_out;
> > > > > > + } else {
> > > > > > + /* protect against disconnect */
> > > > > > + usb_get_dev(interface_to_usbdev(usbhid->intf));
> > > > > > + spin_unlock_irq(&usbhid->lock);
> > > > > > + }
> > > > > > +
> > > > > > switch (lvl) {
> > > > > > case PM_HINT_FULLON:
> > > > > > r = usb_autopm_get_interface(usbhid->intf);
> > > > > > @@ -1238,7 +1249,9 @@ static int usbhid_power(struct hid_device
> > > > > > *hid, int lvl)
> > > > > > usb_autopm_put_interface(usbhid->intf);
> > > > > > break;
> > > > > > }
> > > > > > + usb_put_dev(interface_to_usbdev(usbhid->intf));
> > > > > >
> > > > > > +bail_out:
> > > > > > return r;
> > > > > > }
>
> > This report looks like very similar to these two:
> >
> > https://syzkaller.appspot.com/bug?extid=b156665cf4d1b5e00c76
> > https://syzkaller.appspot.com/bug?extid=3cbe5cd105d2ad56a1df
>
> It also seems to resemble extids a7a6b9c609b9457c62c6,
> 62a1e04fd3ec2abf099e, and 75e6910bf03e266a277f, although this may be an
> illusion.
>
> > Maybe we should mark those two as duplicates.
> >
> > Hillf suggested a fix on one of them, but it looks different from what
> > you propose:
> >
> > https://groups.google.com/d/msg/syzkaller-bugs/xW7LvKfpyn0/SpKbs5ZLEAAJ
>
> Go ahead and try it out on all of them. I don't have a clear feeling
> about it, not having worked on usbhid in quite a while.
>
> Alan Stern
>
Let's try on this one first:
#syz test: https://github.com/google/kasan.git 6a3599ce
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -1410,6 +1410,7 @@ static void usbhid_disconnect(struct usb
spin_lock_irq(&usbhid->lock); /* Sync with error and led handlers */
set_bit(HID_DISCONNECTED, &usbhid->iofl);
spin_unlock_irq(&usbhid->lock);
+ hid_hw_stop(hid);
hid_destroy_device(hid);
kfree(usbhid);
}
KASAN: use-after-free Read in usb_kill_urb
Hello, syzbot found the following crash on: HEAD commit:e96407b4 usb-fuzzer: main usb gadget fuzzer driver git tree: https://github.com/google/kasan.git usb-fuzzer console output: https://syzkaller.appspot.com/x/log.txt?x=1799392c60 kernel config: https://syzkaller.appspot.com/x/.config?x=cfa2c18fb6a8068e dashboard link: https://syzkaller.appspot.com/bug?extid=22ae4e3b9fcc8a5c153a compiler: gcc (GCC) 9.0.0 20181231 (experimental) syz repro: https://syzkaller.appspot.com/x/repro.syz?x=1134c80260 C reproducer: https://syzkaller.appspot.com/x/repro.c?x=13278c4a60 IMPORTANT: if you fix the bug, please add the following tag to the commit: Reported-by: syzbot+22ae4e3b9fcc8a5c1...@syzkaller.appspotmail.com == BUG: KASAN: use-after-free in atomic_read include/asm-generic/atomic-instrumented.h:26 [inline] BUG: KASAN: use-after-free in usb_kill_urb drivers/usb/core/urb.c:695 [inline] BUG: KASAN: use-after-free in usb_kill_urb+0x24b/0x2c0 drivers/usb/core/urb.c:687 Read of size 4 at addr 8881d635b110 by task syz-executor672/1999 CPU: 1 PID: 1999 Comm: syz-executor672 Not tainted 5.3.0-rc2+ #25 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 Call Trace: __dump_stack lib/dump_stack.c:77 [inline] dump_stack+0xca/0x13e lib/dump_stack.c:113 print_address_description+0x6a/0x32c mm/kasan/report.c:351 __kasan_report.cold+0x1a/0x33 mm/kasan/report.c:482 kasan_report+0xe/0x12 mm/kasan/common.c:612 check_memory_region_inline mm/kasan/generic.c:185 [inline] check_memory_region+0x128/0x190 mm/kasan/generic.c:192 atomic_read include/asm-generic/atomic-instrumented.h:26 [inline] usb_kill_urb drivers/usb/core/urb.c:695 [inline] usb_kill_urb+0x24b/0x2c0 drivers/usb/core/urb.c:687 ld_usb_abort_transfers+0xb7/0x1d0 drivers/usb/misc/ldusb.c:196 ld_usb_release+0x19f/0x400 drivers/usb/misc/ldusb.c:406 __fput+0x2d7/0x840 fs/file_table.c:280 task_work_run+0x13f/0x1c0 kernel/task_work.c:113 exit_task_work include/linux/task_work.h:22 [inline] do_exit+0x8ef/0x2c50 kernel/exit.c:878 do_group_exit+0x125/0x340 kernel/exit.c:982 __do_sys_exit_group kernel/exit.c:993 [inline] __se_sys_exit_group kernel/exit.c:991 [inline] __x64_sys_exit_group+0x3a/0x50 kernel/exit.c:991 do_syscall_64+0xb7/0x580 arch/x86/entry/common.c:296 entry_SYSCALL_64_after_hwframe+0x49/0xbe RIP: 0033:0x440418 Code: 00 00 be 3c 00 00 00 eb 19 66 0f 1f 84 00 00 00 00 00 48 89 d7 89 f0 0f 05 48 3d 00 f0 ff ff 77 21 f4 48 89 d7 44 89 c0 0f 05 <48> 3d 00 f0 ff ff 76 e0 f7 d8 64 41 89 01 eb d8 0f 1f 84 00 00 00 RSP: 002b:7ffe884abf88 EFLAGS: 0246 ORIG_RAX: 00e7 RAX: ffda RBX: RCX: 00440418 RDX: RSI: 003c RDI: RBP: 004bff50 R08: 00e7 R09: ffd0 R10: 0064 R11: 0246 R12: 0001 R13: 006d2180 R14: R15: Allocated by task 22: save_stack+0x1b/0x80 mm/kasan/common.c:69 set_track mm/kasan/common.c:77 [inline] __kasan_kmalloc mm/kasan/common.c:487 [inline] __kasan_kmalloc.constprop.0+0xbf/0xd0 mm/kasan/common.c:460 kmalloc include/linux/slab.h:557 [inline] usb_alloc_urb+0x65/0xb0 drivers/usb/core/urb.c:73 ld_usb_probe+0x3e1/0xa65 drivers/usb/misc/ldusb.c:708 usb_probe_interface+0x305/0x7a0 drivers/usb/core/driver.c:361 really_probe+0x281/0x650 drivers/base/dd.c:548 driver_probe_device+0x101/0x1b0 drivers/base/dd.c:709 __device_attach_driver+0x1c2/0x220 drivers/base/dd.c:816 bus_for_each_drv+0x15c/0x1e0 drivers/base/bus.c:454 __device_attach+0x217/0x360 drivers/base/dd.c:882 bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514 device_add+0xae6/0x16f0 drivers/base/core.c:2114 usb_set_configuration+0xdf6/0x1670 drivers/usb/core/message.c:2023 generic_probe+0x9d/0xd5 drivers/usb/core/generic.c:210 usb_probe_device+0x99/0x100 drivers/usb/core/driver.c:266 really_probe+0x281/0x650 drivers/base/dd.c:548 driver_probe_device+0x101/0x1b0 drivers/base/dd.c:709 __device_attach_driver+0x1c2/0x220 drivers/base/dd.c:816 bus_for_each_drv+0x15c/0x1e0 drivers/base/bus.c:454 __device_attach+0x217/0x360 drivers/base/dd.c:882 bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514 device_add+0xae6/0x16f0 drivers/base/core.c:2114 usb_new_device.cold+0x6a4/0xe79 drivers/usb/core/hub.c:2536 hub_port_connect drivers/usb/core/hub.c:5098 [inline] hub_port_connect_change drivers/usb/core/hub.c:5213 [inline] port_event drivers/usb/core/hub.c:5359 [inline] hub_event+0x1b5c/0x3640 drivers/usb/core/hub.c:5441 process_one_work+0x92b/0x1530 kernel/workqueue.c:2269 worker_thread+0x96/0xe20 kernel/workqueue.c:2415 kthread+0x318/0x420 kernel/kthread.c:255 ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352 Freed by task 22: save_stack+0x1b/0x80 mm/kasan/common.c:69 set_track mm/kasan/common.c:77 [inlin
Re: KASAN: use-after-free Read in ld_usb_release
Hello, syzbot has tested the proposed patch and the reproducer did not trigger crash: Reported-and-tested-by: syzbot+30cf45ebfe0b0c484...@syzkaller.appspotmail.com Tested on: commit: e96407b4 usb-fuzzer: main usb gadget fuzzer driver git tree: https://github.com/google/kasan.git kernel config: https://syzkaller.appspot.com/x/.config?x=cfa2c18fb6a8068e compiler: gcc (GCC) 9.0.0 20181231 (experimental) patch: https://syzkaller.appspot.com/x/patch.diff?x=15b4c80260 Note: testing is done by a robot and is best-effort only.
Re: KASAN: use-after-free Read in usbhid_power
Hello, syzbot has tested the proposed patch and the reproducer did not trigger crash: Reported-and-tested-by: syzbot+ef5de9c4f99c4edb4...@syzkaller.appspotmail.com Tested on: commit: 6a3599ce usb-fuzzer: main usb gadget fuzzer driver git tree: https://github.com/google/kasan.git kernel config: https://syzkaller.appspot.com/x/.config?x=700ca426ab83faae compiler: gcc (GCC) 9.0.0 20181231 (experimental) patch: https://syzkaller.appspot.com/x/patch.diff?x=17fcd52c60 Note: testing is done by a robot and is best-effort only.
Re: possible deadlock in usb_deregister_dev
syzbot has found a reproducer for the following crash on:
HEAD commit:e96407b4 usb-fuzzer: main usb gadget fuzzer driver
git tree: https://github.com/google/kasan.git usb-fuzzer
console output: https://syzkaller.appspot.com/x/log.txt?x=15bf780e60
kernel config: https://syzkaller.appspot.com/x/.config?x=cfa2c18fb6a8068e
dashboard link: https://syzkaller.appspot.com/bug?extid=a64a382964bf6c71a9c0
compiler: gcc (GCC) 9.0.0 20181231 (experimental)
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=1678757460
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=136cc4d260
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+a64a382964bf6c71a...@syzkaller.appspotmail.com
usb 1-1: New USB device strings: Mfr=0, Product=0, SerialNumber=0
usb 1-1: config 0 descriptor??
iowarrior 1-1:0.236: IOWarrior product=0x1501, serial= interface=236 now
attached to iowarrior0
usb 1-1: USB disconnect, device number 2
==
WARNING: possible circular locking dependency detected
5.3.0-rc2+ #25 Not tainted
--
kworker/0:1/12 is trying to acquire lock:
cd63e8f1 (minor_rwsem){}, at: usb_deregister_dev
drivers/usb/core/file.c:238 [inline]
cd63e8f1 (minor_rwsem){}, at: usb_deregister_dev+0x61/0x270
drivers/usb/core/file.c:230
but task is already holding lock:
1d1989ef (iowarrior_open_disc_lock){+.+.}, at:
iowarrior_disconnect+0x45/0x2c0 drivers/usb/misc/iowarrior.c:867
which lock already depends on the new lock.
the existing dependency chain (in reverse order) is:
-> #2 (iowarrior_open_disc_lock){+.+.}:
__mutex_lock_common kernel/locking/mutex.c:930 [inline]
__mutex_lock+0x158/0x1360 kernel/locking/mutex.c:1077
iowarrior_open+0x8a/0x2a0 drivers/usb/misc/iowarrior.c:600
usb_open+0x1df/0x270 drivers/usb/core/file.c:48
chrdev_open+0x219/0x5c0 fs/char_dev.c:414
do_dentry_open+0x494/0x1120 fs/open.c:797
do_last fs/namei.c:3416 [inline]
path_openat+0x1430/0x3f50 fs/namei.c:3533
do_filp_open+0x1a1/0x280 fs/namei.c:3563
do_sys_open+0x3c0/0x580 fs/open.c:1089
do_syscall_64+0xb7/0x580 arch/x86/entry/common.c:296
entry_SYSCALL_64_after_hwframe+0x49/0xbe
-> #1 (iowarrior_mutex){+.+.}:
__mutex_lock_common kernel/locking/mutex.c:930 [inline]
__mutex_lock+0x158/0x1360 kernel/locking/mutex.c:1077
iowarrior_open+0x23/0x2a0 drivers/usb/misc/iowarrior.c:589
usb_open+0x1df/0x270 drivers/usb/core/file.c:48
chrdev_open+0x219/0x5c0 fs/char_dev.c:414
do_dentry_open+0x494/0x1120 fs/open.c:797
do_last fs/namei.c:3416 [inline]
path_openat+0x1430/0x3f50 fs/namei.c:3533
do_filp_open+0x1a1/0x280 fs/namei.c:3563
do_sys_open+0x3c0/0x580 fs/open.c:1089
do_syscall_64+0xb7/0x580 arch/x86/entry/common.c:296
entry_SYSCALL_64_after_hwframe+0x49/0xbe
-> #0 (minor_rwsem){}:
check_prev_add kernel/locking/lockdep.c:2405 [inline]
check_prevs_add kernel/locking/lockdep.c:2507 [inline]
validate_chain kernel/locking/lockdep.c:2897 [inline]
__lock_acquire+0x1f7c/0x3b50 kernel/locking/lockdep.c:3880
lock_acquire+0x127/0x320 kernel/locking/lockdep.c:4412
down_write+0x92/0x150 kernel/locking/rwsem.c:1500
usb_deregister_dev drivers/usb/core/file.c:238 [inline]
usb_deregister_dev+0x61/0x270 drivers/usb/core/file.c:230
iowarrior_disconnect+0xa8/0x2c0 drivers/usb/misc/iowarrior.c:873
usb_unbind_interface+0x1bd/0x8a0 drivers/usb/core/driver.c:423
__device_release_driver drivers/base/dd.c:1120 [inline]
device_release_driver_internal+0x404/0x4c0 drivers/base/dd.c:1151
bus_remove_device+0x2dc/0x4a0 drivers/base/bus.c:556
device_del+0x420/0xb10 drivers/base/core.c:2288
usb_disable_device+0x211/0x690 drivers/usb/core/message.c:1237
usb_disconnect+0x284/0x8d0 drivers/usb/core/hub.c:2199
hub_port_connect drivers/usb/core/hub.c:4949 [inline]
hub_port_connect_change drivers/usb/core/hub.c:5213 [inline]
port_event drivers/usb/core/hub.c:5359 [inline]
hub_event+0x1454/0x3640 drivers/usb/core/hub.c:5441
process_one_work+0x92b/0x1530 kernel/workqueue.c:2269
worker_thread+0x96/0xe20 kernel/workqueue.c:2415
kthread+0x318/0x420 kernel/kthread.c:255
ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352
other info that might help us debug this:
Chain exists of:
minor_rwsem --> iowarrior_mutex --> iowarrior_open_disc_lock
Possible unsafe locking scenario:
CPU0CPU1
lock(iowarrior_open_disc_lock);
lock(iowarrior_mutex);
lock(iowarrior_open_disc_lock);
lock(minor_rwsem);
*** DEADLOCK ***
6 locks held by kwo
KASAN: out-of-bounds Read in hidraw_ioctl
Hello, syzbot found the following crash on: HEAD commit:e96407b4 usb-fuzzer: main usb gadget fuzzer driver git tree: https://github.com/google/kasan.git usb-fuzzer console output: https://syzkaller.appspot.com/x/log.txt?x=126120e260 kernel config: https://syzkaller.appspot.com/x/.config?x=cfa2c18fb6a8068e dashboard link: https://syzkaller.appspot.com/bug?extid=f817d84b72194c4a5fe2 compiler: gcc (GCC) 9.0.0 20181231 (experimental) Unfortunately, I don't have any reproducer for this crash yet. IMPORTANT: if you fix the bug, please add the following tag to the commit: Reported-by: syzbot+f817d84b72194c4a5...@syzkaller.appspotmail.com == BUG: KASAN: out-of-bounds in hidraw_ioctl+0x609/0xae0 drivers/hid/hidraw.c:380 Read of size 4 at addr 8881cb9fc018 by task syz-executor.1/3309 CPU: 1 PID: 3309 Comm: syz-executor.1 Not tainted 5.3.0-rc2+ #25 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 Call Trace: __dump_stack lib/dump_stack.c:77 [inline] dump_stack+0xca/0x13e lib/dump_stack.c:113 print_address_description+0x6a/0x32c mm/kasan/report.c:351 __kasan_report.cold+0x1a/0x33 mm/kasan/report.c:482 kasan_report+0xe/0x12 mm/kasan/common.c:612 hidraw_ioctl+0x609/0xae0 drivers/hid/hidraw.c:380 vfs_ioctl fs/ioctl.c:46 [inline] file_ioctl fs/ioctl.c:509 [inline] do_vfs_ioctl+0xd2d/0x1330 fs/ioctl.c:696 ksys_ioctl+0x9b/0xc0 fs/ioctl.c:713 __do_sys_ioctl fs/ioctl.c:720 [inline] __se_sys_ioctl fs/ioctl.c:718 [inline] __x64_sys_ioctl+0x6f/0xb0 fs/ioctl.c:718 do_syscall_64+0xb7/0x580 arch/x86/entry/common.c:296 entry_SYSCALL_64_after_hwframe+0x49/0xbe RIP: 0033:0x459829 Code: fd b7 fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 0f 83 cb b7 fb ff c3 66 2e 0f 1f 84 00 00 00 00 RSP: 002b:7f877bde2c78 EFLAGS: 0246 ORIG_RAX: 0010 RAX: ffda RBX: 0003 RCX: 00459829 RDX: 20001300 RSI: 80044801 RDI: 0004 RBP: 0075c118 R08: R09: R10: R11: 0246 R12: 7f877bde36d4 R13: 004c2206 R14: 004d5610 R15: Allocated by task 2751: save_stack+0x1b/0x80 mm/kasan/common.c:69 set_track mm/kasan/common.c:77 [inline] __kasan_kmalloc mm/kasan/common.c:487 [inline] __kasan_kmalloc.constprop.0+0xbf/0xd0 mm/kasan/common.c:460 slab_post_alloc_hook mm/slab.h:520 [inline] slab_alloc_node mm/slub.c:2766 [inline] __kmalloc_node_track_caller+0xd0/0x230 mm/slub.c:4361 __kmalloc_reserve.isra.0+0x39/0xe0 net/core/skbuff.c:141 __alloc_skb+0xef/0x5a0 net/core/skbuff.c:209 alloc_skb include/linux/skbuff.h:1055 [inline] alloc_uevent_skb+0x7b/0x210 lib/kobject_uevent.c:289 uevent_net_broadcast_untagged lib/kobject_uevent.c:325 [inline] kobject_uevent_net_broadcast lib/kobject_uevent.c:408 [inline] kobject_uevent_env+0x8ee/0x1160 lib/kobject_uevent.c:592 device_del+0x6b2/0xb10 drivers/base/core.c:2298 usb_disconnect+0x4c3/0x8d0 drivers/usb/core/hub.c:2225 hub_port_connect drivers/usb/core/hub.c:4949 [inline] hub_port_connect_change drivers/usb/core/hub.c:5213 [inline] port_event drivers/usb/core/hub.c:5359 [inline] hub_event+0x1454/0x3640 drivers/usb/core/hub.c:5441 process_one_work+0x92b/0x1530 kernel/workqueue.c:2269 worker_thread+0x96/0xe20 kernel/workqueue.c:2415 kthread+0x318/0x420 kernel/kthread.c:255 ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352 Freed by task 238: save_stack+0x1b/0x80 mm/kasan/common.c:69 set_track mm/kasan/common.c:77 [inline] __kasan_slab_free+0x130/0x180 mm/kasan/common.c:449 slab_free_hook mm/slub.c:1423 [inline] slab_free_freelist_hook mm/slub.c:1470 [inline] slab_free mm/slub.c:3012 [inline] kfree+0xe4/0x2f0 mm/slub.c:3953 skb_free_head+0x8b/0xa0 net/core/skbuff.c:591 skb_release_data+0x41f/0x7c0 net/core/skbuff.c:611 skb_release_all+0x46/0x60 net/core/skbuff.c:665 __kfree_skb net/core/skbuff.c:679 [inline] consume_skb net/core/skbuff.c:838 [inline] consume_skb+0xd9/0x320 net/core/skbuff.c:832 skb_free_datagram+0x16/0xf0 net/core/datagram.c:328 netlink_recvmsg+0x65e/0xee0 net/netlink/af_netlink.c:1996 sock_recvmsg_nosec net/socket.c:871 [inline] sock_recvmsg net/socket.c:889 [inline] sock_recvmsg+0xca/0x110 net/socket.c:885 ___sys_recvmsg+0x271/0x5a0 net/socket.c:2480 __sys_recvmsg+0xe9/0x1b0 net/socket.c:2537 do_syscall_64+0xb7/0x580 arch/x86/entry/common.c:296 entry_SYSCALL_64_after_hwframe+0x49/0xbe The buggy address belongs to the object at 8881cb9fc000 which belongs to the cache kmalloc-1k of size 1024 The buggy address is located 24 bytes inside of 1024-byte region [8881cb9fc000, 8881cb9fc400) The buggy address belongs to the page: page:ea00072e7f00 refcount:1 mapcount:0 mapping:8881da002280 index:0x0 compound_mapcount: 0
Re: KASAN: use-after-free Read in usb_kill_urb
(Sorry for the resend, I was in HTML mode earlier :S) Hi, I'm trying to get up to speed on USB kernel code. Sounds like dev->intf should have been set to NULL for the error path in ld_usb_probe() ? https://elixir.bootlin.com/linux/latest/source/drivers/usb/misc/ldusb.c#L666 On Fri, Aug 9, 2019 at 10:48 AM syzbot wrote: > > Hello, > > syzbot found the following crash on: > > HEAD commit:e96407b4 usb-fuzzer: main usb gadget fuzzer driver > git tree: https://github.com/google/kasan.git usb-fuzzer > console output: https://syzkaller.appspot.com/x/log.txt?x=1799392c60 > kernel config: https://syzkaller.appspot.com/x/.config?x=cfa2c18fb6a8068e > dashboard link: https://syzkaller.appspot.com/bug?extid=22ae4e3b9fcc8a5c153a > compiler: gcc (GCC) 9.0.0 20181231 (experimental) > syz repro: https://syzkaller.appspot.com/x/repro.syz?x=1134c80260 > C reproducer: https://syzkaller.appspot.com/x/repro.c?x=13278c4a60 > > IMPORTANT: if you fix the bug, please add the following tag to the commit: > Reported-by: syzbot+22ae4e3b9fcc8a5c1...@syzkaller.appspotmail.com > > == > BUG: KASAN: use-after-free in atomic_read > include/asm-generic/atomic-instrumented.h:26 [inline] > BUG: KASAN: use-after-free in usb_kill_urb drivers/usb/core/urb.c:695 > [inline] > BUG: KASAN: use-after-free in usb_kill_urb+0x24b/0x2c0 > drivers/usb/core/urb.c:687 > Read of size 4 at addr 8881d635b110 by task syz-executor672/1999 > > CPU: 1 PID: 1999 Comm: syz-executor672 Not tainted 5.3.0-rc2+ #25 > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS > Google 01/01/2011 > Call Trace: > __dump_stack lib/dump_stack.c:77 [inline] > dump_stack+0xca/0x13e lib/dump_stack.c:113 > print_address_description+0x6a/0x32c mm/kasan/report.c:351 > __kasan_report.cold+0x1a/0x33 mm/kasan/report.c:482 > kasan_report+0xe/0x12 mm/kasan/common.c:612 > check_memory_region_inline mm/kasan/generic.c:185 [inline] > check_memory_region+0x128/0x190 mm/kasan/generic.c:192 > atomic_read include/asm-generic/atomic-instrumented.h:26 [inline] > usb_kill_urb drivers/usb/core/urb.c:695 [inline] > usb_kill_urb+0x24b/0x2c0 drivers/usb/core/urb.c:687 > ld_usb_abort_transfers+0xb7/0x1d0 drivers/usb/misc/ldusb.c:196 > ld_usb_release+0x19f/0x400 drivers/usb/misc/ldusb.c:406 > __fput+0x2d7/0x840 fs/file_table.c:280 > task_work_run+0x13f/0x1c0 kernel/task_work.c:113 > exit_task_work include/linux/task_work.h:22 [inline] > do_exit+0x8ef/0x2c50 kernel/exit.c:878 > do_group_exit+0x125/0x340 kernel/exit.c:982 > __do_sys_exit_group kernel/exit.c:993 [inline] > __se_sys_exit_group kernel/exit.c:991 [inline] > __x64_sys_exit_group+0x3a/0x50 kernel/exit.c:991 > do_syscall_64+0xb7/0x580 arch/x86/entry/common.c:296 > entry_SYSCALL_64_after_hwframe+0x49/0xbe > RIP: 0033:0x440418 > Code: 00 00 be 3c 00 00 00 eb 19 66 0f 1f 84 00 00 00 00 00 48 89 d7 89 f0 > 0f 05 48 3d 00 f0 ff ff 77 21 f4 48 89 d7 44 89 c0 0f 05 <48> 3d 00 f0 ff > ff 76 e0 f7 d8 64 41 89 01 eb d8 0f 1f 84 00 00 00 > RSP: 002b:7ffe884abf88 EFLAGS: 0246 ORIG_RAX: 00e7 > RAX: ffda RBX: RCX: 00440418 > RDX: RSI: 003c RDI: > RBP: 004bff50 R08: 00e7 R09: ffd0 > R10: 0064 R11: 0246 R12: 0001 > R13: 006d2180 R14: R15: > > Allocated by task 22: > save_stack+0x1b/0x80 mm/kasan/common.c:69 > set_track mm/kasan/common.c:77 [inline] > __kasan_kmalloc mm/kasan/common.c:487 [inline] > __kasan_kmalloc.constprop.0+0xbf/0xd0 mm/kasan/common.c:460 > kmalloc include/linux/slab.h:557 [inline] > usb_alloc_urb+0x65/0xb0 drivers/usb/core/urb.c:73 > ld_usb_probe+0x3e1/0xa65 drivers/usb/misc/ldusb.c:708 > usb_probe_interface+0x305/0x7a0 drivers/usb/core/driver.c:361 > really_probe+0x281/0x650 drivers/base/dd.c:548 > driver_probe_device+0x101/0x1b0 drivers/base/dd.c:709 > __device_attach_driver+0x1c2/0x220 drivers/base/dd.c:816 > bus_for_each_drv+0x15c/0x1e0 drivers/base/bus.c:454 > __device_attach+0x217/0x360 drivers/base/dd.c:882 > bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514 > device_add+0xae6/0x16f0 drivers/base/core.c:2114 > usb_set_configuration+0xdf6/0x1670 drivers/usb/core/message.c:2023 > generic_probe+0x9d/0xd5 drivers/usb/core/generic.c:210 > usb_probe_device+0x99/0x100 drivers/usb/core/driver.c:266 > really_probe+0x281/0x650 drivers/base/dd.c:548 > driver_probe_device+0x101/0x1b0 drivers/base/dd.c:709 > __device_attach_driver+0x1c2/0x220 drivers/base/dd.c:816 > bus_for_each_drv+0x15c/0x1e0 drivers/base/bus.c:454 > __device_attach+0x217/0x360 drivers/base/dd.c:882 > bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514 > device_add+0xae6/0x16f0 drivers/base/core.c:2114 > usb_new_device.cold+0x6a4/0xe79 drivers/usb/core/
RE: [PATCH V2] usb: dwc3: gadget: trb_dequeue is not updated properly
>>> I need tracepoints to see what's going on, please collect tracepoints. >> >> See attached. Search for "length 16384/16384" to the USB request using >> sg list. And the transfer stalls at a request with "length 512/512". > > which gadget driver is this btw? Let's look at what happened leading up to > this problem: I'm using the upstream kernel with some Android patches from Google. Comparing to pure upstream, there is no change in drivers/usb/dwc3/. There are a few Android patches on top of drivers/usb/gadget, but those don't seem to have anything to do with this case as the patches are changing things like f_accessory/f_audio/f_midi. >><...>-3107 [001] d..1 164.184431: dwc3_alloc_request: ep1out: req >> 66963d3c length 0/0 zsI ==> 0 > > Allocated a new request > >><...>-3107 [001] d..2 164.184432: dwc3_ep_queue: ep1out: req >> 66963d3c length 0/16384 zsI ==> -115 > > queued it for a 16kiB transfer (split into 3 sglist entries) > >><...>-3107 [001] d..2 164.184433: dwc3_prepare_trb: ep1out: trb >> 11fb30b4 buf 77caf000 size 4096 ctrl 001d >> (HlCS:sc:normal) > > first one, 4kiB. Note the capital 'C', this one is chained. > >><...>-3107 [001] d..2 164.184433: dwc3_prepare_trb: ep1out: trb >> bc197bc2 buf 77cb size 8192 ctrl 001d >> (HlCS:sc:normal) > >second one, 8kiB. Also chained. > >><...>-3107 [001] d..2 164.184433: dwc3_prepare_trb: ep1out: trb >> ae5c00ad buf 77cb2000 size 4096 ctrl 0819 >> (HlcS:sC:normal) > > last one, 4kiB. NOT CHAINED! > >><...>-3107 [001] d..2 164.184438: dwc3_gadget_ep_cmd: ep1out: cmd >> 'Update Transfer' [20007] params --> status: >> Successful > > issue update transfer > >><...>-3107 [001] d..1 164.18: dwc3_alloc_request: ep1out: req >> fe2c6e9d length 0/0 zsI ==> 0 > > now gadget driver allocates a NEW request > >><...>-3107 [001] d..2 164.18: dwc3_ep_queue: ep1out: req >> fe2c6e9d length 0/512 zsI ==> -115 > > ... and queues it for zero bytes. Why? Why didn't gadget driver set the ZERO > flag in the original request? I don't see any difference between this request and the one above for 16Kbyte (except the length). Why are you expecting "Z" to be set? The 0 is request.actual, which is correct at this point of time. >><...>-3107 [001] d..2 164.184445: dwc3_prepare_trb: ep1out: trb >> 55827a46 buf 77cb3000 size 512 ctrl 0819 (HlcS:sC:normal) > > a single TRB is prepared correctly. > >><...>-3107 [001] d..2 164.184449: dwc3_gadget_ep_cmd: ep1out: cmd >> 'Update Transfer' [20007] params --> status: >> Successful > > Update transfer is issued. > >><...>-3165 [001] d..1 164.192315: dwc3_event: event (4084): >> ep1out: Transfer In Progress [0] (sIm) >><...>-3165 [001] d..1 164.192316: dwc3_complete_trb: ep1out: trb >> 17050f80 buf 77c63000 size 0 ctrl 001c (hlCS:sc:normal) >><...>-3165 [001] d..1 164.192325: dwc3_gadget_giveback: ep1out: >> req af8aa80e length 16384/16384 zsI ==> 0 >> kworker/u8:5-1165 [001] 164.192353: dwc3_free_request: ep1out: req >> af8aa80e length 16384/16384 zsI ==> 0 >><...>-3165 [001] d..1 164.192849: dwc3_event: event (4084): >> ep1out: Transfer In Progress [0] (sIm) >><...>-3165 [001] d..1 164.192851: dwc3_complete_trb: ep1out: trb >> c1f0fd23 buf 77c64000 size 0 ctrl 001c (hlCS:sc:normal) >><...>-3165 [001] d..1 164.192860: dwc3_gadget_giveback: ep1out: >> req 66963d3c length 16384/16384 zsI ==> 0 > > here we giveback the request to the gadget driver. Note that the TRB > addresses that were completed do NOT match the TRB addresses of the prepared > TRBs. > Again, which gadget driver is this? Where is the source code for this gadget > driver? Also note that we requested for event upon completion of the final > TRB only > but we get events for each and every TRB. I don't know how to interpret this trace log, but it didn't help me find the problem until I added my own log in the prepare_trb() and completed_trb() [ 86.817218] ffs_epfile_io:READ:SG: buf=fcc78b10 sg=952c5428 length=16384 num_sgs=4 [ 86.827242] __dwc3_prepare_one_trb: size=4096 bp=7780d000 ctrl=001d enq=b3 [ 86.836172] __dwc3_prepare_one_trb: size=4096 bp=7780e000 ctrl=001d enq=b4 [ 86.845100] __dwc3_prepare_one_trb: size=4096 bp=7780f000 ctrl=001d enq=b5 [ 86.854028] __dwc3_prepare_one_trb: size=4096 bp=7781 ctrl=0819 enq=b6 [ 86.862959] __dwc3_gadget_kick_transfer: updating buf=be7c4a4c sg=952c5428 length=16384 pending_sgs=4 remaining=0 [ 86.876146] ffs_epfile_io:READ:SG: buf=8b573d0c sg=d7992341 length=1
Re: KASAN: use-after-free Read in usb_kill_urb
On Fri, 9 Aug 2019, Prashant Malani wrote: > Hi, > > I'm trying to get up to speed on USB kernel code. Sounds like > dev->intf should have been set to NULL for the error path in > ld_usb_probe() ? Why should it? After all, dev gets deallocated at the end of ld_usb_probe(), where ld_usb_delete() is called. Who cares what value is stored in deallocated memory? Alan Stern
[PATCH] usb: dwc3: Disable phy suspend after power-on reset
For DRD controllers, the programming guide recommended that
GUSB3PIPECTL.SUSPENDABLE and GUSB2PHYCFG.SUSPHY to be cleared after
power-on reset and only set after the controller initialization is
completed. This can be done after device soft-reset in dwc3_core_init().
This patch makes sure to clear GUSB3PIPECTL.SUSPENDABLE and
GUSB2PHYCFG.SUSPHY before core initialization and only set them after
the device soft-reset is completed.
Reference: DWC_usb3 3.30a and DWC_usb31 1.90a programming guide section
1.2.49 and 1.2.45
Signed-off-by: Thinh Nguyen
---
drivers/usb/dwc3/core.c | 37 +
1 file changed, 37 insertions(+)
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 252c397860ef..edbc3709f28e 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -568,8 +568,11 @@ static int dwc3_core_ulpi_init(struct dwc3 *dwc)
*/
static int dwc3_phy_setup(struct dwc3 *dwc)
{
+ unsigned int hw_mode;
u32 reg;
+ hw_mode = DWC3_GHWPARAMS0_MODE(dwc->hwparams.hwparams0);
+
reg = dwc3_readl(dwc->regs, DWC3_GUSB3PIPECTL(0));
/*
@@ -587,6 +590,14 @@ static int dwc3_phy_setup(struct dwc3 *dwc)
if (dwc->revision > DWC3_REVISION_194A)
reg |= DWC3_GUSB3PIPECTL_SUSPHY;
+ /*
+* For DRD controllers, GUSB3PIPECTL.SUSPENDENABLE must be cleared after
+* power-on reset, and it can be set after core initialization, which is
+* after device soft-reset during initialization.
+*/
+ if (hw_mode == DWC3_GHWPARAMS0_MODE_DRD)
+ reg &= ~DWC3_GUSB3PIPECTL_SUSPHY;
+
if (dwc->u2ss_inp3_quirk)
reg |= DWC3_GUSB3PIPECTL_U2SSINP3OK;
@@ -670,6 +681,14 @@ static int dwc3_phy_setup(struct dwc3 *dwc)
if (dwc->revision > DWC3_REVISION_194A)
reg |= DWC3_GUSB2PHYCFG_SUSPHY;
+ /*
+* For DRD controllers, GUSB2PHYCFG.SUSPHY must be cleared after
+* power-on reset, and it can be set after core initialization, which is
+* after device soft-reset during initialization.
+*/
+ if (hw_mode == DWC3_GHWPARAMS0_MODE_DRD)
+ reg &= ~DWC3_GUSB2PHYCFG_SUSPHY;
+
if (dwc->dis_u2_susphy_quirk)
reg &= ~DWC3_GUSB2PHYCFG_SUSPHY;
@@ -905,9 +924,12 @@ static void dwc3_set_incr_burst_type(struct dwc3 *dwc)
*/
static int dwc3_core_init(struct dwc3 *dwc)
{
+ unsigned inthw_mode;
u32 reg;
int ret;
+ hw_mode = DWC3_GHWPARAMS0_MODE(dwc->hwparams.hwparams0);
+
/*
* Write Linux Version Code to our GUID register so it's easy to figure
* out which kernel version a bug was found.
@@ -943,6 +965,21 @@ static int dwc3_core_init(struct dwc3 *dwc)
if (ret)
goto err0a;
+ if (hw_mode == DWC3_GHWPARAMS0_MODE_DRD &&
+ dwc->revision > DWC3_REVISION_194A) {
+ if (!dwc->dis_u3_susphy_quirk) {
+ reg = dwc3_readl(dwc->regs, DWC3_GUSB3PIPECTL(0));
+ reg |= DWC3_GUSB3PIPECTL_SUSPHY;
+ dwc3_writel(dwc->regs, DWC3_GUSB3PIPECTL(0), reg);
+ }
+
+ if (!dwc->dis_u2_susphy_quirk) {
+ reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
+ reg |= DWC3_GUSB2PHYCFG_SUSPHY;
+ dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
+ }
+ }
+
dwc3_core_setup_global_control(dwc);
dwc3_core_num_eps(dwc);
--
2.11.0
Re: KASAN: use-after-free Read in usb_kill_urb
On Fri, 9 Aug 2019, syzbot wrote:
> Hello,
>
> syzbot found the following crash on:
>
> HEAD commit:e96407b4 usb-fuzzer: main usb gadget fuzzer driver
> git tree: https://github.com/google/kasan.git usb-fuzzer
> console output: https://syzkaller.appspot.com/x/log.txt?x=1799392c60
> kernel config: https://syzkaller.appspot.com/x/.config?x=cfa2c18fb6a8068e
> dashboard link: https://syzkaller.appspot.com/bug?extid=22ae4e3b9fcc8a5c153a
> compiler: gcc (GCC) 9.0.0 20181231 (experimental)
> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=1134c80260
> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=13278c4a60
>
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+22ae4e3b9fcc8a5c1...@syzkaller.appspotmail.com
>
> ==
> BUG: KASAN: use-after-free in atomic_read
> include/asm-generic/atomic-instrumented.h:26 [inline]
> BUG: KASAN: use-after-free in usb_kill_urb drivers/usb/core/urb.c:695
> [inline]
> BUG: KASAN: use-after-free in usb_kill_urb+0x24b/0x2c0
> drivers/usb/core/urb.c:687
> Read of size 4 at addr 8881d635b110 by task syz-executor672/1999
>
> CPU: 1 PID: 1999 Comm: syz-executor672 Not tainted 5.3.0-rc2+ #25
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> Google 01/01/2011
> Call Trace:
> __dump_stack lib/dump_stack.c:77 [inline]
> dump_stack+0xca/0x13e lib/dump_stack.c:113
> print_address_description+0x6a/0x32c mm/kasan/report.c:351
> __kasan_report.cold+0x1a/0x33 mm/kasan/report.c:482
> kasan_report+0xe/0x12 mm/kasan/common.c:612
> check_memory_region_inline mm/kasan/generic.c:185 [inline]
> check_memory_region+0x128/0x190 mm/kasan/generic.c:192
> atomic_read include/asm-generic/atomic-instrumented.h:26 [inline]
> usb_kill_urb drivers/usb/core/urb.c:695 [inline]
> usb_kill_urb+0x24b/0x2c0 drivers/usb/core/urb.c:687
> ld_usb_abort_transfers+0xb7/0x1d0 drivers/usb/misc/ldusb.c:196
> ld_usb_release+0x19f/0x400 drivers/usb/misc/ldusb.c:406
Since this also involves ldusb.c, maybe it will be fixed by the same
patch as the other bug.
Alan Stern
#syz test: https://github.com/google/kasan.git e96407b4
Index: usb-devel/drivers/usb/core/file.c
===
--- usb-devel.orig/drivers/usb/core/file.c
+++ usb-devel/drivers/usb/core/file.c
@@ -193,9 +193,10 @@ int usb_register_dev(struct usb_interfac
intf->minor = minor;
break;
}
- up_write(&minor_rwsem);
- if (intf->minor < 0)
+ if (intf->minor < 0) {
+ up_write(&minor_rwsem);
return -EXFULL;
+ }
/* create a usb class device for this usb interface */
snprintf(name, sizeof(name), class_driver->name, minor - minor_base);
@@ -203,12 +204,11 @@ int usb_register_dev(struct usb_interfac
MKDEV(USB_MAJOR, minor), class_driver,
"%s", kbasename(name));
if (IS_ERR(intf->usb_dev)) {
- down_write(&minor_rwsem);
usb_minors[minor] = NULL;
intf->minor = -1;
- up_write(&minor_rwsem);
retval = PTR_ERR(intf->usb_dev);
}
+ up_write(&minor_rwsem);
return retval;
}
EXPORT_SYMBOL_GPL(usb_register_dev);
@@ -234,12 +234,12 @@ void usb_deregister_dev(struct usb_inter
return;
dev_dbg(&intf->dev, "removing %d minor\n", intf->minor);
+ device_destroy(usb_class->class, MKDEV(USB_MAJOR, intf->minor));
down_write(&minor_rwsem);
usb_minors[intf->minor] = NULL;
up_write(&minor_rwsem);
- device_destroy(usb_class->class, MKDEV(USB_MAJOR, intf->minor));
intf->usb_dev = NULL;
intf->minor = -1;
destroy_usb_class();
Re: [PATCH] USB: storage: isd200: remove redundant assignment to variable sendToTransport
On Fri, 9 Aug 2019, Colin King wrote:
> From: Colin Ian King
>
> The variable sendToTransport is being initialized with a value that is
> never read and is being re-assigned a little later on. The assignment
> is redundant and hence can be removed.
>
> Addresses-Coverity: ("Unused value")
Of what use is that tag to general kernel developers?
> Signed-off-by: Colin Ian King
> ---
> drivers/usb/storage/isd200.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/usb/storage/isd200.c b/drivers/usb/storage/isd200.c
> index 2b474d60b4db..28e1128d53a4 100644
> --- a/drivers/usb/storage/isd200.c
> +++ b/drivers/usb/storage/isd200.c
> @@ -1511,7 +1511,7 @@ static int isd200_Initialization(struct us_data *us)
>
> static void isd200_ata_command(struct scsi_cmnd *srb, struct us_data *us)
> {
> - int sendToTransport = 1, orig_bufflen;
> + int sendToTransport, orig_bufflen;
> union ata_cdb ataCdb;
>
> /* Make sure driver was initialized */
Otherwise:
Acked-by: Alan Stern
Alan Stern
Re: possible deadlock in usb_deregister_dev
On Fri, 9 Aug 2019, syzbot wrote:
> syzbot has found a reproducer for the following crash on:
>
> HEAD commit:e96407b4 usb-fuzzer: main usb gadget fuzzer driver
> git tree: https://github.com/google/kasan.git usb-fuzzer
> console output: https://syzkaller.appspot.com/x/log.txt?x=15bf780e60
> kernel config: https://syzkaller.appspot.com/x/.config?x=cfa2c18fb6a8068e
> dashboard link: https://syzkaller.appspot.com/bug?extid=a64a382964bf6c71a9c0
> compiler: gcc (GCC) 9.0.0 20181231 (experimental)
> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=1678757460
> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=136cc4d260
>
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+a64a382964bf6c71a...@syzkaller.appspotmail.com
>
> usb 1-1: New USB device strings: Mfr=0, Product=0, SerialNumber=0
> usb 1-1: config 0 descriptor??
> iowarrior 1-1:0.236: IOWarrior product=0x1501, serial= interface=236 now
> attached to iowarrior0
> usb 1-1: USB disconnect, device number 2
> ==
> WARNING: possible circular locking dependency detected
> 5.3.0-rc2+ #25 Not tainted
> --
> kworker/0:1/12 is trying to acquire lock:
> cd63e8f1 (minor_rwsem){}, at: usb_deregister_dev
> drivers/usb/core/file.c:238 [inline]
> cd63e8f1 (minor_rwsem){}, at: usb_deregister_dev+0x61/0x270
> drivers/usb/core/file.c:230
>
> but task is already holding lock:
> 1d1989ef (iowarrior_open_disc_lock){+.+.}, at:
> iowarrior_disconnect+0x45/0x2c0 drivers/usb/misc/iowarrior.c:867
>
> which lock already depends on the new lock.
https://syzkaller.appspot.com/bug?extid=ca52394faa436d8131df is
undoubtedly a duplicate of this.
Alan Stern
Re: [PATCH] USB: storage: isd200: remove redundant assignment to variable sendToTransport
On 09/08/2019 20:29, Alan Stern wrote:
> On Fri, 9 Aug 2019, Colin King wrote:
>
>> From: Colin Ian King
>>
>> The variable sendToTransport is being initialized with a value that is
>> never read and is being re-assigned a little later on. The assignment
>> is redundant and hence can be removed.
>>
>> Addresses-Coverity: ("Unused value")
>
> Of what use is that tag to general kernel developers?
This is being informally used so that we can track which bugs are
getting found with specific static analysis tools. The public coverity
bug reports also have a CID# number. I'm working on range of coverity
builds (different build configs) that are not public because I can crank
multiple builds per day to find issues.
Colin
>
>> Signed-off-by: Colin Ian King
>> ---
>> drivers/usb/storage/isd200.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/storage/isd200.c b/drivers/usb/storage/isd200.c
>> index 2b474d60b4db..28e1128d53a4 100644
>> --- a/drivers/usb/storage/isd200.c
>> +++ b/drivers/usb/storage/isd200.c
>> @@ -1511,7 +1511,7 @@ static int isd200_Initialization(struct us_data *us)
>>
>> static void isd200_ata_command(struct scsi_cmnd *srb, struct us_data *us)
>> {
>> -int sendToTransport = 1, orig_bufflen;
>> +int sendToTransport, orig_bufflen;
>> union ata_cdb ataCdb;
>>
>> /* Make sure driver was initialized */
>
> Otherwise:
>
> Acked-by: Alan Stern
>
> Alan Stern
>
Re: KASAN: use-after-free Read in usb_kill_urb
Hello, syzbot has tested the proposed patch and the reproducer did not trigger crash: Reported-and-tested-by: syzbot+22ae4e3b9fcc8a5c1...@syzkaller.appspotmail.com Tested on: commit: e96407b4 usb-fuzzer: main usb gadget fuzzer driver git tree: https://github.com/google/kasan.git kernel config: https://syzkaller.appspot.com/x/.config?x=cfa2c18fb6a8068e compiler: gcc (GCC) 9.0.0 20181231 (experimental) patch: https://syzkaller.appspot.com/x/patch.diff?x=173f2d2c60 Note: testing is done by a robot and is best-effort only.
Re: KASAN: use-after-free Read in adu_disconnect
syzbot has found a reproducer for the following crash on: HEAD commit:e96407b4 usb-fuzzer: main usb gadget fuzzer driver git tree: https://github.com/google/kasan.git usb-fuzzer console output: https://syzkaller.appspot.com/x/log.txt?x=13871a4a60 kernel config: https://syzkaller.appspot.com/x/.config?x=cfa2c18fb6a8068e dashboard link: https://syzkaller.appspot.com/bug?extid=0243cb250a51eeefb8cc compiler: gcc (GCC) 9.0.0 20181231 (experimental) syz repro: https://syzkaller.appspot.com/x/repro.syz?x=11c4c8e260 C reproducer: https://syzkaller.appspot.com/x/repro.c?x=11d80d2c60 IMPORTANT: if you fix the bug, please add the following tag to the commit: Reported-by: syzbot+0243cb250a51eeefb...@syzkaller.appspotmail.com usb 1-1: USB disconnect, device number 4 == BUG: KASAN: use-after-free in atomic64_read include/asm-generic/atomic-instrumented.h:836 [inline] BUG: KASAN: use-after-free in atomic_long_read include/asm-generic/atomic-long.h:28 [inline] BUG: KASAN: use-after-free in __mutex_unlock_slowpath+0x96/0x670 kernel/locking/mutex.c:1211 Read of size 8 at addr 8881d1d0aa00 by task kworker/0:1/12 CPU: 0 PID: 12 Comm: kworker/0:1 Not tainted 5.3.0-rc2+ #25 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 Workqueue: usb_hub_wq hub_event Call Trace: __dump_stack lib/dump_stack.c:77 [inline] dump_stack+0xca/0x13e lib/dump_stack.c:113 print_address_description+0x6a/0x32c mm/kasan/report.c:351 __kasan_report.cold+0x1a/0x33 mm/kasan/report.c:482 kasan_report+0xe/0x12 mm/kasan/common.c:612 check_memory_region_inline mm/kasan/generic.c:185 [inline] check_memory_region+0x128/0x190 mm/kasan/generic.c:192 atomic64_read include/asm-generic/atomic-instrumented.h:836 [inline] atomic_long_read include/asm-generic/atomic-long.h:28 [inline] __mutex_unlock_slowpath+0x96/0x670 kernel/locking/mutex.c:1211 adu_disconnect+0x83/0x150 drivers/usb/misc/adutux.c:768 usb_unbind_interface+0x1bd/0x8a0 drivers/usb/core/driver.c:423 __device_release_driver drivers/base/dd.c:1120 [inline] device_release_driver_internal+0x404/0x4c0 drivers/base/dd.c:1151 bus_remove_device+0x2dc/0x4a0 drivers/base/bus.c:556 device_del+0x420/0xb10 drivers/base/core.c:2288 usb_disable_device+0x211/0x690 drivers/usb/core/message.c:1237 usb_disconnect+0x284/0x8d0 drivers/usb/core/hub.c:2199 hub_port_connect drivers/usb/core/hub.c:4949 [inline] hub_port_connect_change drivers/usb/core/hub.c:5213 [inline] port_event drivers/usb/core/hub.c:5359 [inline] hub_event+0x1454/0x3640 drivers/usb/core/hub.c:5441 process_one_work+0x92b/0x1530 kernel/workqueue.c:2269 worker_thread+0x96/0xe20 kernel/workqueue.c:2415 kthread+0x318/0x420 kernel/kthread.c:255 ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352 Allocated by task 22: save_stack+0x1b/0x80 mm/kasan/common.c:69 set_track mm/kasan/common.c:77 [inline] __kasan_kmalloc mm/kasan/common.c:487 [inline] __kasan_kmalloc.constprop.0+0xbf/0xd0 mm/kasan/common.c:460 kmalloc include/linux/slab.h:552 [inline] kzalloc include/linux/slab.h:748 [inline] adu_probe+0x7d/0x6e0 drivers/usb/misc/adutux.c:660 usb_probe_interface+0x305/0x7a0 drivers/usb/core/driver.c:361 really_probe+0x281/0x650 drivers/base/dd.c:548 driver_probe_device+0x101/0x1b0 drivers/base/dd.c:709 __device_attach_driver+0x1c2/0x220 drivers/base/dd.c:816 bus_for_each_drv+0x15c/0x1e0 drivers/base/bus.c:454 __device_attach+0x217/0x360 drivers/base/dd.c:882 bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514 device_add+0xae6/0x16f0 drivers/base/core.c:2114 usb_set_configuration+0xdf6/0x1670 drivers/usb/core/message.c:2023 generic_probe+0x9d/0xd5 drivers/usb/core/generic.c:210 usb_probe_device+0x99/0x100 drivers/usb/core/driver.c:266 really_probe+0x281/0x650 drivers/base/dd.c:548 driver_probe_device+0x101/0x1b0 drivers/base/dd.c:709 __device_attach_driver+0x1c2/0x220 drivers/base/dd.c:816 bus_for_each_drv+0x15c/0x1e0 drivers/base/bus.c:454 __device_attach+0x217/0x360 drivers/base/dd.c:882 bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514 device_add+0xae6/0x16f0 drivers/base/core.c:2114 usb_new_device.cold+0x6a4/0xe79 drivers/usb/core/hub.c:2536 hub_port_connect drivers/usb/core/hub.c:5098 [inline] hub_port_connect_change drivers/usb/core/hub.c:5213 [inline] port_event drivers/usb/core/hub.c:5359 [inline] hub_event+0x1b5c/0x3640 drivers/usb/core/hub.c:5441 process_one_work+0x92b/0x1530 kernel/workqueue.c:2269 worker_thread+0x96/0xe20 kernel/workqueue.c:2415 kthread+0x318/0x420 kernel/kthread.c:255 ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352 Freed by task 1733: save_stack+0x1b/0x80 mm/kasan/common.c:69 set_track mm/kasan/common.c:77 [inline] __kasan_slab_free+0x130/0x180 mm/kasan/common.c:449 slab_free_hook mm/slub.c:1423 [inline] slab_free_freelist_hook mm/slub.c:1470 [inline] slab_free mm/slub.c:3012 [inline] kfree+0xe4/0x2f0 mm/slub.c
[PATCH 15/16] usb: remove ehci-w90x900 driver
The ARM w90x900 platform is getting removed, so this driver is obsolete.
Signed-off-by: Arnd Bergmann
---
drivers/usb/host/Kconfig| 6 --
drivers/usb/host/Makefile | 1 -
drivers/usb/host/ehci-w90x900.c | 130
3 files changed, 137 deletions(-)
delete mode 100644 drivers/usb/host/ehci-w90x900.c
diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index 40b5de597112..782ead054a90 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -287,12 +287,6 @@ config USB_EHCI_MV
Dova, Armada 370 and Armada XP. See "Support for Marvell EBU
on-chip EHCI USB controller" for those.
-config USB_W90X900_EHCI
- tristate "W90X900(W90P910) EHCI support"
- depends on ARCH_W90X900
- ---help---
- Enables support for the W90X900 USB controller
-
config USB_CNS3XXX_EHCI
bool "Cavium CNS3XXX EHCI Module (DEPRECATED)"
depends on ARCH_CNS3XXX
diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
index 84514f71ae44..0bba93de7654 100644
--- a/drivers/usb/host/Makefile
+++ b/drivers/usb/host/Makefile
@@ -51,7 +51,6 @@ obj-$(CONFIG_USB_EHCI_HCD_STI)+= ehci-st.o
obj-$(CONFIG_USB_EHCI_EXYNOS) += ehci-exynos.o
obj-$(CONFIG_USB_EHCI_HCD_AT91) += ehci-atmel.o
obj-$(CONFIG_USB_EHCI_TEGRA) += ehci-tegra.o
-obj-$(CONFIG_USB_W90X900_EHCI) += ehci-w90x900.o
obj-$(CONFIG_USB_OXU210HP_HCD) += oxu210hp-hcd.o
obj-$(CONFIG_USB_ISP116X_HCD) += isp116x-hcd.o
diff --git a/drivers/usb/host/ehci-w90x900.c b/drivers/usb/host/ehci-w90x900.c
deleted file mode 100644
index 6d77ace1697b..
--- a/drivers/usb/host/ehci-w90x900.c
+++ /dev/null
@@ -1,130 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-/*
- * linux/driver/usb/host/ehci-w90x900.c
- *
- * Copyright (c) 2008 Nuvoton technology corporation.
- *
- * Wan ZongShun
- */
-
-#include
-#include
-#include
-#include
-#include
-#include
-#include
-#include
-
-#include "ehci.h"
-
-/* enable phy0 and phy1 for w90p910 */
-#defineENPHY (0x01<<8)
-#define PHY0_CTR (0xA4)
-#define PHY1_CTR (0xA8)
-
-#define DRIVER_DESC "EHCI w90x900 driver"
-
-static const char hcd_name[] = "ehci-w90x900 ";
-
-static struct hc_driver __read_mostly ehci_w90x900_hc_driver;
-
-static int ehci_w90x900_probe(struct platform_device *pdev)
-{
- struct usb_hcd *hcd;
- struct ehci_hcd *ehci;
- struct resource *res;
- int retval = 0, irq;
- unsigned long val;
-
- hcd = usb_create_hcd(&ehci_w90x900_hc_driver,
- &pdev->dev, "w90x900 EHCI");
- if (!hcd) {
- retval = -ENOMEM;
- goto err1;
- }
-
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- hcd->regs = devm_ioremap_resource(&pdev->dev, res);
- if (IS_ERR(hcd->regs)) {
- retval = PTR_ERR(hcd->regs);
- goto err2;
- }
- hcd->rsrc_start = res->start;
- hcd->rsrc_len = resource_size(res);
-
- ehci = hcd_to_ehci(hcd);
- ehci->caps = hcd->regs;
- ehci->regs = hcd->regs +
- HC_LENGTH(ehci, ehci_readl(ehci, &ehci->caps->hc_capbase));
-
- /* enable PHY 0,1,the regs only apply to w90p910
-* 0xA4,0xA8 were offsets of PHY0 and PHY1 controller of
-* w90p910 IC relative to ehci->regs.
-*/
- val = __raw_readl(ehci->regs+PHY0_CTR);
- val |= ENPHY;
- __raw_writel(val, ehci->regs+PHY0_CTR);
-
- val = __raw_readl(ehci->regs+PHY1_CTR);
- val |= ENPHY;
- __raw_writel(val, ehci->regs+PHY1_CTR);
-
- irq = platform_get_irq(pdev, 0);
- if (irq < 0) {
- retval = irq;
- goto err2;
- }
-
- retval = usb_add_hcd(hcd, irq, IRQF_SHARED);
- if (retval != 0)
- goto err2;
-
- device_wakeup_enable(hcd->self.controller);
- return retval;
-err2:
- usb_put_hcd(hcd);
-err1:
- return retval;
-}
-
-static int ehci_w90x900_remove(struct platform_device *pdev)
-{
- struct usb_hcd *hcd = platform_get_drvdata(pdev);
-
- usb_remove_hcd(hcd);
- usb_put_hcd(hcd);
-
- return 0;
-}
-
-static struct platform_driver ehci_hcd_w90x900_driver = {
- .probe = ehci_w90x900_probe,
- .remove = ehci_w90x900_remove,
- .driver = {
- .name = "w90x900-ehci",
- },
-};
-
-static int __init ehci_w90X900_init(void)
-{
- if (usb_disabled())
- return -ENODEV;
-
- pr_info("%s: " DRIVER_DESC "\n", hcd_name);
-
- ehci_init_driver(&ehci_w90x900_hc_driver, NULL);
- return platform_driver_register(&ehci_hcd_w90x900_driver);
-}
-module_init(ehci_w90X900_init);
-
-static void __exit ehci_w90X900_cleanup(void)
-{
- platform_driver_unregister(&ehci_hcd_w90x900_driver);
-}
-module_exit(ehci_w90X900_cleanup);
-
-MODULE_DESCRIPTION(DRIVER_DESC);
-MODULE_AUTHOR("Wan ZongShun ");
-MODULE_
Re: KASAN: use-after-free Read in usbhid_power
Am Donnerstag, den 08.08.2019, 20:54 +0200 schrieb Andrey Konovalov:
> On Thu, Jul 25, 2019 at 5:09 PM Alan Stern wrote:
> >
> > On Thu, 25 Jul 2019, Oliver Neukum wrote:
> >
> > > Am Mittwoch, den 24.07.2019, 17:02 -0400 schrieb Alan Stern:
> > > > On Wed, 24 Jul 2019, Oliver Neukum wrote:
> > > >
> > > > > drivers/hid/usbhid/hid-core.c | 13 +
> > > > > 1 file changed, 13 insertions(+)
> > > > >
> > > > > diff --git a/drivers/hid/usbhid/hid-core.c
> > > > > b/drivers/hid/usbhid/hid-core.c
> > > > > index c7bc9db5b192..98b996ecf4d3 100644
> > > > > --- a/drivers/hid/usbhid/hid-core.c
> > > > > +++ b/drivers/hid/usbhid/hid-core.c
> > > > > @@ -1229,6 +1229,17 @@ static int usbhid_power(struct hid_device
> > > > > *hid, int lvl)
> > > > > struct usbhid_device *usbhid = hid->driver_data;
> > > > > int r = 0;
> > > > >
> > > > > + spin_lock_irq(&usbhid->lock);
> > > > > + if (test_bit(HID_DISCONNECTED, &usbhid->iofl)) {
> > > > > + r = -ENODEV;
> > > > > + spin_unlock_irq(&usbhid->lock);
> > > > > + goto bail_out;
> > > > > + } else {
> > > > > + /* protect against disconnect */
> > > > > + usb_get_dev(interface_to_usbdev(usbhid->intf));
> > > > > + spin_unlock_irq(&usbhid->lock);
> > > > > + }
> > > > > +
> > > > > switch (lvl) {
> > > > > case PM_HINT_FULLON:
> > > > > r = usb_autopm_get_interface(usbhid->intf);
> > > > > @@ -1238,7 +1249,9 @@ static int usbhid_power(struct hid_device *hid,
> > > > > int lvl)
> > > > > usb_autopm_put_interface(usbhid->intf);
> > > > > break;
> > > > > }
> > > > > + usb_put_dev(interface_to_usbdev(usbhid->intf));
> > > > >
> > > > > +bail_out:
> > > > > return r;
> > > > > }
> > > >
> > > > Isn't this treating the symptom instead of the cause?
> > >
> > > Sort of. Holding a reference for the whole time would have merit,
> > > but I doubt it is strictly necessary.
> >
> > Just to be crystal clear, I was talking about a device reference --
> > usb_{get,put}_dev or usb_{get,put}_intf -- not a runtime PM reference.
> >
> > (Incidentally, your patch could be simplified by using usb_get_intf
> > instead of usb_get_dev.)
> >
> > > > Shouldn't the hid_device hold a reference to usbhid->intf throughout
> > > > its lifetime? That way this sort of problem wouldn't arise in any
> > > > routine, not just usbhid_power().
> > >
> > > Unfortunately the semantics would still be wrong without the check
> > > in corner cases. In case disconnect() is called without a physical
> > > unplug, we must not touch the power state.
> > > I am indeed afraid that in that case my putative fix is still racy.
> > > But I don't to just introduce a mutex just for this. Any ideas?
> >
> > That's a separate issue. USB drivers -- indeed, all drivers -- are
> > required to balance their runtime PM gets and puts (although in the
> > case of a physical disconnection it doesn't matter). Are you asking
> > about the best way to do this?
> >
> > Normally a driver's release or disconnect routine will stop all
> > asynchronous accesses to the device (interrupt handlers, work queues,
> > URBs, and so on). At that point the only remaining runtime PM activity
> > will be whatever the routine itself does. So it can see if any extra
> > runtime PM gets or puts are needed, and do whatever is necessary.
> >
> > Does that answer your question? I can't tell for sure...
> >
> > Note: I did not try to track down the reason for the invalid access
> > reported by syzbot. It looked like a simple use-after-free, which
> > would normally be fixed by taking the appropriate reference. Which is
> > what your patch does, except that it holds the reference only for a
> > short time instead of over the entire lifetime of the private data
> > structure (the usbhid structure), which is what normally happens.
>
> This report looks like very similar to these two:
>
> https://syzkaller.appspot.com/bug?extid=b156665cf4d1b5e00c76
> https://syzkaller.appspot.com/bug?extid=3cbe5cd105d2ad56a1df
Yes, they stem from the same root cause most likely.
> Maybe we should mark those two as duplicates.
>
> Hillf suggested a fix on one of them, but it looks different from what
> you propose:
>
> https://groups.google.com/d/msg/syzkaller-bugs/xW7LvKfpyn0/SpKbs5ZLEAAJ
The fix may indeed be necessary, but it is incomplete. It does
not help keeping the PM counters consistent.
Regards
Oliver
Re: KASAN: use-after-free Read in usb_kill_urb
On Fri, 9 Aug 2019, syzbot wrote: > Hello, > > syzbot has tested the proposed patch and the reproducer did not trigger > crash: > > Reported-and-tested-by: > syzbot+22ae4e3b9fcc8a5c1...@syzkaller.appspotmail.com > > Tested on: > > commit: e96407b4 usb-fuzzer: main usb gadget fuzzer driver > git tree: https://github.com/google/kasan.git > kernel config: https://syzkaller.appspot.com/x/.config?x=cfa2c18fb6a8068e > compiler: gcc (GCC) 9.0.0 20181231 (experimental) > patch: https://syzkaller.appspot.com/x/patch.diff?x=173f2d2c60 > > Note: testing is done by a robot and is best-effort only. This shows that this bug is a duplicate of extid=30cf45ebfe0b0c4847a1. This fact is also visible in the console logs; both have lines saying something like: [ 549.416341][ T22] sysfs: cannot create duplicate filename '/class/usbmisc/ldusb0' and [ 549.958755][ T22] ldusb 1-1:0.28: Not able to get a minor for this device. preceding the invalid access. Alan Stern
[PATCH v2] dt-bindings: usb: renesas_gen3: Rename bindings documentation file to reflect IP block
For consistency with the naming of (most) other documentation files for DT
bindings for Renesas IP blocks rename the Renesas USB3.0 peripheral
documentation file from renesas,usb3.txt to renesas,usb3-peri.txt
This refines a recent rename from renesas_usb3.txt to renesas-usb3.txt.
The motivation is to more accurately reflect the IP block documented in
this file.
Signed-off-by: Simon Horman
Reviewed-by: Geert Uytterhoeven
---
* Based on v5.3-rc1
v2
* Add review tag
* Correct changelog
---
.../devicetree/bindings/usb/{renesas,usb3.txt => renesas,usb3-peri.txt} | 0
1 file changed, 0 insertions(+), 0 deletions(-)
rename Documentation/devicetree/bindings/usb/{renesas,usb3.txt =>
renesas,usb3-peri.txt} (100%)
diff --git a/Documentation/devicetree/bindings/usb/renesas,usb3.txt
b/Documentation/devicetree/bindings/usb/renesas,usb3-peri.txt
similarity index 100%
rename from Documentation/devicetree/bindings/usb/renesas,usb3.txt
rename to Documentation/devicetree/bindings/usb/renesas,usb3-peri.txt
--
2.11.0
Re: [PATCH v2] dt-bindings: usb: renesas_gen3: Rename bindings documentation file to reflect IP block
Hi Simon,
Thanks for your work.
On 2019-08-09 14:37:10 -0700, Simon Horman wrote:
> For consistency with the naming of (most) other documentation files for DT
> bindings for Renesas IP blocks rename the Renesas USB3.0 peripheral
> documentation file from renesas,usb3.txt to renesas,usb3-peri.txt
>
> This refines a recent rename from renesas_usb3.txt to renesas-usb3.txt.
> The motivation is to more accurately reflect the IP block documented in
> this file.
>
> Signed-off-by: Simon Horman
> Reviewed-by: Geert Uytterhoeven
Reviewed-by: Niklas Söderlund
> ---
> * Based on v5.3-rc1
>
> v2
> * Add review tag
> * Correct changelog
> ---
> .../devicetree/bindings/usb/{renesas,usb3.txt => renesas,usb3-peri.txt} | 0
> 1 file changed, 0 insertions(+), 0 deletions(-)
> rename Documentation/devicetree/bindings/usb/{renesas,usb3.txt =>
> renesas,usb3-peri.txt} (100%)
>
> diff --git a/Documentation/devicetree/bindings/usb/renesas,usb3.txt
> b/Documentation/devicetree/bindings/usb/renesas,usb3-peri.txt
> similarity index 100%
> rename from Documentation/devicetree/bindings/usb/renesas,usb3.txt
> rename to Documentation/devicetree/bindings/usb/renesas,usb3-peri.txt
> --
> 2.11.0
>
--
Regards,
Niklas Söderlund
Re: [PATCH v2] dt-bindings: usb: renesas_gen3: Rename bindings documentation file to reflect IP block
Hi Simon,
On Fri, Aug 9, 2019 at 11:37 PM Simon Horman wrote:
> For consistency with the naming of (most) other documentation files for DT
> bindings for Renesas IP blocks rename the Renesas USB3.0 peripheral
> documentation file from renesas,usb3.txt to renesas,usb3-peri.txt
>
> This refines a recent rename from renesas_usb3.txt to renesas-usb3.txt.
s/renesas-usb3.txt/renesas,usb3.txt/
> The motivation is to more accurately reflect the IP block documented in
> this file.
>
> Signed-off-by: Simon Horman
> Reviewed-by: Geert Uytterhoeven
> ---
> * Based on v5.3-rc1
>
> v2
> * Add review tag
> * Correct changelog
I feel sorry for you...
> ---
> .../devicetree/bindings/usb/{renesas,usb3.txt => renesas,usb3-peri.txt} | 0
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
