RE: net: fec: memory corruption caused by err_enet_mii_probe error path

Hi Kegl,

Removing Andy Duan, since he has left NXP, and mail is unavailable any longer, to avoid noise.

-----Original Message-----
From: Kegl Rohit <redacted>
Sent: 2021年12月22日 15:06
To: netdev <redacted>
Cc: Andy Duan <redacted>
Subject: net: fec: memory corruption caused by err_enet_mii_probe error
path

Hello!

There is an issue with the error path of fec_enet_mii_probe in
fec_enet_open(struct net_device *ndev) which leads to random memory
corruption.

In open() the buffers are initialized:
https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithu
b.com%2Ftorvalds%2Flinux%2Fblob%2Fv5.10%2Fdrivers%2Fnet%2Fethernet
%2Ffreescale%2Ffec_main.c%23L3001&amp;data=04%7C01%7Cqiangqing.zh
ang%40nxp.com%7Cc7511826d9684e2b9d3508d9c51991d0%7C686ea1d3bc2b
4c6fa92cd99c5c301635%7C0%7C1%7C637757535872985607%7CUnknown%7C
TWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiL
CJXVCI6Mn0%3D%7C3000&amp;sdata=FovOc4z3RIuatEraWWO8lgco8sIea8B0
3RVKNd83QZc%3D&amp;reserved=0

Then fec_restart will start the DMA engines.
https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithu
b.com%2Ftorvalds%2Flinux%2Fblob%2Fv5.10%2Fdrivers%2Fnet%2Fethernet
%2Ffreescale%2Ffec_main.c%23L3006&amp;data=04%7C01%7Cqiangqing.zh
ang%40nxp.com%7Cc7511826d9684e2b9d3508d9c51991d0%7C686ea1d3bc2b
4c6fa92cd99c5c301635%7C0%7C1%7C637757535872985607%7CUnknown%7C
TWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiL
CJXVCI6Mn0%3D%7C3000&amp;sdata=FAEb9MAGVmVYv4gdiJrwhT7k4Pys8
UlRLnydpAJ6N94%3D&amp;reserved=0

Now if fec_enet_mii_probe fails (e.g. phy did not respond via mii) the
err_enet_mii_probe error path will be used
https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithu
b.com%2Ftorvalds%2Flinux%2Fblob%2Fv5.10%2Fdrivers%2Fnet%2Fethernet
%2Ffreescale%2Ffec_main.c%23L3031&amp;data=04%7C01%7Cqiangqing.zh
ang%40nxp.com%7Cc7511826d9684e2b9d3508d9c51991d0%7C686ea1d3bc2b
4c6fa92cd99c5c301635%7C0%7C1%7C637757535872985607%7CUnknown%7C
TWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiL
CJXVCI6Mn0%3D%7C3000&amp;sdata=QLT9%2BcOkH7%2BdRSZzdIzp6oq0rc
wnOy0vfVN7Sa4YTAM%3D&amp;reserved=0

err_enet_mii_probe:
fec_enet_free_buffers(ndev);
err_enet_alloc:
fec_enet_clk_enable(ndev, false);
clk_enable:
pm_runtime_mark_last_busy(&fep->pdev->dev);
pm_runtime_put_autosuspend(&fep->pdev->dev);
pinctrl_pm_select_sleep_state(&fep->pdev->dev);
return ret;

This error path frees the DMA buffers, BUT as far I could see it does not stop
the DMA engines.
=> open() fails => frees buffers => DMA still active => MAC receives network
packet => DMA starts => random memory corruption (use after
free) => random kernel panics

A question here, why receive path still active? MAC has not connected to PHY when this failure happened, should not see network activities.

So maybe fec_stop() as counterpart to fec_restart() is missing before freeing
the buffers?
err_enet_mii_probe:
fec_stop(ndev);
fec_enet_free_buffers(ndev);

Issue happend with 5.10.83 and should also also happen with current master.

It's fine for me, please see if anyone else has some comments. If not, please cook a formal patch, thanks.

Best Regards,
Joakim Zhang