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

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <c0352b99-ca25-4357-bc26-470d802dbf71@lexina.in>
Date: Thu, 14 Mar 2024 16:59:09 +0300
From: Viacheslav <adeep@...ina.in>
To: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
 Neil Armstrong <neil.armstrong@...aro.org>,
 Kevin Hilman <khilman@...libre.com>, Jerome Brunet <jbrunet@...libre.com>,
 Martin Blumenstingl <martin.blumenstingl@...glemail.com>,
 linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
 linux-amlogic@...ts.infradead.org
Cc: Rob Herring <robh+dt@...nel.org>,
 Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>
Subject: Re: [PATCH v3 2/4] soc: amlogic: meson-gx-socinfo-sm: Add Amlogic
 secure-monitor SoC Information driver



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