linux-kernel - Re: [PATCH] crypto: tegra - Remove an incorrect iommu_fwspec_free() call in tegra_se

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]

Date: Fri, 31 May 2024 22:04:52 +0200
From: Marion & Christophe JAILLET <christophe.jaillet@...adoo.fr>
To: Akhil R <akhilrajeev@...dia.com>, Herbert Xu <herbert@...dor.apana.org.au>
Cc: "David S. Miller" <davem@...emloft.net>,
 Thierry Reding <thierry.reding@...il.com>, Jon Hunter
 <jonathanh@...dia.com>,
 "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
 "kernel-janitors@...r.kernel.org" <kernel-janitors@...r.kernel.org>,
 "linux-crypto@...r.kernel.org" <linux-crypto@...r.kernel.org>,
 "linux-tegra@...r.kernel.org" <linux-tegra@...r.kernel.org>
Subject: Re: [PATCH] crypto: tegra - Remove an incorrect iommu_fwspec_free()
 call in tegra_se_remove()


Le 31/05/2024 à 07:36, Akhil R a écrit :
>> -----Original Message-----
>> From: Herbert Xu <herbert@...dor.apana.org.au>
>> Sent: Friday, May 31, 2024 10:53 AM
>> To: Akhil R <akhilrajeev@...dia.com>
>> Cc: Christophe JAILLET <christophe.jaillet@...adoo.fr>; David S. Miller
>> <davem@...emloft.net>; Thierry Reding <thierry.reding@...il.com>; Jon
>> Hunter <jonathanh@...dia.com>; linux-kernel@...r.kernel.org; kernel-
>> janitors@...r.kernel.org; linux-crypto@...r.kernel.org; linux-
>> tegra@...r.kernel.org
>> Subject: Re: [PATCH] crypto: tegra - Remove an incorrect iommu_fwspec_free()
>> call in tegra_se_remove()
>>
>> External email: Use caution opening links or attachments
>>
>>
>> On Wed, May 29, 2024 at 06:53:42AM +0000, Akhil R wrote:
>>>> The only iommu function call in this driver is a
>>>> tegra_dev_iommu_get_stream_id() which does not allocate anything and
>> does
>>>> not take any reference.
>>>>
>>>> More-over, what is freed is "se->dev" which has been devm_kzalloc()'ed in the
>>>> probe.
>>> I did not completely understand what is being tried to convey here.
>>> If I understand it right, iommu_fwspec_free() does not do anything
>>> with the "devm_kzalloc"ed variable.
>>>
>>> It would probably be a good idea to remove this line from the commit message.
>> I think he means that as the memory was allocated via devm, it will
>> be automatically freed by the kernel and the driver does not need
>> to (and should not) free the memory by hand.


Yes, that is my point.

> Ya. But iommu_fwspec_free() does not free the memory allocated via devm.
>
> I think iommu_fwspec_free() is expected to be called in symmetry with
> iommu_fwspec_init(). So, I do agree that the SE driver does not allocate
> what is freed by iommu_fwspec_free(), but I feel this line is a bit misleading.
>
Yes, I spoke too fast.
What is freed is dev_iommu_fwspec_get(dev);, not dev. So the sentence I 
wrote makes no sense and should be removed :(


CJ