linux-kernel - RE: [PATCH 1/1] usb: cdns3: Put the cdns set active part outside the spin lock

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <BYAPR07MB53814C647FA59B26D176E150DD5AA@BYAPR07MB5381.namprd07.prod.outlook.com>
Date:   Wed, 14 Jun 2023 06:14:56 +0000
From:   Pawel Laszczak <pawell@...ence.com>
To:     Xiaolei Wang <xiaolei.wang@...driver.com>,
        "peter.chen@...nel.org" <peter.chen@...nel.org>,
        "rogerq@...nel.org" <rogerq@...nel.org>,
        "a-govindraju@...com" <a-govindraju@...com>,
        "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>
CC:     "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH 1/1] usb: cdns3: Put the cdns set active part outside the
 spin lock


>The device may be scheduled during the resume process, so this cannot
>appear in atomic operations. Since pm_runtime_set_active will resume
>suppliers, put set active outside the spin lock, which is only used to protect the
>struct cdns data structure, otherwise the kernel will report the following
>warning:
>
>  BUG: sleeping function called from invalid context at
>drivers/base/power/runtime.c:1163
>  in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 651, name: sh
>  preempt_count: 1, expected: 0
>  RCU nest depth: 0, expected: 0
>  CPU: 0 PID: 651 Comm: sh Tainted: G        WC         6.1.20 #1
>  Hardware name: Freescale i.MX8QM MEK (DT)
>  Call trace:
>    dump_backtrace.part.0+0xe0/0xf0
>    show_stack+0x18/0x30
>    dump_stack_lvl+0x64/0x80
>    dump_stack+0x1c/0x38
>    __might_resched+0x1fc/0x240
>    __might_sleep+0x68/0xc0
>    __pm_runtime_resume+0x9c/0xe0
>    rpm_get_suppliers+0x68/0x1b0
>    __pm_runtime_set_status+0x298/0x560
>    cdns_resume+0xb0/0x1c0
>    cdns3_controller_resume.isra.0+0x1e0/0x250
>    cdns3_plat_resume+0x28/0x40
>
>Signed-off-by: Xiaolei Wang <xiaolei.wang@...driver.com>

Reviewed-by: Pawel Laszczak <pawell@...ence.com>

>---
> drivers/usb/cdns3/cdns3-plat.c |  3 ++-  drivers/usb/cdns3/cdnsp-pci.c  |  3
>++-
> drivers/usb/cdns3/core.c       | 14 +++++++++++---
> drivers/usb/cdns3/core.h       |  3 ++-
> 4 files changed, 17 insertions(+), 6 deletions(-)
>
>diff --git a/drivers/usb/cdns3/cdns3-plat.c b/drivers/usb/cdns3/cdns3-plat.c
>index 2bc5d094548b..726b2e4f67e4 100644
>--- a/drivers/usb/cdns3/cdns3-plat.c
>+++ b/drivers/usb/cdns3/cdns3-plat.c
>@@ -256,9 +256,10 @@ static int cdns3_controller_resume(struct device
>*dev, pm_message_t msg)
> 	cdns3_set_platform_suspend(cdns->dev, false, false);
>
> 	spin_lock_irqsave(&cdns->lock, flags);
>-	cdns_resume(cdns, !PMSG_IS_AUTO(msg));
>+	cdns_resume(cdns);
> 	cdns->in_lpm = false;
> 	spin_unlock_irqrestore(&cdns->lock, flags);
>+	cdns_set_active(cdns, !PMSG_IS_AUTO(msg));
> 	if (cdns->wakeup_pending) {
> 		cdns->wakeup_pending = false;
> 		enable_irq(cdns->wakeup_irq);
>diff --git a/drivers/usb/cdns3/cdnsp-pci.c b/drivers/usb/cdns3/cdnsp-pci.c
>index 7b151f5af3cc..0725668ffea4 100644
>--- a/drivers/usb/cdns3/cdnsp-pci.c
>+++ b/drivers/usb/cdns3/cdnsp-pci.c
>@@ -208,8 +208,9 @@ static int __maybe_unused cdnsp_pci_resume(struct
>device *dev)
> 	int ret;
>
> 	spin_lock_irqsave(&cdns->lock, flags);
>-	ret = cdns_resume(cdns, 1);
>+	ret = cdns_resume(cdns);
> 	spin_unlock_irqrestore(&cdns->lock, flags);
>+	cdns_set_active(cdns, 1);
>
> 	return ret;
> }
>diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c index
>dbcdf3b24b47..661d5597fb68 100644
>--- a/drivers/usb/cdns3/core.c
>+++ b/drivers/usb/cdns3/core.c
>@@ -522,7 +522,7 @@ int cdns_suspend(struct cdns *cdns)  }
>EXPORT_SYMBOL_GPL(cdns_suspend);
>
>-int cdns_resume(struct cdns *cdns, u8 set_active)
>+int cdns_resume(struct cdns *cdns)
> {
> 	struct device *dev = cdns->dev;
> 	enum usb_role real_role;
>@@ -556,15 +556,23 @@ int cdns_resume(struct cdns *cdns, u8 set_active)
> 	if (cdns->roles[cdns->role]->resume)
> 		cdns->roles[cdns->role]->resume(cdns,
>cdns_power_is_lost(cdns));
>
>+	return 0;
>+}
>+EXPORT_SYMBOL_GPL(cdns_resume);
>+
>+void cdns_set_active(struct cdns *cdns, u8 set_active) {
>+	struct device *dev = cdns->dev;
>+
> 	if (set_active) {
> 		pm_runtime_disable(dev);
> 		pm_runtime_set_active(dev);
> 		pm_runtime_enable(dev);
> 	}
>
>-	return 0;
>+	return;
> }
>-EXPORT_SYMBOL_GPL(cdns_resume);
>+EXPORT_SYMBOL_GPL(cdns_set_active);
> #endif /* CONFIG_PM_SLEEP */
>
> MODULE_AUTHOR("Peter Chen <peter.chen@....com>"); diff --git
>a/drivers/usb/cdns3/core.h b/drivers/usb/cdns3/core.h index
>2d332a788871..0f429042f997 100644
>--- a/drivers/usb/cdns3/core.h
>+++ b/drivers/usb/cdns3/core.h
>@@ -125,8 +125,9 @@ int cdns_init(struct cdns *cdns);  int
>cdns_remove(struct cdns *cdns);
>
> #ifdef CONFIG_PM_SLEEP
>-int cdns_resume(struct cdns *cdns, u8 set_active);
>+int cdns_resume(struct cdns *cdns);
> int cdns_suspend(struct cdns *cdns);
>+void cdns_set_active(struct cdns *cdns, u8 set_active);
> #else /* CONFIG_PM_SLEEP */
> static inline int cdns_resume(struct cdns *cdns, u8 set_active)  { return 0; }
>--
>2.25.1