Re: [PATCH v2 4/4] platform/x86/amd/hsmp: Ensure initialization of the sock struct in hsmp_send_message()

[Suma Hegde <Suma.Hegde@amd.com>]

Hi Ilpo,


On 7/24/2025 2:50 PM, Suma Hegde wrote:
> Caution: This message originated from an External Source. Use proper 
> caution when opening attachments, clicking links, or responding.
>
>
> Hi Ilpo,
>
>
> On 7/23/2025 12:40 PM, Ilpo Järvinen wrote:
>> Caution: This message originated from an External Source. Use proper 
>> caution when opening attachments, clicking links, or responding.
>>
>>
>> On Wed, 23 Jul 2025, Suma Hegde wrote:
>>
>>> If all sockets are not probed, invoking hsmp_send_message() might 
>>> result in
>>> unexpected behavior due to accessing an uninitialized socket structure.
>>>
>>> The initialization of the sock structure can be confirmed if sock->dev
>>> is initialized.
>>>
>>> Signed-off-by: Suma Hegde <suma.hegde@amd.com>
>>> Reviewed-by: Naveen Krishna Chatradhi <naveenkrishna.chatradhi@amd.com>
>>> ---
>>> Changes since v1:
>>> New patch to address the partial probe/removal issue.
>>>
>>>   drivers/platform/x86/amd/hsmp/acpi.c | 4 ++++
>>>   drivers/platform/x86/amd/hsmp/hsmp.c | 3 +++
>>>   2 files changed, 7 insertions(+)
>>>
>>> diff --git a/drivers/platform/x86/amd/hsmp/acpi.c 
>>> b/drivers/platform/x86/amd/hsmp/acpi.c
>>> index 15c4cedc2759..a2d91d4a3e02 100644
>>> --- a/drivers/platform/x86/amd/hsmp/acpi.c
>>> +++ b/drivers/platform/x86/amd/hsmp/acpi.c
>>> @@ -617,6 +617,10 @@ static int hsmp_acpi_probe(struct 
>>> platform_device *pdev)
>>>
>>>   static void hsmp_acpi_remove(struct platform_device *pdev)
>>>   {
>>> +     struct hsmp_socket *sock = dev_get_drvdata(&pdev->dev);
>>> +
>>> +     sock->dev = NULL;
>>> +
>>>        guard(mutex)(&hsmp_lock);
>>>        /*
>>>         * We register only one misc_device even on multi-socket system.
>>> diff --git a/drivers/platform/x86/amd/hsmp/hsmp.c 
>>> b/drivers/platform/x86/amd/hsmp/hsmp.c
>>> index e05d824045d6..a4420db42781 100644
>>> --- a/drivers/platform/x86/amd/hsmp/hsmp.c
>>> +++ b/drivers/platform/x86/amd/hsmp/hsmp.c
>>> @@ -219,6 +219,9 @@ int hsmp_send_message(struct hsmp_message *msg)
>>>                return -ENODEV;
>>>        sock = &hsmp_pdev.sock[msg->sock_ind];
>>>
>>> +     if (!sock->dev)
>>> +             return -ENODEV;
>>> +
>>>        ret = down_interruptible(&sock->hsmp_sem);
>>>        if (ret < 0)
>>>                return ret;
>> This is still racy. AFAICT, nothing prevents assigning NULL into 
>> sock->dev
>> because of remove while hsmp_send_message() is running and that can 
>> result
>> in dereferencing NULL.
>
> Could you please suggest on how to resolve this issue?
>
>

I will relocate hsmp_fops to acpi.c and plat.c from hsmp.c. 
Consequently, the module reference

count for hsmp_acpi should be incremented when the device file is opened.

Does this solution address the issue adequately? Is it acceptable?


>> -- 
>>   i.
>
>
> Thanks and Regards,
>
> Suma
>
>