| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CANAwSgQObCHr19fmwuc2BfQmoiV3ZgDTFumtfCH8PP_9VbQw+w@mail.gmail.com>
Date: Wed, 9 Oct 2024 19:59:38 +0530
From: Anand Moon <linux.amoon@...il.com>
To: Dan Carpenter <dan.carpenter@...aro.org>
Cc: oe-kbuild@...ts.linux.dev, Vinod Koul <vkoul@...nel.org>,
Kishon Vijay Abraham I <kishon@...nel.org>, Heiko Stuebner <heiko@...ech.de>, linux-phy@...ts.infradead.org,
linux-kernel@...r.kernel.org, lkp@...el.com, oe-kbuild-all@...ts.linux.dev
Subject: Re: [PATCH v2 2/3] phy: rockchip-pcie: Use devm_clk_get_enabled() helper
Hi Dan,
Thanks for the report.
On Wed, 9 Oct 2024 at 17:55, Dan Carpenter <dan.carpenter@...aro.org> wrote:
>
> Hi Anand,
>
> kernel test robot noticed the following build warnings:
>
> url: https://github.com/intel-lab-lkp/linux/commits/Anand-Moon/phy-rockchip-pcie-Simplify-error-handling-with-dev_err_probe/20241007-115910
> base: 8f602276d3902642fdc3429b548d73c745446601
> patch link: https://lore.kernel.org/r/20241007035616.2701-3-linux.amoon%40gmail.com
> patch subject: [PATCH v2 2/3] phy: rockchip-pcie: Use devm_clk_get_enabled() helper
> config: loongarch-randconfig-r071-20241009 (https://download.01.org/0day-ci/archive/20241009/202410092019.vGogfPIO-lkp@intel.com/config)
> compiler: loongarch64-linux-gcc (GCC) 14.1.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@...el.com>
> | Reported-by: Dan Carpenter <dan.carpenter@...aro.org>
> | Closes: https://lore.kernel.org/r/202410092019.vGogfPIO-lkp@intel.com/
>
> smatch warnings:
> drivers/phy/rockchip/phy-rockchip-pcie.c:278 rockchip_pcie_phy_init() warn: missing error code 'err'
>
All the functions in this file explicitly return 0 instead of err, I
will fix this.
> vim +/err +278 drivers/phy/rockchip/phy-rockchip-pcie.c
>
> fcffee3d54fcad drivers/phy/phy-rockchip-pcie.c Shawn Lin 2016-09-01 269 static int rockchip_pcie_phy_init(struct phy *phy)
> fcffee3d54fcad drivers/phy/phy-rockchip-pcie.c Shawn Lin 2016-09-01 270 {
> 90a7612d070d5c drivers/phy/rockchip/phy-rockchip-pcie.c Shawn Lin 2017-07-19 271 struct phy_pcie_instance *inst = phy_get_drvdata(phy);
> 90a7612d070d5c drivers/phy/rockchip/phy-rockchip-pcie.c Shawn Lin 2017-07-19 272 struct rockchip_pcie_phy *rk_phy = to_pcie_phy(inst);
> fcffee3d54fcad drivers/phy/phy-rockchip-pcie.c Shawn Lin 2016-09-01 273 int err = 0;
> fcffee3d54fcad drivers/phy/phy-rockchip-pcie.c Shawn Lin 2016-09-01 274
> 90a7612d070d5c drivers/phy/rockchip/phy-rockchip-pcie.c Shawn Lin 2017-07-19 275 mutex_lock(&rk_phy->pcie_mutex);
> 90a7612d070d5c drivers/phy/rockchip/phy-rockchip-pcie.c Shawn Lin 2017-07-19 276
> 90a7612d070d5c drivers/phy/rockchip/phy-rockchip-pcie.c Shawn Lin 2017-07-19 277 if (rk_phy->init_cnt++)
> 90a7612d070d5c drivers/phy/rockchip/phy-rockchip-pcie.c Shawn Lin 2017-07-19 @278 goto err_out;
>
> Originally, this path just unlocked at returned zero.
>
> 90a7612d070d5c drivers/phy/rockchip/phy-rockchip-pcie.c Shawn Lin 2017-07-19 279
> fcffee3d54fcad drivers/phy/phy-rockchip-pcie.c Shawn Lin 2016-09-01 280 err = reset_control_assert(rk_phy->phy_rst);
> fcffee3d54fcad drivers/phy/phy-rockchip-pcie.c Shawn Lin 2016-09-01 281 if (err) {
> fcffee3d54fcad drivers/phy/phy-rockchip-pcie.c Shawn Lin 2016-09-01 282 dev_err(&phy->dev, "assert phy_rst err %d\n", err);
> 3114329651e74f drivers/phy/rockchip/phy-rockchip-pcie.c Anand Moon 2024-10-07 283 goto err_out;
> fcffee3d54fcad drivers/phy/phy-rockchip-pcie.c Shawn Lin 2016-09-01 284 }
> fcffee3d54fcad drivers/phy/phy-rockchip-pcie.c Shawn Lin 2016-09-01 285
> 90a7612d070d5c drivers/phy/rockchip/phy-rockchip-pcie.c Shawn Lin 2017-07-19 286 mutex_unlock(&rk_phy->pcie_mutex);
> 90a7612d070d5c drivers/phy/rockchip/phy-rockchip-pcie.c Shawn Lin 2017-07-19 287 return 0;
> fcffee3d54fcad drivers/phy/phy-rockchip-pcie.c Shawn Lin 2016-09-01 288
> 3114329651e74f drivers/phy/rockchip/phy-rockchip-pcie.c Anand Moon 2024-10-07 289 err_out:
> 90a7612d070d5c drivers/phy/rockchip/phy-rockchip-pcie.c Shawn Lin 2017-07-19 290 rk_phy->init_cnt--;
>
> Now it decrements the counter so presumably it leads to an underflow/use after
> free.
I was planning to replace the mutex_lock / mutex_unlock
with guard(mutex)(&rk_phy->pcie_mutex) in the follow up patch.
I will add this in the next revision.
>
> 90a7612d070d5c drivers/phy/rockchip/phy-rockchip-pcie.c Shawn Lin 2017-07-19 291 mutex_unlock(&rk_phy->pcie_mutex);
> fcffee3d54fcad drivers/phy/phy-rockchip-pcie.c Shawn Lin 2016-09-01 292 return err;
> fcffee3d54fcad drivers/phy/phy-rockchip-pcie.c Shawn Lin 2016-09-01 293 }
>
Here are my modified changes on top of my changes for the review process.
-----8<----------8<----------8<----------8<----------8<----------8<-----
diff --git a/drivers/phy/rockchip/phy-rockchip-pcie.c
b/drivers/phy/rockchip/phy-rockchip-pcie.c
index 2c4d6f68f02a..09685dc3fe17 100644
--- a/drivers/phy/rockchip/phy-rockchip-pcie.c
+++ b/drivers/phy/rockchip/phy-rockchip-pcie.c
@@ -248,20 +248,19 @@ static int rockchip_pcie_phy_init(struct phy *phy)
mutex_lock(&rk_phy->pcie_mutex);
- if (rk_phy->init_cnt++)
- goto err_out;
+ if (rk_phy->init_cnt++) {
+ mutex_unlock(&rk_phy->pcie_mutex);
+ return err;
+ }
err = reset_control_assert(rk_phy->phy_rst);
if (err) {
dev_err(&phy->dev, "assert phy_rst err %d\n", err);
- goto err_out;
+ rk_phy->init_cnt--;
+ mutex_unlock(&rk_phy->pcie_mutex);
+ return err;
}
- mutex_unlock(&rk_phy->pcie_mutex);
- return 0;
-
-err_out:
- rk_phy->init_cnt--;
mutex_unlock(&rk_phy->pcie_mutex);
return err;
}
Thanks
-Anand
Powered by blists - more mailing lists