.../stmicro/stmmac/dwmac-intel-plat.c | 25 +++++++++++++------ 1 file changed, 17 insertions(+), 8 deletions(-)
If the clock dwmac->tx_clk was not enabled in intel_eth_plat_probe,
it should not be disabled in any path.
Conversely, if it was enabled in intel_eth_plat_probe, it must be disabled
in all error paths to ensure proper cleanup.
Found by Linux Verification Center (linuxtesting.org) with Klever.
Fixes: 9efc9b2b04c7 ("net: stmmac: Add dwmac-intel-plat for GBE driver")
Signed-off-by: Vitalii Mordan <mordan@ispras.ru>
---
v2: Unwind using a goto label, as per Simon Horman's request.
.../stmicro/stmmac/dwmac-intel-plat.c | 25 +++++++++++++------
1 file changed, 17 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-intel-plat.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-intel-plat.c
index d68f0c4e7835..9739bc9867c5 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-intel-plat.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-intel-plat.c
@@ -108,7 +108,12 @@ static int intel_eth_plat_probe(struct platform_device *pdev)
if (IS_ERR(dwmac->tx_clk))
return PTR_ERR(dwmac->tx_clk);
- clk_prepare_enable(dwmac->tx_clk);
+ ret = clk_prepare_enable(dwmac->tx_clk);
+ if (ret) {
+ dev_err(&pdev->dev,
+ "Failed to enable tx_clk\n");
+ return ret;
+ }
/* Check and configure TX clock rate */
rate = clk_get_rate(dwmac->tx_clk);
@@ -119,7 +124,7 @@ static int intel_eth_plat_probe(struct platform_device *pdev)
if (ret) {
dev_err(&pdev->dev,
"Failed to set tx_clk\n");
- return ret;
+ goto err_tx_clk_disable;
}
}
}
@@ -133,7 +138,7 @@ static int intel_eth_plat_probe(struct platform_device *pdev)
if (ret) {
dev_err(&pdev->dev,
"Failed to set clk_ptp_ref\n");
- return ret;
+ goto err_tx_clk_disable;
}
}
}
@@ -149,12 +154,15 @@ static int intel_eth_plat_probe(struct platform_device *pdev)
}
ret = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
- if (ret) {
- clk_disable_unprepare(dwmac->tx_clk);
- return ret;
- }
+ if (ret)
+ goto err_tx_clk_disable;
return 0;
+
+err_tx_clk_disable:
+ if (dwmac->data->tx_clk_en)
+ clk_disable_unprepare(dwmac->tx_clk);
+ return ret;
}
static void intel_eth_plat_remove(struct platform_device *pdev)
@@ -162,7 +170,8 @@ static void intel_eth_plat_remove(struct platform_device *pdev)
struct intel_dwmac *dwmac = get_stmmac_bsp_priv(&pdev->dev);
stmmac_pltfr_remove(pdev);
- clk_disable_unprepare(dwmac->tx_clk);
+ if (dwmac->data->tx_clk_en)
+ clk_disable_unprepare(dwmac->tx_clk);
}
static struct platform_driver intel_eth_plat_driver = {
--
2.25.1Hi Vitalii,
kernel test robot noticed the following build warnings:
url: https://github.com/intel-lab-lkp/linux/commits/Vitalii-Mordan/stmmac-dwmac-intel-plat-fix-call-balance-of-tx_clk-handling-routines/20241109-013647
base: net/main
patch link: https://lore.kernel.org/r/20241108173334.2973603-1-mordan%40ispras.ru
patch subject: [PATCH net v2]: stmmac: dwmac-intel-plat: fix call balance of tx_clk handling routines
config: arm-randconfig-r071-20241110 (https://download.01.org/0day-ci/archive/20241111/202411110911.fxtHBKSw-lkp@intel.com/config)
compiler: arm-linux-gnueabi-gcc (GCC) 14.2.0
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
| Closes: https://lore.kernel.org/r/202411110911.fxtHBKSw-lkp@intel.com/
smatch warnings:
drivers/net/ethernet/stmicro/stmmac/dwmac-intel-plat.c:163 intel_eth_plat_probe() error: we previously assumed 'dwmac->data' could be null (see line 101)
vim +163 drivers/net/ethernet/stmicro/stmmac/dwmac-intel-plat.c
9efc9b2b04c74e Rusaimi Amira Ruslan 2020-08-26 75 static int intel_eth_plat_probe(struct platform_device *pdev)
9efc9b2b04c74e Rusaimi Amira Ruslan 2020-08-26 76 {
9efc9b2b04c74e Rusaimi Amira Ruslan 2020-08-26 77 struct plat_stmmacenet_data *plat_dat;
9efc9b2b04c74e Rusaimi Amira Ruslan 2020-08-26 78 struct stmmac_resources stmmac_res;
9efc9b2b04c74e Rusaimi Amira Ruslan 2020-08-26 79 struct intel_dwmac *dwmac;
9efc9b2b04c74e Rusaimi Amira Ruslan 2020-08-26 80 unsigned long rate;
9efc9b2b04c74e Rusaimi Amira Ruslan 2020-08-26 81 int ret;
9efc9b2b04c74e Rusaimi Amira Ruslan 2020-08-26 82
9efc9b2b04c74e Rusaimi Amira Ruslan 2020-08-26 83 ret = stmmac_get_platform_resources(pdev, &stmmac_res);
9efc9b2b04c74e Rusaimi Amira Ruslan 2020-08-26 84 if (ret)
9efc9b2b04c74e Rusaimi Amira Ruslan 2020-08-26 85 return ret;
9efc9b2b04c74e Rusaimi Amira Ruslan 2020-08-26 86
abea8fd5e801a6 Jisheng Zhang 2023-09-16 87 plat_dat = devm_stmmac_probe_config_dt(pdev, stmmac_res.mac);
9efc9b2b04c74e Rusaimi Amira Ruslan 2020-08-26 88 if (IS_ERR(plat_dat)) {
9efc9b2b04c74e Rusaimi Amira Ruslan 2020-08-26 89 dev_err(&pdev->dev, "dt configuration failed\n");
9efc9b2b04c74e Rusaimi Amira Ruslan 2020-08-26 90 return PTR_ERR(plat_dat);
9efc9b2b04c74e Rusaimi Amira Ruslan 2020-08-26 91 }
9efc9b2b04c74e Rusaimi Amira Ruslan 2020-08-26 92
9efc9b2b04c74e Rusaimi Amira Ruslan 2020-08-26 93 dwmac = devm_kzalloc(&pdev->dev, sizeof(*dwmac), GFP_KERNEL);
abea8fd5e801a6 Jisheng Zhang 2023-09-16 94 if (!dwmac)
abea8fd5e801a6 Jisheng Zhang 2023-09-16 95 return -ENOMEM;
9efc9b2b04c74e Rusaimi Amira Ruslan 2020-08-26 96
9efc9b2b04c74e Rusaimi Amira Ruslan 2020-08-26 97 dwmac->dev = &pdev->dev;
9efc9b2b04c74e Rusaimi Amira Ruslan 2020-08-26 98 dwmac->tx_clk = NULL;
9efc9b2b04c74e Rusaimi Amira Ruslan 2020-08-26 99
b0377116decdee Rob Herring 2023-10-09 100 dwmac->data = device_get_match_data(&pdev->dev);
b0377116decdee Rob Herring 2023-10-09 @101 if (dwmac->data) {
^^^^^^^^^^^
Check for NULL
9efc9b2b04c74e Rusaimi Amira Ruslan 2020-08-26 102 if (dwmac->data->fix_mac_speed)
9efc9b2b04c74e Rusaimi Amira Ruslan 2020-08-26 103 plat_dat->fix_mac_speed = dwmac->data->fix_mac_speed;
9efc9b2b04c74e Rusaimi Amira Ruslan 2020-08-26 104
9efc9b2b04c74e Rusaimi Amira Ruslan 2020-08-26 105 /* Enable TX clock */
9efc9b2b04c74e Rusaimi Amira Ruslan 2020-08-26 106 if (dwmac->data->tx_clk_en) {
9efc9b2b04c74e Rusaimi Amira Ruslan 2020-08-26 107 dwmac->tx_clk = devm_clk_get(&pdev->dev, "tx_clk");
abea8fd5e801a6 Jisheng Zhang 2023-09-16 108 if (IS_ERR(dwmac->tx_clk))
abea8fd5e801a6 Jisheng Zhang 2023-09-16 109 return PTR_ERR(dwmac->tx_clk);
9efc9b2b04c74e Rusaimi Amira Ruslan 2020-08-26 110
bd8cfad17c9530 Vitalii Mordan 2024-11-08 111 ret = clk_prepare_enable(dwmac->tx_clk);
bd8cfad17c9530 Vitalii Mordan 2024-11-08 112 if (ret) {
bd8cfad17c9530 Vitalii Mordan 2024-11-08 113 dev_err(&pdev->dev,
bd8cfad17c9530 Vitalii Mordan 2024-11-08 114 "Failed to enable tx_clk\n");
bd8cfad17c9530 Vitalii Mordan 2024-11-08 115 return ret;
bd8cfad17c9530 Vitalii Mordan 2024-11-08 116 }
9efc9b2b04c74e Rusaimi Amira Ruslan 2020-08-26 117
9efc9b2b04c74e Rusaimi Amira Ruslan 2020-08-26 118 /* Check and configure TX clock rate */
9efc9b2b04c74e Rusaimi Amira Ruslan 2020-08-26 119 rate = clk_get_rate(dwmac->tx_clk);
9efc9b2b04c74e Rusaimi Amira Ruslan 2020-08-26 120 if (dwmac->data->tx_clk_rate &&
9efc9b2b04c74e Rusaimi Amira Ruslan 2020-08-26 121 rate != dwmac->data->tx_clk_rate) {
9efc9b2b04c74e Rusaimi Amira Ruslan 2020-08-26 122 rate = dwmac->data->tx_clk_rate;
9efc9b2b04c74e Rusaimi Amira Ruslan 2020-08-26 123 ret = clk_set_rate(dwmac->tx_clk, rate);
9efc9b2b04c74e Rusaimi Amira Ruslan 2020-08-26 124 if (ret) {
9efc9b2b04c74e Rusaimi Amira Ruslan 2020-08-26 125 dev_err(&pdev->dev,
9efc9b2b04c74e Rusaimi Amira Ruslan 2020-08-26 126 "Failed to set tx_clk\n");
bd8cfad17c9530 Vitalii Mordan 2024-11-08 127 goto err_tx_clk_disable;
9efc9b2b04c74e Rusaimi Amira Ruslan 2020-08-26 128 }
9efc9b2b04c74e Rusaimi Amira Ruslan 2020-08-26 129 }
9efc9b2b04c74e Rusaimi Amira Ruslan 2020-08-26 130 }
9efc9b2b04c74e Rusaimi Amira Ruslan 2020-08-26 131
9efc9b2b04c74e Rusaimi Amira Ruslan 2020-08-26 132 /* Check and configure PTP ref clock rate */
9efc9b2b04c74e Rusaimi Amira Ruslan 2020-08-26 133 rate = clk_get_rate(plat_dat->clk_ptp_ref);
9efc9b2b04c74e Rusaimi Amira Ruslan 2020-08-26 134 if (dwmac->data->ptp_ref_clk_rate &&
9efc9b2b04c74e Rusaimi Amira Ruslan 2020-08-26 135 rate != dwmac->data->ptp_ref_clk_rate) {
9efc9b2b04c74e Rusaimi Amira Ruslan 2020-08-26 136 rate = dwmac->data->ptp_ref_clk_rate;
9efc9b2b04c74e Rusaimi Amira Ruslan 2020-08-26 137 ret = clk_set_rate(plat_dat->clk_ptp_ref, rate);
9efc9b2b04c74e Rusaimi Amira Ruslan 2020-08-26 138 if (ret) {
9efc9b2b04c74e Rusaimi Amira Ruslan 2020-08-26 139 dev_err(&pdev->dev,
9efc9b2b04c74e Rusaimi Amira Ruslan 2020-08-26 140 "Failed to set clk_ptp_ref\n");
bd8cfad17c9530 Vitalii Mordan 2024-11-08 141 goto err_tx_clk_disable;
9efc9b2b04c74e Rusaimi Amira Ruslan 2020-08-26 142 }
9efc9b2b04c74e Rusaimi Amira Ruslan 2020-08-26 143 }
9efc9b2b04c74e Rusaimi Amira Ruslan 2020-08-26 144 }
9efc9b2b04c74e Rusaimi Amira Ruslan 2020-08-26 145
9efc9b2b04c74e Rusaimi Amira Ruslan 2020-08-26 146 plat_dat->bsp_priv = dwmac;
b4c5f83ae3f3e2 Rusaimi Amira Ruslan 2020-09-28 147 plat_dat->eee_usecs_rate = plat_dat->clk_ptp_rate;
b4c5f83ae3f3e2 Rusaimi Amira Ruslan 2020-09-28 148
b4c5f83ae3f3e2 Rusaimi Amira Ruslan 2020-09-28 149 if (plat_dat->eee_usecs_rate > 0) {
b4c5f83ae3f3e2 Rusaimi Amira Ruslan 2020-09-28 150 u32 tx_lpi_usec;
b4c5f83ae3f3e2 Rusaimi Amira Ruslan 2020-09-28 151
b4c5f83ae3f3e2 Rusaimi Amira Ruslan 2020-09-28 152 tx_lpi_usec = (plat_dat->eee_usecs_rate / 1000000) - 1;
b4c5f83ae3f3e2 Rusaimi Amira Ruslan 2020-09-28 153 writel(tx_lpi_usec, stmmac_res.addr + GMAC_1US_TIC_COUNTER);
b4c5f83ae3f3e2 Rusaimi Amira Ruslan 2020-09-28 154 }
9efc9b2b04c74e Rusaimi Amira Ruslan 2020-08-26 155
9efc9b2b04c74e Rusaimi Amira Ruslan 2020-08-26 156 ret = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
bd8cfad17c9530 Vitalii Mordan 2024-11-08 157 if (ret)
bd8cfad17c9530 Vitalii Mordan 2024-11-08 158 goto err_tx_clk_disable;
9efc9b2b04c74e Rusaimi Amira Ruslan 2020-08-26 159
9efc9b2b04c74e Rusaimi Amira Ruslan 2020-08-26 160 return 0;
bd8cfad17c9530 Vitalii Mordan 2024-11-08 161
bd8cfad17c9530 Vitalii Mordan 2024-11-08 162 err_tx_clk_disable:
bd8cfad17c9530 Vitalii Mordan 2024-11-08 @163 if (dwmac->data->tx_clk_en)
^^^^^^^^^^^^^
Unchecked dereference
bd8cfad17c9530 Vitalii Mordan 2024-11-08 164 clk_disable_unprepare(dwmac->tx_clk);
bd8cfad17c9530 Vitalii Mordan 2024-11-08 165 return ret;
9efc9b2b04c74e Rusaimi Amira Ruslan 2020-08-26 166 }
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Hi, On Mon, 11. Nov 12:39, Dan Carpenter wrote: > smatch warnings: > drivers/net/ethernet/stmicro/stmmac/dwmac-intel-plat.c:163 intel_eth_plat_probe() error: we previously assumed 'dwmac->data' could be null (see line 101) There is a patch [1] targeted at net-next tree which removes the check. I think there should be v2 posted soon. As it's not the first time Smatch is pointing at this issue [2], is there something to improve? I mean, posting the patches in form of a series or explaining in commit message that the check is redundant and is a subject for removal? Adding new redundant checks for the fix-patch would not be good.. What would be the most appropriate way? [1]: https://lore.kernel.org/netdev/20240930183926.2112546-1-mordan@ispras.ru/ [2]: https://lore.kernel.org/netdev/20241003111811.GJ1310185@kernel.org/ -- Thanks, Fedor
On Mon, Nov 11, 2024 at 01:25:42PM +0300, Fedor Pchelkin wrote: > Hi, > > On Mon, 11. Nov 12:39, Dan Carpenter wrote: > > smatch warnings: > > drivers/net/ethernet/stmicro/stmmac/dwmac-intel-plat.c:163 intel_eth_plat_probe() error: we previously assumed 'dwmac->data' could be null (see line 101) > > There is a patch [1] targeted at net-next tree which removes the check. I > think there should be v2 posted soon. > > As it's not the first time Smatch is pointing at this issue [2], is there > something to improve? I mean, posting the patches in form of a series or > explaining in commit message that the check is redundant and is a subject > for removal? Adding new redundant checks for the fix-patch would not be > good.. > > What would be the most appropriate way? > > [1]: https://lore.kernel.org/netdev/20240930183926.2112546-1-mordan@ispras.ru/ > [2]: https://lore.kernel.org/netdev/20241003111811.GJ1310185@kernel.org/ > Once we remove the NULL check then the warning will go away. I don't look at it like a big deal that both Simon and kbuild-bot reported the same issue. Especially since he reported it against an earlier version of this patch. regards, dan carpenter
© 2016 - 2024 Red Hat, Inc.