Commit 633efc8b authored by Chen Zhongjin's avatar Chen Zhongjin Committed by David S. Miller

net: dsa: Fix possible memory leaks in dsa_loop_init()

kmemleak reported memory leaks in dsa_loop_init():

kmemleak: 12 new suspected memory leaks

unreferenced object 0xffff8880138ce000 (size 2048):
  comm "modprobe", pid 390, jiffies 4295040478 (age 238.976s)
  backtrace:
    [<000000006a94f1d5>] kmalloc_trace+0x26/0x60
    [<00000000a9c44622>] phy_device_create+0x5d/0x970
    [<00000000d0ee2afc>] get_phy_device+0xf3/0x2b0
    [<00000000dca0c71f>] __fixed_phy_register.part.0+0x92/0x4e0
    [<000000008a834798>] fixed_phy_register+0x84/0xb0
    [<0000000055223fcb>] dsa_loop_init+0xa9/0x116 [dsa_loop]
    ...

There are two reasons for memleak in dsa_loop_init().

First, fixed_phy_register() create and register phy_device:

fixed_phy_register()
  get_phy_device()
    phy_device_create() # freed by phy_device_free()
  phy_device_register() # freed by phy_device_remove()

But fixed_phy_unregister() only calls phy_device_remove().
So the memory allocated in phy_device_create() is leaked.

Second, when mdio_driver_register() fail in dsa_loop_init(),
it just returns and there is no cleanup for phydevs.

Fix the problems by catching the error of mdio_driver_register()
in dsa_loop_init(), then calling both fixed_phy_unregister() and
phy_device_free() to release phydevs.
Also add a function for phydevs cleanup to avoid duplacate.

Fixes: 98cd1552 ("net: dsa: Mock-up driver")
Signed-off-by: default avatarChen Zhongjin <chenzhongjin@huawei.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 23758867
...@@ -376,6 +376,17 @@ static struct mdio_driver dsa_loop_drv = { ...@@ -376,6 +376,17 @@ static struct mdio_driver dsa_loop_drv = {
#define NUM_FIXED_PHYS (DSA_LOOP_NUM_PORTS - 2) #define NUM_FIXED_PHYS (DSA_LOOP_NUM_PORTS - 2)
static void dsa_loop_phydevs_unregister(void)
{
unsigned int i;
for (i = 0; i < NUM_FIXED_PHYS; i++)
if (!IS_ERR(phydevs[i])) {
fixed_phy_unregister(phydevs[i]);
phy_device_free(phydevs[i]);
}
}
static int __init dsa_loop_init(void) static int __init dsa_loop_init(void)
{ {
struct fixed_phy_status status = { struct fixed_phy_status status = {
...@@ -383,23 +394,23 @@ static int __init dsa_loop_init(void) ...@@ -383,23 +394,23 @@ static int __init dsa_loop_init(void)
.speed = SPEED_100, .speed = SPEED_100,
.duplex = DUPLEX_FULL, .duplex = DUPLEX_FULL,
}; };
unsigned int i; unsigned int i, ret;
for (i = 0; i < NUM_FIXED_PHYS; i++) for (i = 0; i < NUM_FIXED_PHYS; i++)
phydevs[i] = fixed_phy_register(PHY_POLL, &status, NULL); phydevs[i] = fixed_phy_register(PHY_POLL, &status, NULL);
return mdio_driver_register(&dsa_loop_drv); ret = mdio_driver_register(&dsa_loop_drv);
if (ret)
dsa_loop_phydevs_unregister();
return ret;
} }
module_init(dsa_loop_init); module_init(dsa_loop_init);
static void __exit dsa_loop_exit(void) static void __exit dsa_loop_exit(void)
{ {
unsigned int i;
mdio_driver_unregister(&dsa_loop_drv); mdio_driver_unregister(&dsa_loop_drv);
for (i = 0; i < NUM_FIXED_PHYS; i++) dsa_loop_phydevs_unregister();
if (!IS_ERR(phydevs[i]))
fixed_phy_unregister(phydevs[i]);
} }
module_exit(dsa_loop_exit); module_exit(dsa_loop_exit);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment