2020-08-07 10:10:37

by kernel test robot

[permalink] [raw]
Subject: drivers/usb/chipidea/core.c:657 ci_usb_role_switch_set() error: double unlocked 'ci->lock' (orig line 638)

tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
head: 86cfccb66937dd6cbf26ed619958b9e587e6a115
commit: 05559f10ed797b79f7fa47313682c48919a2b111 usb: chipidea: add role switch class support
date: 12 months ago
config: parisc-randconfig-m031-20200807 (attached as .config)
compiler: hppa-linux-gcc (GCC) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>

smatch warnings:
drivers/usb/chipidea/core.c:657 ci_usb_role_switch_set() error: double unlocked 'ci->lock' (orig line 638)

vim +657 drivers/usb/chipidea/core.c

615
616 static int ci_usb_role_switch_set(struct device *dev, enum usb_role role)
617 {
618 struct ci_hdrc *ci = dev_get_drvdata(dev);
619 struct ci_hdrc_cable *cable = NULL;
620 enum usb_role current_role = ci_role_to_usb_role(ci);
621 unsigned long flags;
622
623 if (current_role == role)
624 return 0;
625
626 pm_runtime_get_sync(ci->dev);
627 /* Stop current role */
628 spin_lock_irqsave(&ci->lock, flags);
629 if (current_role == USB_ROLE_DEVICE)
630 cable = &ci->platdata->vbus_extcon;
631 else if (current_role == USB_ROLE_HOST)
632 cable = &ci->platdata->id_extcon;
633
634 if (cable) {
635 cable->changed = true;
636 cable->connected = false;
637 ci_irq(ci->irq, ci);
> 638 spin_unlock_irqrestore(&ci->lock, flags);
639 if (ci->wq && role != USB_ROLE_NONE)
640 flush_workqueue(ci->wq);
641 spin_lock_irqsave(&ci->lock, flags);
642 }
643
644 cable = NULL;
645
646 /* Start target role */
647 if (role == USB_ROLE_DEVICE)
648 cable = &ci->platdata->vbus_extcon;
649 else if (role == USB_ROLE_HOST)
650 cable = &ci->platdata->id_extcon;
651
652 if (cable) {
653 cable->changed = true;
654 cable->connected = true;
655 ci_irq(ci->irq, ci);
656 }
> 657 spin_unlock_irqrestore(&ci->lock, flags);
658 pm_runtime_put_sync(ci->dev);
659
660 return 0;
661 }
662

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]


Attachments:
(No filename) (2.21 kB)
.config.gz (24.62 kB)
Download all attachments

2020-08-08 02:45:33

by Jun Li

[permalink] [raw]
Subject: RE: drivers/usb/chipidea/core.c:657 ci_usb_role_switch_set() error: double unlocked 'ci->lock' (orig line 638)

Hi,

> -----Original Message-----
> From: kernel test robot <[email protected]>
> Sent: Friday, August 7, 2020 6:00 PM
> To: Jun Li <[email protected]>
> Cc: [email protected]; [email protected]; Peter Chen
> <[email protected]>
> Subject: drivers/usb/chipidea/core.c:657 ci_usb_role_switch_set() error: double
> unlocked 'ci->lock' (orig line 638)
>
> tree:
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.kernel.o
> rg%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftorvalds%2Flinux.git&amp;data=02%7C01%
> 7Cjun.li%40nxp.com%7Ce261827f14f6446b3e8308d83ab8c407%7C686ea1d3bc2b4c6fa92cd9
> 9c5c301635%7C0%7C0%7C637323912694699319&amp;sdata=ioDexF9tIACz%2BlnZC%2BzZsiFz
> iOeeLembxqr9PwE%2Fpgg%3D&amp;reserved=0 master
> head: 86cfccb66937dd6cbf26ed619958b9e587e6a115
> commit: 05559f10ed797b79f7fa47313682c48919a2b111 usb: chipidea: add role switch
> class support
> date: 12 months ago
> config: parisc-randconfig-m031-20200807 (attached as .config)
> compiler: hppa-linux-gcc (GCC) 9.3.0
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <[email protected]>
>
> smatch warnings:
> drivers/usb/chipidea/core.c:657 ci_usb_role_switch_set() error: double unlocked
> 'ci->lock' (orig line 638)
>
> vim +657 drivers/usb/chipidea/core.c
>
> 615
> 616 static int ci_usb_role_switch_set(struct device *dev, enum usb_role role)
> 617 {
> 618 struct ci_hdrc *ci = dev_get_drvdata(dev);
> 619 struct ci_hdrc_cable *cable = NULL;
> 620 enum usb_role current_role = ci_role_to_usb_role(ci);
> 621 unsigned long flags;
> 622
> 623 if (current_role == role)
> 624 return 0;
> 625
> 626 pm_runtime_get_sync(ci->dev);
> 627 /* Stop current role */
> 628 spin_lock_irqsave(&ci->lock, flags);
> 629 if (current_role == USB_ROLE_DEVICE)
> 630 cable = &ci->platdata->vbus_extcon;
> 631 else if (current_role == USB_ROLE_HOST)
> 632 cable = &ci->platdata->id_extcon;
> 633
> 634 if (cable) {
> 635 cable->changed = true;
> 636 cable->connected = false;
> 637 ci_irq(ci->irq, ci);
> > 638 spin_unlock_irqrestore(&ci->lock, flags);
> 639 if (ci->wq && role != USB_ROLE_NONE)
> 640 flush_workqueue(ci->wq);
> 641 spin_lock_irqsave(&ci->lock, flags);

Here we hold the lock again.

> 642 }
> 643
> 644 cable = NULL;
> 645
> 646 /* Start target role */
> 647 if (role == USB_ROLE_DEVICE)
> 648 cable = &ci->platdata->vbus_extcon;
> 649 else if (role == USB_ROLE_HOST)
> 650 cable = &ci->platdata->id_extcon;
> 651
> 652 if (cable) {
> 653 cable->changed = true;
> 654 cable->connected = true;
> 655 ci_irq(ci->irq, ci);
> 656 }
> > 657 spin_unlock_irqrestore(&ci->lock, flags);

So it's fine to unlock here.

Thanks
Li Jun
> 658 pm_runtime_put_sync(ci->dev);
> 659
> 660 return 0;
> 661 }
> 662
>
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.01.org
> %2Fhyperkitty%2Flist%2Fkbuild-all%40lists.01.org&amp;data=02%7C01%7Cjun.li%40n
> xp.com%7Ce261827f14f6446b3e8308d83ab8c407%7C686ea1d3bc2b4c6fa92cd99c5c301635%7
> C0%7C0%7C637323912694709319&amp;sdata=tAxmh6eq3yT3A89mY2RrGQxwdkUN4A9r708lFQJW
> gcI%3D&amp;reserved=0