diff mbox series

[v3] ieee802154: ca8210: Fix a potential UAF in ca8210_probe

Message ID 20231001054949.14624-1-dinghao.liu@zju.edu.cn (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series [v3] ieee802154: ca8210: Fix a potential UAF in ca8210_probe | expand

Checks

Context Check Description
netdev/series_format warning Single patches do not need cover letters; Target tree name not specified in the subject
netdev/tree_selection success Guessed tree name to be net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit fail Errors and warnings before: 1343 this patch: 1344
netdev/cc_maintainers success CCed 11 of 11 maintainers
netdev/build_clang fail Errors and warnings before: 1363 this patch: 1365
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn fail Errors and warnings before: 1366 this patch: 1367
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 29 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Dinghao Liu Oct. 1, 2023, 5:49 a.m. UTC 
If of_clk_add_provider() fails in ca8210_register_ext_clock(),
it calls clk_unregister() to release priv->clk and returns an
error. However, the caller ca8210_probe() then calls ca8210_remove(),
where priv->clk is freed again in ca8210_unregister_ext_clock(). In
this case, a use-after-free may happen in the second time we call
clk_unregister().

Fix this by removing the first clk_unregister(). Also, priv->clk could
be an error code on failure of clk_register_fixed_rate(). Use
IS_ERR_OR_NULL to catch this case in ca8210_unregister_ext_clock().

Fixes: ded845a781a5 ("ieee802154: Add CA8210 IEEE 802.15.4 device driver")
Signed-off-by: Dinghao Liu <dinghao.liu@zju.edu.cn>
---

Changelog:

v2: -Remove the first clk_unregister() instead of nulling priv->clk.

v3: -Simplify ca8210_register_ext_clock().
    -Add a ';' after return in ca8210_unregister_ext_clock().
---
 drivers/net/ieee802154/ca8210.c | 16 +++-------------
 1 file changed, 3 insertions(+), 13 deletions(-)

Comments

kernel test robot Oct. 1, 2023, 7:19 a.m. UTC | #1 
Hi Dinghao,

kernel test robot noticed the following build warnings:

[auto build test WARNING on linus/master]
[also build test WARNING on v6.6-rc3 next-20230929]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Dinghao-Liu/ieee802154-ca8210-Fix-a-potential-UAF-in-ca8210_probe/20231001-135130
base:   linus/master
patch link:    https://lore.kernel.org/r/20231001054949.14624-1-dinghao.liu%40zju.edu.cn
patch subject: [PATCH] [v3] ieee802154: ca8210: Fix a potential UAF in ca8210_probe
config: m68k-allyesconfig (https://download.01.org/0day-ci/archive/20231001/202310011548.qyQMuodI-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231001/202310011548.qyQMuodI-lkp@intel.com/reproduce)

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>
| Closes: https://lore.kernel.org/oe-kbuild-all/202310011548.qyQMuodI-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/net/ieee802154/ca8210.c: In function 'ca8210_register_ext_clock':
>> drivers/net/ieee802154/ca8210.c:2743:13: warning: unused variable 'ret' [-Wunused-variable]
    2743 |         int ret = 0;
         |             ^~~


vim +/ret +2743 drivers/net/ieee802154/ca8210.c

ded845a781a578 Harry Morris 2017-03-28  2731  
ded845a781a578 Harry Morris 2017-03-28  2732  /**
ded845a781a578 Harry Morris 2017-03-28  2733   * ca8210_register_ext_clock() - Register ca8210's external clock with kernel
ded845a781a578 Harry Morris 2017-03-28  2734   * @spi:  Pointer to target ca8210 spi device
ded845a781a578 Harry Morris 2017-03-28  2735   *
ded845a781a578 Harry Morris 2017-03-28  2736   * Return: 0 or linux error code
ded845a781a578 Harry Morris 2017-03-28  2737   */
ded845a781a578 Harry Morris 2017-03-28  2738  static int ca8210_register_ext_clock(struct spi_device *spi)
ded845a781a578 Harry Morris 2017-03-28  2739  {
ded845a781a578 Harry Morris 2017-03-28  2740  	struct device_node *np = spi->dev.of_node;
ded845a781a578 Harry Morris 2017-03-28  2741  	struct ca8210_priv *priv = spi_get_drvdata(spi);
ded845a781a578 Harry Morris 2017-03-28  2742  	struct ca8210_platform_data *pdata = spi->dev.platform_data;
ded845a781a578 Harry Morris 2017-03-28 @2743  	int ret = 0;
ded845a781a578 Harry Morris 2017-03-28  2744  
ded845a781a578 Harry Morris 2017-03-28  2745  	if (!np)
ded845a781a578 Harry Morris 2017-03-28  2746  		return -EFAULT;
ded845a781a578 Harry Morris 2017-03-28  2747  
ded845a781a578 Harry Morris 2017-03-28  2748  	priv->clk = clk_register_fixed_rate(
ded845a781a578 Harry Morris 2017-03-28  2749  		&spi->dev,
ded845a781a578 Harry Morris 2017-03-28  2750  		np->name,
ded845a781a578 Harry Morris 2017-03-28  2751  		NULL,
ded845a781a578 Harry Morris 2017-03-28  2752  		0,
ded845a781a578 Harry Morris 2017-03-28  2753  		pdata->extclockfreq
ded845a781a578 Harry Morris 2017-03-28  2754  	);
ded845a781a578 Harry Morris 2017-03-28  2755  
ded845a781a578 Harry Morris 2017-03-28  2756  	if (IS_ERR(priv->clk)) {
ded845a781a578 Harry Morris 2017-03-28  2757  		dev_crit(&spi->dev, "Failed to register external clk\n");
ded845a781a578 Harry Morris 2017-03-28  2758  		return PTR_ERR(priv->clk);
ded845a781a578 Harry Morris 2017-03-28  2759  	}
ded845a781a578 Harry Morris 2017-03-28  2760  
d0603f3c78f0aa Dinghao Liu  2023-10-01  2761  	return of_clk_add_provider(np, of_clk_src_simple_get, priv->clk);
ded845a781a578 Harry Morris 2017-03-28  2762  }
ded845a781a578 Harry Morris 2017-03-28  2763
Stefan Schmidt Oct. 5, 2023, 6:25 p.m. UTC | #2 
Hello Dinghao,


On 01.10.23 09:19, kernel test robot wrote:
> Hi Dinghao,
> 
> kernel test robot noticed the following build warnings:
> 
> [auto build test WARNING on linus/master]
> [also build test WARNING on v6.6-rc3 next-20230929]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
> 
> url:    https://github.com/intel-lab-lkp/linux/commits/Dinghao-Liu/ieee802154-ca8210-Fix-a-potential-UAF-in-ca8210_probe/20231001-135130
> base:   linus/master
> patch link:    https://lore.kernel.org/r/20231001054949.14624-1-dinghao.liu%40zju.edu.cn
> patch subject: [PATCH] [v3] ieee802154: ca8210: Fix a potential UAF in ca8210_probe
> config: m68k-allyesconfig (https://download.01.org/0day-ci/archive/20231001/202310011548.qyQMuodI-lkp@intel.com/config)
> compiler: m68k-linux-gcc (GCC) 13.2.0
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231001/202310011548.qyQMuodI-lkp@intel.com/reproduce)
> 
> 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>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202310011548.qyQMuodI-lkp@intel.com/
> 
> All warnings (new ones prefixed by >>):
> 
>     drivers/net/ieee802154/ca8210.c: In function 'ca8210_register_ext_clock':
>>> drivers/net/ieee802154/ca8210.c:2743:13: warning: unused variable 'ret' [-Wunused-variable]
>      2743 |         int ret = 0;
>           |             ^~~
> 

Please take care of this now unused variable after your re-factor.
With this fixed and send out as v4 I am happy to get this applied to the 
wpan tree.

regards
Stefan Schmidt
Dinghao Liu Oct. 7, 2023, 1:35 a.m. UTC | #3 
> Hello Dinghao,
> 
> 
> On 01.10.23 09:19, kernel test robot wrote:
> > Hi Dinghao,
> > 
> > kernel test robot noticed the following build warnings:
> > 
> > [auto build test WARNING on linus/master]
> > [also build test WARNING on v6.6-rc3 next-20230929]
> > [If your patch is applied to the wrong git tree, kindly drop us a note.
> > And when submitting patch, we suggest to use '--base' as documented in
> > https://git-scm.com/docs/git-format-patch#_base_tree_information]
> > 
> > url:    https://github.com/intel-lab-lkp/linux/commits/Dinghao-Liu/ieee802154-ca8210-Fix-a-potential-UAF-in-ca8210_probe/20231001-135130
> > base:   linus/master
> > patch link:    https://lore.kernel.org/r/20231001054949.14624-1-dinghao.liu%40zju.edu.cn
> > patch subject: [PATCH] [v3] ieee802154: ca8210: Fix a potential UAF in ca8210_probe
> > config: m68k-allyesconfig (https://download.01.org/0day-ci/archive/20231001/202310011548.qyQMuodI-lkp@intel.com/config)
> > compiler: m68k-linux-gcc (GCC) 13.2.0
> > reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231001/202310011548.qyQMuodI-lkp@intel.com/reproduce)
> > 
> > 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>
> > | Closes: https://lore.kernel.org/oe-kbuild-all/202310011548.qyQMuodI-lkp@intel.com/
> > 
> > All warnings (new ones prefixed by >>):
> > 
> >     drivers/net/ieee802154/ca8210.c: In function 'ca8210_register_ext_clock':
> >>> drivers/net/ieee802154/ca8210.c:2743:13: warning: unused variable 'ret' [-Wunused-variable]
> >      2743 |         int ret = 0;
> >           |             ^~~
> > 
> 
> Please take care of this now unused variable after your re-factor.
> With this fixed and send out as v4 I am happy to get this applied to the 
> wpan tree.

I will resend the v4 patch soon, thanks!

Regards,
Dinghao
diff mbox series

Patch

diff --git a/drivers/net/ieee802154/ca8210.c b/drivers/net/ieee802154/ca8210.c
index aebb19f1b3a4..ae44a9133937 100644
--- a/drivers/net/ieee802154/ca8210.c
+++ b/drivers/net/ieee802154/ca8210.c
@@ -2757,18 +2757,8 @@  static int ca8210_register_ext_clock(struct spi_device *spi)
 		dev_crit(&spi->dev, "Failed to register external clk\n");
 		return PTR_ERR(priv->clk);
 	}
-	ret = of_clk_add_provider(np, of_clk_src_simple_get, priv->clk);
-	if (ret) {
-		clk_unregister(priv->clk);
-		dev_crit(
-			&spi->dev,
-			"Failed to register external clock as clock provider\n"
-		);
-	} else {
-		dev_info(&spi->dev, "External clock set as clock provider\n");
-	}
 
-	return ret;
+	return of_clk_add_provider(np, of_clk_src_simple_get, priv->clk);
 }
 
 /**
@@ -2780,8 +2770,8 @@  static void ca8210_unregister_ext_clock(struct spi_device *spi)
 {
 	struct ca8210_priv *priv = spi_get_drvdata(spi);
 
-	if (!priv->clk)
-		return
+	if (IS_ERR_OR_NULL(priv->clk))
+		return;
 
 	of_clk_del_provider(spi->dev.of_node);
 	clk_unregister(priv->clk);