linux-kernel - Re: [PATCH] ACPI: processor: idle: Fix resource rollback in acpi_processor_power

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: <d768fd79-ef55-4d40-829f-7ca4749c8370@huawei.com>
Date: Tue, 22 Jul 2025 19:28:22 +0800
From: "lihuisong (C)" <lihuisong@...wei.com>
To: "Rafael J. Wysocki" <rafael@...nel.org>
CC: <lenb@...nel.org>, <linux-acpi@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>, <linuxarm@...wei.com>,
	<jonathan.cameron@...wei.com>, <zhanjie9@...ilicon.com>,
	<zhenglifeng1@...wei.com>, <yubowen8@...wei.com>, <liuyonglong@...wei.com>,
	<lihuisong@...wei.com>
Subject: Re: [PATCH] ACPI: processor: idle: Fix resource rollback in
 acpi_processor_power_init


在 2025/7/22 19:10, Rafael J. Wysocki 写道:
> On Mon, Jul 14, 2025 at 3:34 AM lihuisong (C) <lihuisong@...wei.com> wrote:
>>
>> 在 2025/7/3 19:09, Rafael J. Wysocki 写道:
>>> On Thu, Jul 3, 2025 at 8:23 AM lihuisong (C) <lihuisong@...wei.com> wrote:
>>>> Hi,
>>>>
>>>> Thanks for your review.
>>>>
>>>>
>>>> 在 2025/7/3 1:42, Rafael J. Wysocki 写道:
>>>>> On Thu, Jun 19, 2025 at 8:13 AM Huisong Li <lihuisong@...wei.com> wrote:
>>>>>> There are two resource rollback issues in acpi_processor_power_init:
>>>>>> 1> Do not unregister acpi_idle_driver when do kzalloc failed.
>>>>>> 2> Do not free cpuidle device memory when register cpuidle device failed.
>>>>>>
>>>>>> Signed-off-by: Huisong Li <lihuisong@...wei.com>
>>>>>> ---
>>>>>>     drivers/acpi/processor_idle.c | 24 +++++++++++++++++-------
>>>>>>     1 file changed, 17 insertions(+), 7 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
>>>>>> index 2c2dc559e0f8..3548ab9dac9e 100644
>>>>>> --- a/drivers/acpi/processor_idle.c
>>>>>> +++ b/drivers/acpi/processor_idle.c
>>>>>> @@ -1392,8 +1392,10 @@ int acpi_processor_power_init(struct acpi_processor *pr)
>>>>>>                    }
>>>>>>
>>>>>>                    dev = kzalloc(sizeof(*dev), GFP_KERNEL);
>>>>>> -               if (!dev)
>>>>>> -                       return -ENOMEM;
>>>>>> +               if (!dev) {
>>>>>> +                       retval = -ENOMEM;
>>>>>> +                       goto unregister_driver;
>>>>> No, unregistering the driver here is pointless.
>>>> I don't quite understand why here is pointless. Can you explain it?
>>> When this function is run for another CPU, it will attempt to register
>>> the driver again if it is unregistered here.
>> Yeah, got it.
>> So registering cpuidle also has a potential race issue here.
>>> Quite frankly, the driver should be registered before running this
>>> function because it is a CPU hotplug callback and registering a
>>> cpuidle driver from within it is quite questionable.
>>>
>>> Alternatively, it can be registered when all of the CPUs have been brought up.
>> Agree with you.
>> The reason why is that the initialization of acpi_idle_driver depands on
>> the power management information of CPU.
>> But the power management information of CPU is obtained in this callback.
>> I have an idea.
>> Because acpi_idle_driver is applied to all possiable CPUs. And use the
>> power information of the first onlined CPU to initialize and register
>> acpi_idle_driver, currently.
>> So I think we can use this logic and dependency to extract a function to
>> initialize and register acpi_idle_driver. And put this function to
>> acpi_processor_driver_init().
>> I tested it is ok.
>> What do you think about this?
> This is one of the options I mentioned above, isn't it?
Yes, it is what you mentioned. I think this option is relatively easier 
to handle.
I am not sure if I understand correctly.
So I wanted to confirmed with you.