netdev - Re: [PATCH v3] net: dsa: bcm_sf2: Fix possible memory leak in bcm_sf2_mdio

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: <20231013114641.GG29570@kernel.org>
Date: Fri, 13 Oct 2023 13:46:41 +0200
From: Simon Horman <horms@...nel.org>
To: Jinjie Ruan <ruanjinjie@...wei.com>
Cc: netdev@...r.kernel.org,
	Florian Fainelli <florian.fainelli@...adcom.com>,
	Andrew Lunn <andrew@...n.ch>, Vladimir Oltean <olteanv@...il.com>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Vivien Didelot <vivien.didelot@...il.com>
Subject: Re: [PATCH v3] net: dsa: bcm_sf2: Fix possible memory leak in
 bcm_sf2_mdio_register()

On Wed, Oct 11, 2023 at 11:24:19AM +0800, Jinjie Ruan wrote:
> In bcm_sf2_mdio_register(), the class_find_device() will call get_device()
> to increment reference count for priv->master_mii_bus->dev if
> of_mdio_find_bus() succeeds. If mdiobus_alloc() or mdiobus_register()
> fails, it will call get_device() twice without decrement reference count
> for the device. And it is the same if bcm_sf2_mdio_register() succeeds but
> fails in bcm_sf2_sw_probe(), or if bcm_sf2_sw_probe() succeeds. If the
> reference count has not decremented to zero, the dev related resource will
> not be freed.
> 
> So remove the get_device() in bcm_sf2_mdio_register(), and call
> put_device() if mdiobus_alloc() or mdiobus_register() fails and in
> bcm_sf2_mdio_unregister() to solve the issue.
> 
> And as Simon suggested, unwind from errors for bcm_sf2_mdio_register() and
> just return 0 if it succeeds to make it cleaner.
> 
> Fixes: 461cd1b03e32 ("net: dsa: bcm_sf2: Register our slave MDIO bus")
> Signed-off-by: Jinjie Ruan <ruanjinjie@...wei.com>
> Suggested-by: Simon Horman <horms@...nel.org>

Thanks, this addresses my concerns around v2.

Reviewed-by: Simon Horman <horms@...nel.org>