> -----Original Message-----
> From: Xiaolei Wang <[email protected]>
> Sent: 2024??5??22?? 10:13
> To: Wei Fang <[email protected]>; Shenwei Wang
> <[email protected]>; Clark Wang <[email protected]>;
> [email protected]; [email protected]; [email protected];
> [email protected]
> Cc: [email protected]; [email protected];
> [email protected]
> Subject: [net PATCH] net: fec: free fec queue when fec_enet_mii_init() fails
>
> commit 63e3cc2b87c2 ("arm64: dts: imx93-11x11-evk: add
> reset gpios for ethernet PHYs") the rese-gpios attribute
nit: reset-gpios
> is added, but this pcal6524 is loaded later, which causes
> fec driver defer, the following memory leak occurs.
>
> unreferenced object 0xffffff8010350000 (size 8192):
> comm "kworker/u8:3", pid 39, jiffies 4294893562
> hex dump (first 32 bytes):
> 02 00 00 00 00 00 00 00 00 50 06 8a c0 ff ff ff .........P......
> e0 6f 06 8a c0 ff ff ff 00 50 06 8a c0 ff ff ff .o.......P......
> backtrace (crc f1b8b79f):
> [<0000000057d2c6ae>] kmemleak_alloc+0x34/0x40
> [<000000003c413e60>] kmalloc_trace+0x2f8/0x460
> [<00000000663f64e6>] fec_probe+0x1364/0x3a04
> [<0000000024d7e427>] platform_probe+0xc4/0x198
> [<00000000293aa124>] really_probe+0x17c/0x4f0
> [<00000000dfd1e0f3>] __driver_probe_device+0x158/0x2c4
> [<000000004ae0034a>] driver_probe_device+0x60/0x18c
> [<00000000fa3ad0e1>] __device_attach_driver+0x168/0x208
> [<00000000394a38d3>] bus_for_each_drv+0x104/0x190
> [<00000000c44e3dea>] __device_attach+0x1f8/0x33c
> [<000000004db69c14>] device_initial_probe+0x14/0x20
> [<00000000f4705309>] bus_probe_device+0x128/0x158
> [<00000000f7115919>] deferred_probe_work_func+0x12c/0x1d8
> [<0000000012315b3b>] process_scheduled_works+0x6c0/0x164c
> [<0000000089b2b6e1>] worker_thread+0x370/0x95c
> [<000000004dbe3d1a>] kthread+0x360/0x420
>
> Fixes: 63e3cc2b87c2 ("arm64: dts: imx93-11x11-evk: add reset gpios for
> ethernet PHYs")
I don't think this commit should be blamed, because this issue should
have existed earlier, it is just pcal6524 driver is loaded later than the
fec driver, which triggers this issue.
The commit 59d0f7465644 ("net: fec: init multi queue date structure")
was the first to introduce this issue, commit 619fee9eb13b
("net: fec: fix the potential memory leak in fec_enet_init() ")
fixed this, but it does not seem to be completely fixed.
We should find the correct blamed commit so that stable maintainers
can backport to the correct kernel version.
> Signed-off-by: Xiaolei Wang <[email protected]>
> ---
> drivers/net/ethernet/freescale/fec_main.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/net/ethernet/freescale/fec_main.c
> b/drivers/net/ethernet/freescale/fec_main.c
> index a72d8a2eb0b3..2b3534d434d8 100644
> --- a/drivers/net/ethernet/freescale/fec_main.c
> +++ b/drivers/net/ethernet/freescale/fec_main.c
> @@ -4524,6 +4524,7 @@ fec_probe(struct platform_device *pdev)
> fec_enet_mii_remove(fep);
> failed_mii_init:
> failed_irq:
> + fec_enet_free_queue(ndev);
> failed_init:
> fec_ptp_stop(pdev);
> failed_reset:
> --
> 2.25.1
> The commit 59d0f7465644 ("net: fec: init multi queue date structure")
> was the first to introduce this issue, commit 619fee9eb13b
> ("net: fec: fix the potential memory leak in fec_enet_init() ")
> fixed this, but it does not seem to be completely fixed.
This fix is also not great, and i would say the initial design is
really the problem. There needs to be a function which is the opposite
of fec_enet_init(). It can then be called in the probe cleanup code,
and in fec_drv_remove() which also appears to leak the queues.
Andrew
> -----Original Message-----
> From: Andrew Lunn <[email protected]>
> Sent: 2024??5??22?? 11:15
> To: Wei Fang <[email protected]>
> Cc: Xiaolei Wang <[email protected]>; Shenwei Wang
> <[email protected]>; Clark Wang <[email protected]>;
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]
> Subject: Re: [net PATCH] net: fec: free fec queue when fec_enet_mii_init() fails
>
> > The commit 59d0f7465644 ("net: fec: init multi queue date structure")
> > was the first to introduce this issue, commit 619fee9eb13b
> > ("net: fec: fix the potential memory leak in fec_enet_init() ") fixed
> > this, but it does not seem to be completely fixed.
>
> This fix is also not great, and i would say the initial design is really the problem.
> There needs to be a function which is the opposite of fec_enet_init(). It can
> then be called in the probe cleanup code, and in fec_drv_remove() which also
> appears to leak the queues.
>
Yes, this issue also exists when the fec driver is unbound, maybe Xiaolei can
help improve it in his patch.