diff mbox series

[1/2] padata: add pd get/put refcnt helper

Message ID 20241123080509.2573987-2-chenridong@huaweicloud.com
State New
Headers show
Series padata: fix UAF in padata_reorder | expand

Commit Message

Chen Ridong Nov. 23, 2024, 8:05 a.m. UTC 
From: Chen Ridong <chenridong@huawei.com>

Add helpers for pd to get/put refcnt to make code consice.

Signed-off-by: Chen Ridong <chenridong@huawei.com>
---
 kernel/padata.c | 28 +++++++++++++++++++++-------
 1 file changed, 21 insertions(+), 7 deletions(-)

Comments

kernel test robot Nov. 25, 2024, 1:17 p.m. UTC | #1 
Hi Chen,

kernel test robot noticed the following build warnings:

[auto build test WARNING on linus/master]
[also build test WARNING on v6.12 next-20241125]
[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/Chen-Ridong/padata-add-pd-get-put-refcnt-helper/20241125-111043
base:   linus/master
patch link:    https://lore.kernel.org/r/20241123080509.2573987-2-chenridong%40huaweicloud.com
patch subject: [PATCH 1/2] padata: add pd get/put refcnt helper
config: x86_64-randconfig-161-20241125 (https://download.01.org/0day-ci/archive/20241125/202411252108.XGoGQSTI-lkp@intel.com/config)
compiler: clang version 19.1.3 (https://github.com/llvm/llvm-project ab51eccf88f5321e7c60591c5546b254b6afab99)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241125/202411252108.XGoGQSTI-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/202411252108.XGoGQSTI-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> kernel/padata.c:1134:24: warning: variable 'pd' set but not used [-Wunused-but-set-variable]
    1134 |         struct parallel_data *pd;
         |                               ^
   1 warning generated.


vim +/pd +1134 kernel/padata.c

bbefa1dd6a6d53 Herbert Xu   2019-11-26  1126  
bbefa1dd6a6d53 Herbert Xu   2019-11-26  1127  /**
bbefa1dd6a6d53 Herbert Xu   2019-11-26  1128   * padata_free_shell - free a padata shell
bbefa1dd6a6d53 Herbert Xu   2019-11-26  1129   *
bbefa1dd6a6d53 Herbert Xu   2019-11-26  1130   * @ps: padata shell to free
bbefa1dd6a6d53 Herbert Xu   2019-11-26  1131   */
bbefa1dd6a6d53 Herbert Xu   2019-11-26  1132  void padata_free_shell(struct padata_shell *ps)
bbefa1dd6a6d53 Herbert Xu   2019-11-26  1133  {
7ddc21e317b360 WangJinchao  2023-10-16 @1134  	struct parallel_data *pd;
7ddc21e317b360 WangJinchao  2023-10-16  1135  
07b24c7c08bdc2 Eric Biggers 2020-02-25  1136  	if (!ps)
07b24c7c08bdc2 Eric Biggers 2020-02-25  1137  		return;
bbefa1dd6a6d53 Herbert Xu   2019-11-26  1138  
07b24c7c08bdc2 Eric Biggers 2020-02-25  1139  	mutex_lock(&ps->pinst->lock);
bbefa1dd6a6d53 Herbert Xu   2019-11-26  1140  	list_del(&ps->list);
7ddc21e317b360 WangJinchao  2023-10-16  1141  	pd = rcu_dereference_protected(ps->pd, 1);
31df8a12c672a5 Chen Ridong  2024-11-23  1142  	padata_put_pd(ps->pd);
07b24c7c08bdc2 Eric Biggers 2020-02-25  1143  	mutex_unlock(&ps->pinst->lock);
bbefa1dd6a6d53 Herbert Xu   2019-11-26  1144  
bbefa1dd6a6d53 Herbert Xu   2019-11-26  1145  	kfree(ps);
bbefa1dd6a6d53 Herbert Xu   2019-11-26  1146  }
bbefa1dd6a6d53 Herbert Xu   2019-11-26  1147  EXPORT_SYMBOL(padata_free_shell);
bbefa1dd6a6d53 Herbert Xu   2019-11-26  1148
kernel test robot Nov. 26, 2024, 11:04 a.m. UTC | #2 
Hi Chen,

kernel test robot noticed the following build warnings:

[auto build test WARNING on linus/master]
[also build test WARNING on v6.12 next-20241126]
[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/Chen-Ridong/padata-add-pd-get-put-refcnt-helper/20241125-111043
base:   linus/master
patch link:    https://lore.kernel.org/r/20241123080509.2573987-2-chenridong%40huaweicloud.com
patch subject: [PATCH 1/2] padata: add pd get/put refcnt helper
config: x86_64-randconfig-122-20241125 (https://download.01.org/0day-ci/archive/20241126/202411261818.iINyAe83-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241126/202411261818.iINyAe83-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/202411261818.iINyAe83-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> kernel/padata.c:1142:25: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected struct parallel_data *pd @@     got struct parallel_data [noderef] __rcu *pd @@
   kernel/padata.c:1142:25: sparse:     expected struct parallel_data *pd
   kernel/padata.c:1142:25: sparse:     got struct parallel_data [noderef] __rcu *pd
   kernel/padata.c: note: in included file (through include/linux/swait.h, include/linux/completion.h):
   include/linux/list.h:83:21: sparse: sparse: self-comparison always evaluates to true
   include/linux/list.h:83:21: sparse: sparse: self-comparison always evaluates to true
   include/linux/list.h:83:21: sparse: sparse: self-comparison always evaluates to true
   include/linux/list.h:83:21: sparse: sparse: self-comparison always evaluates to true
   include/linux/list.h:83:21: sparse: sparse: self-comparison always evaluates to true
   include/linux/list.h:83:21: sparse: sparse: self-comparison always evaluates to true

vim +1142 kernel/padata.c

  1126	
  1127	/**
  1128	 * padata_free_shell - free a padata shell
  1129	 *
  1130	 * @ps: padata shell to free
  1131	 */
  1132	void padata_free_shell(struct padata_shell *ps)
  1133	{
  1134		struct parallel_data *pd;
  1135	
  1136		if (!ps)
  1137			return;
  1138	
  1139		mutex_lock(&ps->pinst->lock);
  1140		list_del(&ps->list);
  1141		pd = rcu_dereference_protected(ps->pd, 1);
> 1142		padata_put_pd(ps->pd);
  1143		mutex_unlock(&ps->pinst->lock);
  1144	
  1145		kfree(ps);
  1146	}
  1147	EXPORT_SYMBOL(padata_free_shell);
  1148
chenridong Nov. 29, 2024, 8 a.m. UTC | #3 
On 2024/11/23 16:05, Chen Ridong wrote:
> From: Chen Ridong <chenridong@huawei.com>
> 
> Add helpers for pd to get/put refcnt to make code consice.
> 
> Signed-off-by: Chen Ridong <chenridong@huawei.com>

Friendly ping.

I am looking forward to someone reviewing these patches, and if there
are any opinions, I will update the patch, as well as fixing the compile
warnings.

Thanks,
Ridong
Daniel Jordan Dec. 5, 2024, 11:03 p.m. UTC | #4 
On Sat, Nov 23, 2024 at 08:05:08AM +0000, Chen Ridong wrote:
> From: Chen Ridong <chenridong@huawei.com>
> 
> Add helpers for pd to get/put refcnt to make code consice.

Seems reasonable.

> +static inline void padata_put_pd(struct parallel_data *pd)
> +{
> +	if (refcount_dec_and_test(&pd->refcnt))
> +		padata_free_pd(pd);
> +}
> +
> +static inline void padata_put_pd_cnt(struct parallel_data *pd, int cnt)
> +{
> +	if (refcount_sub_and_test(cnt, &pd->refcnt))
> +		padata_free_pd(pd);
> +}

padata_put_pd could be defined as padata_put_pd_cnt(pd, 1).
chenridong Dec. 6, 2024, 2:13 a.m. UTC | #5 
On 2024/12/6 7:03, Daniel Jordan wrote:
> On Sat, Nov 23, 2024 at 08:05:08AM +0000, Chen Ridong wrote:
>> From: Chen Ridong <chenridong@huawei.com>
>>
>> Add helpers for pd to get/put refcnt to make code consice.
> 
> Seems reasonable.
> 
>> +static inline void padata_put_pd(struct parallel_data *pd)
>> +{
>> +	if (refcount_dec_and_test(&pd->refcnt))
>> +		padata_free_pd(pd);
>> +}
>> +
>> +static inline void padata_put_pd_cnt(struct parallel_data *pd, int cnt)
>> +{
>> +	if (refcount_sub_and_test(cnt, &pd->refcnt))
>> +		padata_free_pd(pd);
>> +}
> 
> padata_put_pd could be defined as padata_put_pd_cnt(pd, 1).
> 

Thank you, will update.

Best regards,
Ridong
diff mbox series

Patch

diff --git a/kernel/padata.c b/kernel/padata.c
index d51bbc76b227..5d8e18cdcb25 100644
--- a/kernel/padata.c
+++ b/kernel/padata.c
@@ -47,6 +47,23 @@  struct padata_mt_job_state {
 static void padata_free_pd(struct parallel_data *pd);
 static void __init padata_mt_helper(struct work_struct *work);
 
+static inline void padata_get_pd(struct parallel_data *pd)
+{
+	refcount_inc(&pd->refcnt);
+}
+
+static inline void padata_put_pd(struct parallel_data *pd)
+{
+	if (refcount_dec_and_test(&pd->refcnt))
+		padata_free_pd(pd);
+}
+
+static inline void padata_put_pd_cnt(struct parallel_data *pd, int cnt)
+{
+	if (refcount_sub_and_test(cnt, &pd->refcnt))
+		padata_free_pd(pd);
+}
+
 static int padata_index_to_cpu(struct parallel_data *pd, int cpu_index)
 {
 	int cpu, target_cpu;
@@ -206,7 +223,7 @@  int padata_do_parallel(struct padata_shell *ps,
 	if ((pinst->flags & PADATA_RESET))
 		goto out;
 
-	refcount_inc(&pd->refcnt);
+	padata_get_pd(pd);
 	padata->pd = pd;
 	padata->cb_cpu = *cb_cpu;
 
@@ -380,8 +397,7 @@  static void padata_serial_worker(struct work_struct *serial_work)
 	}
 	local_bh_enable();
 
-	if (refcount_sub_and_test(cnt, &pd->refcnt))
-		padata_free_pd(pd);
+	padata_put_pd_cnt(pd, cnt);
 }
 
 /**
@@ -681,8 +697,7 @@  static int padata_replace(struct padata_instance *pinst)
 	synchronize_rcu();
 
 	list_for_each_entry_continue_reverse(ps, &pinst->pslist, list)
-		if (refcount_dec_and_test(&ps->opd->refcnt))
-			padata_free_pd(ps->opd);
+		padata_put_pd(ps->opd);
 
 	pinst->flags &= ~PADATA_RESET;
 
@@ -1124,8 +1139,7 @@  void padata_free_shell(struct padata_shell *ps)
 	mutex_lock(&ps->pinst->lock);
 	list_del(&ps->list);
 	pd = rcu_dereference_protected(ps->pd, 1);
-	if (refcount_dec_and_test(&pd->refcnt))
-		padata_free_pd(pd);
+	padata_put_pd(ps->pd);
 	mutex_unlock(&ps->pinst->lock);
 
 	kfree(ps);