forked from mirrors/linux
		
	padata: validate cpumask without removed CPU during offline
Configuring an instance's parallel mask without any online CPUs...
  echo 2 > /sys/kernel/pcrypt/pencrypt/parallel_cpumask
  echo 0 > /sys/devices/system/cpu/cpu1/online
...makes tcrypt mode=215 crash like this:
  divide error: 0000 [#1] SMP PTI
  CPU: 4 PID: 283 Comm: modprobe Not tainted 5.4.0-rc8-padata-doc-v2+ #2
  Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS ?-20191013_105130-anatol 04/01/2014
  RIP: 0010:padata_do_parallel+0x114/0x300
  Call Trace:
   pcrypt_aead_encrypt+0xc0/0xd0 [pcrypt]
   crypto_aead_encrypt+0x1f/0x30
   do_mult_aead_op+0x4e/0xdf [tcrypt]
   test_mb_aead_speed.constprop.0.cold+0x226/0x564 [tcrypt]
   do_test+0x28c2/0x4d49 [tcrypt]
   tcrypt_mod_init+0x55/0x1000 [tcrypt]
   ...
cpumask_weight() in padata_cpu_hash() returns 0 because the mask has no
CPUs.  The problem is __padata_remove_cpu() checks for valid masks too
early and so doesn't mark the instance PADATA_INVALID as expected, which
would have made padata_do_parallel() return error before doing the
division.
Fix by introducing a second padata CPU hotplug state before
CPUHP_BRINGUP_CPU so that __padata_remove_cpu() sees the online mask
without @cpu.  No need for the second argument to padata_replace() since
@cpu is now already missing from the online mask.
Fixes: 33e5445068 ("padata: Handle empty padata cpumasks")
Signed-off-by: Daniel Jordan <daniel.m.jordan@oracle.com>
Cc: Eric Biggers <ebiggers@kernel.org>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Steffen Klassert <steffen.klassert@secunet.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-crypto@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
			
			
This commit is contained in:
		
							parent
							
								
									e8cfed5e4e
								
							
						
					
					
						commit
						894c9ef978
					
				
					 2 changed files with 19 additions and 12 deletions
				
			
		| 
						 | 
					@ -59,6 +59,7 @@ enum cpuhp_state {
 | 
				
			||||||
	CPUHP_IOMMU_INTEL_DEAD,
 | 
						CPUHP_IOMMU_INTEL_DEAD,
 | 
				
			||||||
	CPUHP_LUSTRE_CFS_DEAD,
 | 
						CPUHP_LUSTRE_CFS_DEAD,
 | 
				
			||||||
	CPUHP_AP_ARM_CACHE_B15_RAC_DEAD,
 | 
						CPUHP_AP_ARM_CACHE_B15_RAC_DEAD,
 | 
				
			||||||
 | 
						CPUHP_PADATA_DEAD,
 | 
				
			||||||
	CPUHP_WORKQUEUE_PREP,
 | 
						CPUHP_WORKQUEUE_PREP,
 | 
				
			||||||
	CPUHP_POWER_NUMA_PREPARE,
 | 
						CPUHP_POWER_NUMA_PREPARE,
 | 
				
			||||||
	CPUHP_HRTIMERS_PREPARE,
 | 
						CPUHP_HRTIMERS_PREPARE,
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -512,7 +512,7 @@ static int padata_replace_one(struct padata_shell *ps)
 | 
				
			||||||
	return 0;
 | 
						return 0;
 | 
				
			||||||
}
 | 
					}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
static int padata_replace(struct padata_instance *pinst, int cpu)
 | 
					static int padata_replace(struct padata_instance *pinst)
 | 
				
			||||||
{
 | 
					{
 | 
				
			||||||
	int notification_mask = 0;
 | 
						int notification_mask = 0;
 | 
				
			||||||
	struct padata_shell *ps;
 | 
						struct padata_shell *ps;
 | 
				
			||||||
| 
						 | 
					@ -523,16 +523,12 @@ static int padata_replace(struct padata_instance *pinst, int cpu)
 | 
				
			||||||
	cpumask_copy(pinst->omask, pinst->rcpumask.pcpu);
 | 
						cpumask_copy(pinst->omask, pinst->rcpumask.pcpu);
 | 
				
			||||||
	cpumask_and(pinst->rcpumask.pcpu, pinst->cpumask.pcpu,
 | 
						cpumask_and(pinst->rcpumask.pcpu, pinst->cpumask.pcpu,
 | 
				
			||||||
		    cpu_online_mask);
 | 
							    cpu_online_mask);
 | 
				
			||||||
	if (cpu >= 0)
 | 
					 | 
				
			||||||
		cpumask_clear_cpu(cpu, pinst->rcpumask.pcpu);
 | 
					 | 
				
			||||||
	if (!cpumask_equal(pinst->omask, pinst->rcpumask.pcpu))
 | 
						if (!cpumask_equal(pinst->omask, pinst->rcpumask.pcpu))
 | 
				
			||||||
		notification_mask |= PADATA_CPU_PARALLEL;
 | 
							notification_mask |= PADATA_CPU_PARALLEL;
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	cpumask_copy(pinst->omask, pinst->rcpumask.cbcpu);
 | 
						cpumask_copy(pinst->omask, pinst->rcpumask.cbcpu);
 | 
				
			||||||
	cpumask_and(pinst->rcpumask.cbcpu, pinst->cpumask.cbcpu,
 | 
						cpumask_and(pinst->rcpumask.cbcpu, pinst->cpumask.cbcpu,
 | 
				
			||||||
		    cpu_online_mask);
 | 
							    cpu_online_mask);
 | 
				
			||||||
	if (cpu >= 0)
 | 
					 | 
				
			||||||
		cpumask_clear_cpu(cpu, pinst->rcpumask.cbcpu);
 | 
					 | 
				
			||||||
	if (!cpumask_equal(pinst->omask, pinst->rcpumask.cbcpu))
 | 
						if (!cpumask_equal(pinst->omask, pinst->rcpumask.cbcpu))
 | 
				
			||||||
		notification_mask |= PADATA_CPU_SERIAL;
 | 
							notification_mask |= PADATA_CPU_SERIAL;
 | 
				
			||||||
 | 
					
 | 
				
			||||||
| 
						 | 
					@ -624,7 +620,7 @@ static int __padata_set_cpumasks(struct padata_instance *pinst,
 | 
				
			||||||
	cpumask_copy(pinst->cpumask.pcpu, pcpumask);
 | 
						cpumask_copy(pinst->cpumask.pcpu, pcpumask);
 | 
				
			||||||
	cpumask_copy(pinst->cpumask.cbcpu, cbcpumask);
 | 
						cpumask_copy(pinst->cpumask.cbcpu, cbcpumask);
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	err = padata_setup_cpumasks(pinst) ?: padata_replace(pinst, -1);
 | 
						err = padata_setup_cpumasks(pinst) ?: padata_replace(pinst);
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	if (valid)
 | 
						if (valid)
 | 
				
			||||||
		__padata_start(pinst);
 | 
							__padata_start(pinst);
 | 
				
			||||||
| 
						 | 
					@ -715,7 +711,7 @@ static int __padata_add_cpu(struct padata_instance *pinst, int cpu)
 | 
				
			||||||
	int err = 0;
 | 
						int err = 0;
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	if (cpumask_test_cpu(cpu, cpu_online_mask)) {
 | 
						if (cpumask_test_cpu(cpu, cpu_online_mask)) {
 | 
				
			||||||
		err = padata_replace(pinst, -1);
 | 
							err = padata_replace(pinst);
 | 
				
			||||||
 | 
					
 | 
				
			||||||
		if (padata_validate_cpumask(pinst, pinst->cpumask.pcpu) &&
 | 
							if (padata_validate_cpumask(pinst, pinst->cpumask.pcpu) &&
 | 
				
			||||||
		    padata_validate_cpumask(pinst, pinst->cpumask.cbcpu))
 | 
							    padata_validate_cpumask(pinst, pinst->cpumask.cbcpu))
 | 
				
			||||||
| 
						 | 
					@ -729,12 +725,12 @@ static int __padata_remove_cpu(struct padata_instance *pinst, int cpu)
 | 
				
			||||||
{
 | 
					{
 | 
				
			||||||
	int err = 0;
 | 
						int err = 0;
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	if (cpumask_test_cpu(cpu, cpu_online_mask)) {
 | 
						if (!cpumask_test_cpu(cpu, cpu_online_mask)) {
 | 
				
			||||||
		if (!padata_validate_cpumask(pinst, pinst->cpumask.pcpu) ||
 | 
							if (!padata_validate_cpumask(pinst, pinst->cpumask.pcpu) ||
 | 
				
			||||||
		    !padata_validate_cpumask(pinst, pinst->cpumask.cbcpu))
 | 
							    !padata_validate_cpumask(pinst, pinst->cpumask.cbcpu))
 | 
				
			||||||
			__padata_stop(pinst);
 | 
								__padata_stop(pinst);
 | 
				
			||||||
 | 
					
 | 
				
			||||||
		err = padata_replace(pinst, cpu);
 | 
							err = padata_replace(pinst);
 | 
				
			||||||
	}
 | 
						}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	return err;
 | 
						return err;
 | 
				
			||||||
| 
						 | 
					@ -761,7 +757,7 @@ static int padata_cpu_online(unsigned int cpu, struct hlist_node *node)
 | 
				
			||||||
	return ret;
 | 
						return ret;
 | 
				
			||||||
}
 | 
					}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
static int padata_cpu_prep_down(unsigned int cpu, struct hlist_node *node)
 | 
					static int padata_cpu_dead(unsigned int cpu, struct hlist_node *node)
 | 
				
			||||||
{
 | 
					{
 | 
				
			||||||
	struct padata_instance *pinst;
 | 
						struct padata_instance *pinst;
 | 
				
			||||||
	int ret;
 | 
						int ret;
 | 
				
			||||||
| 
						 | 
					@ -782,6 +778,7 @@ static enum cpuhp_state hp_online;
 | 
				
			||||||
static void __padata_free(struct padata_instance *pinst)
 | 
					static void __padata_free(struct padata_instance *pinst)
 | 
				
			||||||
{
 | 
					{
 | 
				
			||||||
#ifdef CONFIG_HOTPLUG_CPU
 | 
					#ifdef CONFIG_HOTPLUG_CPU
 | 
				
			||||||
 | 
						cpuhp_state_remove_instance_nocalls(CPUHP_PADATA_DEAD, &pinst->node);
 | 
				
			||||||
	cpuhp_state_remove_instance_nocalls(hp_online, &pinst->node);
 | 
						cpuhp_state_remove_instance_nocalls(hp_online, &pinst->node);
 | 
				
			||||||
#endif
 | 
					#endif
 | 
				
			||||||
 | 
					
 | 
				
			||||||
| 
						 | 
					@ -989,6 +986,8 @@ static struct padata_instance *padata_alloc(const char *name,
 | 
				
			||||||
 | 
					
 | 
				
			||||||
#ifdef CONFIG_HOTPLUG_CPU
 | 
					#ifdef CONFIG_HOTPLUG_CPU
 | 
				
			||||||
	cpuhp_state_add_instance_nocalls_cpuslocked(hp_online, &pinst->node);
 | 
						cpuhp_state_add_instance_nocalls_cpuslocked(hp_online, &pinst->node);
 | 
				
			||||||
 | 
						cpuhp_state_add_instance_nocalls_cpuslocked(CPUHP_PADATA_DEAD,
 | 
				
			||||||
 | 
											    &pinst->node);
 | 
				
			||||||
#endif
 | 
					#endif
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	put_online_cpus();
 | 
						put_online_cpus();
 | 
				
			||||||
| 
						 | 
					@ -1101,17 +1100,24 @@ static __init int padata_driver_init(void)
 | 
				
			||||||
	int ret;
 | 
						int ret;
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	ret = cpuhp_setup_state_multi(CPUHP_AP_ONLINE_DYN, "padata:online",
 | 
						ret = cpuhp_setup_state_multi(CPUHP_AP_ONLINE_DYN, "padata:online",
 | 
				
			||||||
				      padata_cpu_online,
 | 
									      padata_cpu_online, NULL);
 | 
				
			||||||
				      padata_cpu_prep_down);
 | 
					 | 
				
			||||||
	if (ret < 0)
 | 
						if (ret < 0)
 | 
				
			||||||
		return ret;
 | 
							return ret;
 | 
				
			||||||
	hp_online = ret;
 | 
						hp_online = ret;
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
						ret = cpuhp_setup_state_multi(CPUHP_PADATA_DEAD, "padata:dead",
 | 
				
			||||||
 | 
									      NULL, padata_cpu_dead);
 | 
				
			||||||
 | 
						if (ret < 0) {
 | 
				
			||||||
 | 
							cpuhp_remove_multi_state(hp_online);
 | 
				
			||||||
 | 
							return ret;
 | 
				
			||||||
 | 
						}
 | 
				
			||||||
	return 0;
 | 
						return 0;
 | 
				
			||||||
}
 | 
					}
 | 
				
			||||||
module_init(padata_driver_init);
 | 
					module_init(padata_driver_init);
 | 
				
			||||||
 | 
					
 | 
				
			||||||
static __exit void padata_driver_exit(void)
 | 
					static __exit void padata_driver_exit(void)
 | 
				
			||||||
{
 | 
					{
 | 
				
			||||||
 | 
						cpuhp_remove_multi_state(CPUHP_PADATA_DEAD);
 | 
				
			||||||
	cpuhp_remove_multi_state(hp_online);
 | 
						cpuhp_remove_multi_state(hp_online);
 | 
				
			||||||
}
 | 
					}
 | 
				
			||||||
module_exit(padata_driver_exit);
 | 
					module_exit(padata_driver_exit);
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
		Loading…
	
		Reference in a new issue