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: <a6d7efe2-ec92-4ffa-a1f1-bc73ebd49d16@icloud.com>
Date: Tue, 29 Oct 2024 23:35:48 +0800
From: Zijun Hu <zijun_hu@...oud.com>
To: Johan Hovold <johan@...nel.org>
Cc: Vinod Koul <vkoul@...nel.org>, Kishon Vijay Abraham I
 <kishon@...nel.org>, Felipe Balbi <balbi@...com>,
 Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
 Rob Herring <robh@...nel.org>, Arnd Bergmann <arnd@...db.de>,
 Lee Jones <lee@...nel.org>, Lorenzo Pieralisi <lpieralisi@...nel.org>,
 Krzysztof WilczyƄski <kw@...ux.com>,
 Bjorn Helgaas <bhelgaas@...gle.com>, "David S. Miller"
 <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
 Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
 Christophe JAILLET <christophe.jaillet@...adoo.fr>, stable@...r.kernel.org,
 linux-phy@...ts.infradead.org, linux-kernel@...r.kernel.org,
 Zijun Hu <quic_zijuhu@...cinc.com>
Subject: Re: [PATCH v2 2/6] phy: core: Fix that API
 devm_of_phy_provider_unregister() fails to unregister the phy provider

On 2024/10/29 21:43, Johan Hovold wrote:
> On Thu, Oct 24, 2024 at 10:39:27PM +0800, Zijun Hu wrote:
>> From: Zijun Hu <quic_zijuhu@...cinc.com>
>>
>> For devm_of_phy_provider_unregister(), its comment says it needs to invoke
>> of_phy_provider_unregister() to unregister the phy provider, but it does
>> not invoke the function actually since devres_destroy() will not call
>> devm_phy_provider_release() at all which will call the function, and the
>> missing of_phy_provider_unregister() call will case:
> 
> Please split this up in two sentences as well.
> 

good suggestions. will do it.

>> - The phy provider fails to be unregistered.
>> - Leak both memory and the OF node refcount.
> 
> Perhaps a comment about there not being any in-tree users of this API is
> in place here?
> 

okay, will do it as you suggest.

> And you could consider dropping the function altogether as well.
>

Remove the API devm_of_phy_provider_unregister()?

i prefer fixing it instead of removing it based on below considerations.

1) it is simper. just about one line change.
2) the API may be used in future. the similar API of [PATCH 1/6] have 2
usages.

>> Fixed by using devres_release() instead of devres_destroy() within the API.
>>
>> Fixes: ff764963479a ("drivers: phy: add generic PHY framework")
>> Cc: stable@...r.kernel.org
>> Signed-off-by: Zijun Hu <quic_zijuhu@...cinc.com>
> 
> Looks good otherwise.
> 
> Johan


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ