When hwmon_device_register_with_info() fails in sfp_hwmon_probe(),
the string allocated by hwmon_sanitize_name() is not freed, leaking
memory on repeated hotplug/probe attempts.

Also, sfp_hwmon_remove() only freed hwmon_name inside the
IS_ERR_OR_NULL(hwmon_dev) block, so a partial-init failure could
leave the name allocation behind.

Fix by freeing hwmon_name and clearing both pointers on registration
failure, and by freeing hwmon_name unconditionally in remove while
still only unregistering hwmon_dev when it is valid.

Fixes: 3f118c449c8e ("net: sfp: use hwmon_sanitize_name()")
Signed-off-by: Krishan Singh <krishanmohan298@gmail.com>
---
 drivers/net/phy/sfp.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/sfp.c b/drivers/net/phy/sfp.c
index f52020673..f605fb399 100644
--- a/drivers/net/phy/sfp.c
+++ b/drivers/net/phy/sfp.c
@@ -1895,9 +1895,13 @@ static void sfp_hwmon_probe(struct work_struct *work)
 							 sfp->hwmon_name, sfp,
 							 &sfp_hwmon_chip_info,
 							 NULL);
-	if (IS_ERR(sfp->hwmon_dev))
+	if (IS_ERR(sfp->hwmon_dev)) {
 		dev_err(sfp->dev, "failed to register hwmon device: %ld\n",
 			PTR_ERR(sfp->hwmon_dev));
+		kfree(sfp->hwmon_name);
+		sfp->hwmon_name = NULL;
+		sfp->hwmon_dev  = NULL;
+	 }
 }
 
 static int sfp_hwmon_insert(struct sfp *sfp)
-- 
2.34.1

On Sat, Aug 01, 2026 at 12:27:03PM +0530, Krishan Singh wrote:
> When hwmon_device_register_with_info() fails in sfp_hwmon_probe(),
> the string allocated by hwmon_sanitize_name() is not freed, leaking
> memory on repeated hotplug/probe attempts.

Have you actually seen this, or is this theoretical?

I'm not too familiar with this code, but what i find interesting is
that sfp_hwmon_probe() is a void function. It has no way to say the
probe has failed. So how does the caller know it failed, and it should
not call sfp_hwmon_remove()? I suspect sfp_hwmon_remove() is always
called. So sfp_hwmon_remove() should be changed to release the
hwmon_name if it is not NULL.


    Andrew

---
pw-bot: cr

I have looked into the code for sfp_hwmon_remove calls to the 
best of my abilities and Andrew is right that sfp_hwmon_remove 
is called on every sfp device. 

If sfp_hwmon_probe fails in hwmon_device_register_with_info function call,
it leaves sfp->hwmon_dev as an error pointer. When sfp_check_state detects 
that SFP_F_PRESENT has changed, it sends SFP_E_REMOVE which leads to sfp_hwmon_remove  
call. In sfp_hwmon_remove sfp->hwmon_name is not freed because
!IS_ERR_OR_NULL(sfp->hwmon_dev) is false therefore the whole cleanup block is skipped.

So, as Andrew suggested, freeing sfp->hwmon_name if it is not NULL
and it is not an error pointer (hwmon_sanitize_name can return an error 
pointer) instead of freeing only when sfp->hwmon_dev is valid seems
to be the correct approach.