padata: Handle empty padata cpumasks

This patch fixes a bug when the padata cpumask does not intersect with the active cpumask. In this case we get a division by zero in padata_alloc_pd and we end up with a useless padata instance. Padata can end up with an empty cpumask for two reasons: 1. A user removed the last cpu that belongs to the padata cpumask and the active cpumask. 2. The last cpu that belongs to the padata cpumask and the active cpumask goes offline. We introduce a function padata_validate_cpumask to check if the padata cpumask does intersect with the active cpumask. If the cpumasks do not intersect we mark the instance as invalid, so it can't be used. We do not allocate the cpumask dependend recources in this case. This fixes the division by zero and keeps the padate instance in a consistent state. It's not possible to trigger this bug by now because the only padata user, pcrypt uses always the possible cpumask. Reported-by: Dan Kruchinin <dkruchinin@acm.org> Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

padata: Handle empty padata cpumasks
This patch fixes a bug when the padata cpumask does not intersect with the active cpumask. In this case we get a division by zero in padata_alloc_pd and we end up with a useless padata instance. Padata can end up with an empty cpumask for two reasons: 1. A user removed the last cpu that belongs to the padata cpumask and the active cpumask. 2. The last cpu that belongs to the padata cpumask and the active cpumask goes offline. We introduce a function padata_validate_cpumask to check if the padata cpumask does intersect with the active cpumask. If the cpumasks do not intersect we mark the instance as invalid, so it can't be used. We do not allocate the cpumask dependend recources in this case. This fixes the division by zero and keeps the padate instance in a consistent state. It's not possible to trigger this bug by now because the only padata user, pcrypt uses always the possible cpumask. Reported-by: Dan Kruchinin <dkruchinin@acm.org> Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Steffen Klassert · Herbert Xu
1 parent ee83655512
Showing 1 changed file with 50 additions and 11 deletions Side-by-side Diff
kernel/padata.c
@@ -516,12 +516,27 @@
  
 	synchronize_rcu();
  
-	padata_flush_queues(pd_old);
-	padata_free_pd(pd_old);
+	if (pd_old) {
+		padata_flush_queues(pd_old);
+		padata_free_pd(pd_old);
+	}
  
 	pinst->flags &= ~PADATA_RESET;
 }
  
+/* If cpumask contains no active cpu, we mark the instance as invalid. */
+static bool padata_validate_cpumask(struct padata_instance *pinst,
+				    const struct cpumask *cpumask)
+{
+	if (!cpumask_intersects(cpumask, cpu_active_mask)) {
+		pinst->flags |= PADATA_INVALID;
+		return false;
+	}
+
+	pinst->flags &= ~PADATA_INVALID;
+	return true;
+}
+
 /**
  * padata_set_cpumask - set the cpumask that padata should use
  *
  
  
@@ -531,11 +546,18 @@
 int padata_set_cpumask(struct padata_instance *pinst,
 			cpumask_var_t cpumask)
 {
-	struct parallel_data *pd;
+	int valid;
 	int err = 0;
+	struct parallel_data *pd = NULL;
  
 	mutex_lock(&pinst->lock);
  
+	valid = padata_validate_cpumask(pinst, cpumask);
+	if (!valid) {
+		__padata_stop(pinst);
+		goto out_replace;
+	}
+
 	get_online_cpus();
  
 	pd = padata_alloc_pd(pinst, cpumask);
  
@@ -544,10 +566,14 @@
 		goto out;
 	}
  
+out_replace:
 	cpumask_copy(pinst->cpumask, cpumask);
  
 	padata_replace(pinst, pd);
  
+	if (valid)
+		__padata_start(pinst);
+
 out:
 	put_online_cpus();
  
@@ -567,6 +593,9 @@
 			return -ENOMEM;
  
 		padata_replace(pinst, pd);
+
+		if (padata_validate_cpumask(pinst, pinst->cpumask))
+			__padata_start(pinst);
 	}
  
 	return 0;
  
@@ -597,9 +626,16 @@
  
 static int __padata_remove_cpu(struct padata_instance *pinst, int cpu)
 {
-	struct parallel_data *pd;
+	struct parallel_data *pd = NULL;
  
 	if (cpumask_test_cpu(cpu, cpu_online_mask)) {
+
+		if (!padata_validate_cpumask(pinst, pinst->cpumask)) {
+			__padata_stop(pinst);
+			padata_replace(pinst, pd);
+			goto out;
+		}
+
 		pd = padata_alloc_pd(pinst, pinst->cpumask);
 		if (!pd)
 			return -ENOMEM;
@@ -607,6 +643,7 @@
 		padata_replace(pinst, pd);
 	}
  
+out:
 	return 0;
 }
  
@@ -732,7 +769,7 @@
 				     struct workqueue_struct *wq)
 {
 	struct padata_instance *pinst;
-	struct parallel_data *pd;
+	struct parallel_data *pd = NULL;
  
 	pinst = kzalloc(sizeof(struct padata_instance), GFP_KERNEL);
 	if (!pinst)
  
@@ -740,12 +777,14 @@
  
 	get_online_cpus();
  
-	pd = padata_alloc_pd(pinst, cpumask);
-	if (!pd)
+	if (!alloc_cpumask_var(&pinst->cpumask, GFP_KERNEL))
 		goto err_free_inst;
  
-	if (!alloc_cpumask_var(&pinst->cpumask, GFP_KERNEL))
-		goto err_free_pd;
+	if (padata_validate_cpumask(pinst, cpumask)) {
+		pd = padata_alloc_pd(pinst, cpumask);
+		if (!pd)
+			goto err_free_mask;
+	}
  
 	rcu_assign_pointer(pinst->pd, pd);
  
@@ -767,8 +806,8 @@
  
 	return pinst;
  
-err_free_pd:
-	padata_free_pd(pd);
+err_free_mask:
+	free_cpumask_var(pinst->cpumask);
 err_free_inst:
 	kfree(pinst);
 	put_online_cpus();
...	...	@@ -516,12 +516,27 @@
516	516
517	517	synchronize_rcu();
518	518
519		- padata_flush_queues(pd_old);
520		- padata_free_pd(pd_old);
	519	+ if (pd_old) {
	520	+ padata_flush_queues(pd_old);
	521	+ padata_free_pd(pd_old);
	522	+ }
521	523
522	524	pinst->flags &= ~PADATA_RESET;
523	525	}
524	526
	527	+/* If cpumask contains no active cpu, we mark the instance as invalid. */
	528	+static bool padata_validate_cpumask(struct padata_instance *pinst,
	529	+ const struct cpumask *cpumask)
	530	+{
	531	+ if (!cpumask_intersects(cpumask, cpu_active_mask)) {
	532	+ pinst->flags \|= PADATA_INVALID;
	533	+ return false;
	534	+ }
	535	+
	536	+ pinst->flags &= ~PADATA_INVALID;
	537	+ return true;
	538	+}
	539	+
525	540	/**
526	541	* padata_set_cpumask - set the cpumask that padata should use
527	542	*
528	543
529	544
...	...	@@ -531,11 +546,18 @@
531	546	int padata_set_cpumask(struct padata_instance *pinst,
532	547	cpumask_var_t cpumask)
533	548	{
534		- struct parallel_data *pd;
	549	+ int valid;
535	550	int err = 0;
	551	+ struct parallel_data *pd = NULL;
536	552
537	553	mutex_lock(&pinst->lock);
538	554
	555	+ valid = padata_validate_cpumask(pinst, cpumask);
	556	+ if (!valid) {
	557	+ __padata_stop(pinst);
	558	+ goto out_replace;
	559	+ }
	560	+
539	561	get_online_cpus();
540	562
541	563	pd = padata_alloc_pd(pinst, cpumask);
542	564
...	...	@@ -544,10 +566,14 @@
544	566	goto out;
545	567	}
546	568
	569	+out_replace:
547	570	cpumask_copy(pinst->cpumask, cpumask);
548	571
549	572	padata_replace(pinst, pd);
550	573
	574	+ if (valid)
	575	+ __padata_start(pinst);
	576	+
551	577	out:
552	578	put_online_cpus();
553	579
...	...	@@ -567,6 +593,9 @@
567	593	return -ENOMEM;
568	594
569	595	padata_replace(pinst, pd);
	596	+
	597	+ if (padata_validate_cpumask(pinst, pinst->cpumask))
	598	+ __padata_start(pinst);
570	599	}
571	600
572	601	return 0;
573	602
...	...	@@ -597,9 +626,16 @@
597	626
598	627	static int __padata_remove_cpu(struct padata_instance *pinst, int cpu)
599	628	{
600		- struct parallel_data *pd;
	629	+ struct parallel_data *pd = NULL;
601	630
602	631	if (cpumask_test_cpu(cpu, cpu_online_mask)) {
	632	+
	633	+ if (!padata_validate_cpumask(pinst, pinst->cpumask)) {
	634	+ __padata_stop(pinst);
	635	+ padata_replace(pinst, pd);
	636	+ goto out;
	637	+ }
	638	+
603	639	pd = padata_alloc_pd(pinst, pinst->cpumask);
604	640	if (!pd)
605	641	return -ENOMEM;
...	...	@@ -607,6 +643,7 @@
607	643	padata_replace(pinst, pd);
608	644	}
609	645
	646	+out:
610	647	return 0;
611	648	}
612	649
...	...	@@ -732,7 +769,7 @@
732	769	struct workqueue_struct *wq)
733	770	{
734	771	struct padata_instance *pinst;
735		- struct parallel_data *pd;
	772	+ struct parallel_data *pd = NULL;
736	773
737	774	pinst = kzalloc(sizeof(struct padata_instance), GFP_KERNEL);
738	775	if (!pinst)
739	776
...	...	@@ -740,12 +777,14 @@
740	777
741	778	get_online_cpus();
742	779
743		- pd = padata_alloc_pd(pinst, cpumask);
744		- if (!pd)
	780	+ if (!alloc_cpumask_var(&pinst->cpumask, GFP_KERNEL))
745	781	goto err_free_inst;
746	782
747		- if (!alloc_cpumask_var(&pinst->cpumask, GFP_KERNEL))
748		- goto err_free_pd;
	783	+ if (padata_validate_cpumask(pinst, cpumask)) {
	784	+ pd = padata_alloc_pd(pinst, cpumask);
	785	+ if (!pd)
	786	+ goto err_free_mask;
	787	+ }
749	788
750	789	rcu_assign_pointer(pinst->pd, pd);
751	790
...	...	@@ -767,8 +806,8 @@
767	806
768	807	return pinst;
769	808
770		-err_free_pd:
771		- padata_free_pd(pd);
	809	+err_free_mask:
	810	+ free_cpumask_var(pinst->cpumask);
772	811	err_free_inst:
773	812	kfree(pinst);
774	813	put_online_cpus();