linux-kernel - Re: [PATCH AUTOSEL 5.15 10/19] 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: <6738b9d9-c172-8323-a144-79f7ae46bd0f@windriver.com>
Date:   Tue, 12 Sep 2023 10:39:10 +0800
From:   wangxiaolei <xiaolei.wang@...driver.com>
To:     Pavel Machek <pavel@...x.de>, Sasha Levin <sashal@...nel.org>
Cc:     linux-kernel@...r.kernel.org, stable@...r.kernel.org,
        Peter Chen <peter.chen@...nel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        pawell@...ence.com, linux-usb@...r.kernel.org
Subject: Re: [PATCH AUTOSEL 5.15 10/19] usb: cdns3: Put the cdns set active
 part outside the spin lock


On 9/11/23 6:00 PM, Pavel Machek wrote:
> Hi!
>
>> From: Xiaolei Wang <xiaolei.wang@...driver.com>
>>
>> [ Upstream commit 2319b9c87fe243327285f2fefd7374ffd75a65fc ]
>>
>> 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:
> There's something wrong with this patch: cdns_set_active returns
> either void or int depending on config. That can't be intentional.

Thanks for the reminder, I will send a new patch to fix this problem

thanks

xiaolei

>
> Best regards,
> 								Pavel
>
>> +++ b/drivers/usb/cdns3/core.c
>> @@ -556,15 +555,23 @@ int cdns_resume(struct cdns *cdns, u8 set_active)
> ...
>> +
>> +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;
>>   }
>> +++ b/drivers/usb/cdns3/core.h
>> @@ -125,10 +125,13 @@ int cdns_init(struct cdns *cdns);
>>   int cdns_remove(struct cdns *cdns);
>>   
>>   #ifdef CONFIG_PM_SLEEP
> ...
>>   int cdns_suspend(struct cdns *cdns);
>> +void cdns_set_active(struct cdns *cdns, u8 set_active);
>>   #else /* CONFIG_PM_SLEEP */
> ...
>> +static inline int cdns_set_active(struct cdns *cdns, u8 set_active)
>>   { return 0; }
>>   static inline int cdns_suspend(struct cdns *cdns)
>>   { return 0; }