[PATCH v3 2/4] soc: amlogic: meson-gx-socinfo-sm: Add Amlogic secure-monitor SoC Information driver

[Viacheslav]

14/03/2024 16.31, Krzysztof Kozlowski wrote:
> On 14/03/2024 13:22, Viacheslav wrote:
>>> +
>>>> +	soc_dev_attr = devm_kzalloc(&pdev->dev, sizeof(*soc_dev_attr),
>>>> +				    GFP_KERNEL);
>>>> +	if (!soc_dev_attr)
>>>> +		return -ENOMEM;
>>>> +
>>>> +	soc_dev_attr->serial_number = socinfo_get_chipid(&pdev->dev, fw, &socinfo);
>>>> +
>>>> +	soc_dev_attr->family = "Amlogic Meson";
>>>> +	soc_dev_attr->revision = kasprintf(GFP_KERNEL, "%x:%x - %x:%x",
>>>> +					   socinfo.v1.major_id,
>>>> +					   socinfo.v1.chip_rev,
>>>> +					   socinfo.v1.pack_id,
>>>> +					   (socinfo.v1.reserved<<4) + socinfo.v1.layout_ver);
>>>> +	soc_dev_attr->soc_id = kasprintf(GFP_KERNEL, "%s (%s)",
>>>> +					 socinfo_v1_to_soc_id(socinfo),
>>>> +					 socinfo_v1_to_package_id(socinfo));
>>>> +
>>>> +	soc_dev = soc_device_register(soc_dev_attr);
>>>> +
>>>> +
>>>> +	if (IS_ERR(soc_dev)) {
>>>> +		kfree(soc_dev_attr->revision);
>>>> +		kfree_const(soc_dev_attr->soc_id);
>>>> +		kfree(soc_dev_attr);
>>>
>>> That's a double free. This was not tested.
>>
>>
>> Please, describe the problem.
> 
> Test your code. What's the point of arguing over it if regular test
> would show this?
> 
>> I don't quite understand what the issue is:
>>
>> - kfree() releases memory allocated with kmalloc()
> 
> So point me where is kmalloc(). I don't see. I see only devm.

I missed the point that devm_kzalloc is automatically freed. You are 
right. Thanks!


-- 
Best regards
Viacheslav