From: Xulin Sun <hidden> Date: 2021-02-05 07:28:38
If the previous can_net device has been successfully allocated, its
private data structure is impossible to be empty, remove this redundant
error return judgment. Otherwise, memory leaks for alloc_candev() will
be triggered.
Signed-off-by: Xulin Sun <redacted>
---
drivers/net/can/m_can/m_can.c | 5 -----
1 file changed, 5 deletions(-)
From: Marc Kleine-Budde <mkl@pengutronix.de> Date: 2021-02-05 08:10:23
On 05.02.2021 15:25:58, Xulin Sun wrote:
The can_net device is allocated through kvzalloc(), if the subsequent probe
cases fail to initialize, it should free the can_net device that has been
successfully allocated before.
To fix below memory leaks call trace:
unreferenced object 0xfffffc08418b0000 (size 32768):
comm "kworker/0:1", pid 22, jiffies 4294893966 (age 931.976s)
hex dump (first 32 bytes):
63 61 6e 25 64 00 00 00 00 00 00 00 00 00 00 00 can%d...........
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
backtrace:
[<000000003faec9cc>] __kmalloc+0x1a4/0x3e0
[<00000000560b1cad>] kvmalloc_node+0xa0/0xb0
[<0000000093bada32>] alloc_netdev_mqs+0x60/0x380
[<0000000041ddbb53>] alloc_candev_mqs+0x6c/0x14c
[<00000000d08c7529>] m_can_class_allocate_dev+0x64/0x18c
[<000000009fef1617>] m_can_plat_probe+0x2c/0x1f4
[<000000006fdcc497>] platform_drv_probe+0x5c/0xb0
[<00000000fd0f0726>] really_probe+0xec/0x41c
[<000000003ffa5158>] driver_probe_device+0x60/0xf0
[<000000005986c77e>] __device_attach_driver+0xb0/0x100
[<00000000757823bc>] bus_for_each_drv+0x8c/0xe0
[<0000000059253919>] __device_attach+0xdc/0x180
[<0000000035c2b9f1>] device_initial_probe+0x28/0x34
[<0000000082e2c85c>] bus_probe_device+0xa4/0xb0
[<00000000cc6181c3>] deferred_probe_work_func+0x7c/0xb0
[<0000000001b85f22>] process_one_work+0x1ec/0x480
Signed-off-by: Xulin Sun <redacted>
This patch doesn't apply to net/master, since v5.10 there is a
similar fix:
85816aba460c can: m_can: Fix freeing of can device from peripherials
Please update to latest v5.10.x. If you're on a kernel that's still
supported, and you're using the latest stable of that kernel, and it
doesn't have that patch applied, ask on linux-stable to pick up that
patch.
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Embedded Linux | https://www.pengutronix.de |
Vertretung West/Dortmund | Phone: +49-231-2826-924 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
From: Marc Kleine-Budde <mkl@pengutronix.de> Date: 2021-02-05 08:20:29
On 05.02.2021 15:25:59, Xulin Sun wrote:
If the previous can_net device has been successfully allocated, its
private data structure is impossible to be empty, remove this redundant
error return judgment. Otherwise, memory leaks for alloc_candev() will
be triggered.
Your analysis is correct, the netdev_priv() will never fail. But how
will this trigger a mem leak on alloc_candev()? I've removed that
statement. I'll add it back, if I've missed something.
Signed-off-by: Xulin Sun <redacted>
Applied to linux-can-next/testing.
Thanks,
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Embedded Linux | https://www.pengutronix.de |
Vertretung West/Dortmund | Phone: +49-231-2826-924 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
From: Xulin Sun <hidden> Date: 2021-02-05 08:47:16
On 2021/2/5 下午4:19, Marc Kleine-Budde wrote:
On 05.02.2021 15:25:59, Xulin Sun wrote:
quoted
If the previous can_net device has been successfully allocated, its
private data structure is impossible to be empty, remove this redundant
error return judgment. Otherwise, memory leaks for alloc_candev() will
be triggered.
Your analysis is correct, the netdev_priv() will never fail. But how
will this trigger a mem leak on alloc_candev()? I've removed that
Hi Marc,
The previous code judges the netdev_priv is empty, and then goto out.
The correct approach should add free_candev(net_dev) before goto.
The code Like:
class_dev = netdev_priv(net_dev);
if (!class_dev) {
dev_err(dev, "Failed to init netdev cdevate");
+ free_candev(net_dev);
goto out;
}
Otherwise, memory leaks for alloc_candev() will be triggered.
Now directly remove the impossible error return judgment to resolve the above possible issue.
Thanks
Xulin
statement. I'll add it back, if I've missed something.
From: Marc Kleine-Budde <mkl@pengutronix.de> Date: 2021-02-05 08:53:23
On 05.02.2021 16:46:16, Xulin Sun wrote:
On 2021/2/5 下午4:19, Marc Kleine-Budde wrote:
quoted
On 05.02.2021 15:25:59, Xulin Sun wrote:
quoted
If the previous can_net device has been successfully allocated, its
private data structure is impossible to be empty, remove this redundant
error return judgment. Otherwise, memory leaks for alloc_candev() will
be triggered.
Your analysis is correct, the netdev_priv() will never fail. But how
will this trigger a mem leak on alloc_candev()? I've removed that
The previous code judges the netdev_priv is empty, and then goto out. The
correct approach should add free_candev(net_dev) before goto.
The code Like:
class_dev = netdev_priv(net_dev);
if (!class_dev) {
dev_err(dev, "Failed to init netdev cdevate");
+ free_candev(net_dev);
goto out;
}
Otherwise, memory leaks for alloc_candev() will be triggered.
No - as you said in the original patch description. The return value
of netdev_priv() cannot be NULL, as net_dev is not NULL.
Now directly remove the impossible error return judgment to resolve
the above possible issue.
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Embedded Linux | https://www.pengutronix.de |
Vertretung West/Dortmund | Phone: +49-231-2826-924 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |