forked from mirrors/linux
		
	random: add a spinlock_t to struct batched_entropy
The per-CPU variable batched_entropy_uXX is protected by get_cpu_var(). This is just a preempt_disable() which ensures that the variable is only from the local CPU. It does not protect against users on the same CPU from another context. It is possible that a preemptible context reads slot 0 and then an interrupt occurs and the same value is read again. The above scenario is confirmed by lockdep if we add a spinlock: | ================================ | WARNING: inconsistent lock state | 5.1.0-rc3+ #42 Not tainted | -------------------------------- | inconsistent {SOFTIRQ-ON-W} -> {IN-SOFTIRQ-W} usage. | ksoftirqd/9/56 [HC0[0]:SC1[1]:HE0:SE0] takes: | (____ptrval____) (batched_entropy_u32.lock){+.?.}, at: get_random_u32+0x3e/0xe0 | {SOFTIRQ-ON-W} state was registered at: | _raw_spin_lock+0x2a/0x40 | get_random_u32+0x3e/0xe0 | new_slab+0x15c/0x7b0 | ___slab_alloc+0x492/0x620 | __slab_alloc.isra.73+0x53/0xa0 | kmem_cache_alloc_node+0xaf/0x2a0 | copy_process.part.41+0x1e1/0x2370 | _do_fork+0xdb/0x6d0 | kernel_thread+0x20/0x30 | kthreadd+0x1ba/0x220 | ret_from_fork+0x3a/0x50 … | other info that might help us debug this: | Possible unsafe locking scenario: | | CPU0 | ---- | lock(batched_entropy_u32.lock); | <Interrupt> | lock(batched_entropy_u32.lock); | | *** DEADLOCK *** | | stack backtrace: | Call Trace: … | kmem_cache_alloc_trace+0x20e/0x270 | ipmi_alloc_recv_msg+0x16/0x40 … | __do_softirq+0xec/0x48d | run_ksoftirqd+0x37/0x60 | smpboot_thread_fn+0x191/0x290 | kthread+0xfe/0x130 | ret_from_fork+0x3a/0x50 Add a spinlock_t to the batched_entropy data structure and acquire the lock while accessing it. Acquire the lock with disabled interrupts because this function may be used from interrupt context. Remove the batched_entropy_reset_lock lock. Now that we have a lock for the data scructure, we can access it from a remote CPU. Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de> Signed-off-by: Theodore Ts'o <tytso@mit.edu>
This commit is contained in:
		
							parent
							
								
									92e507d216
								
							
						
					
					
						commit
						b7d5dc2107
					
				
					 1 changed files with 27 additions and 25 deletions
				
			
		|  | @ -2282,8 +2282,8 @@ struct batched_entropy { | ||||||
| 		u32 entropy_u32[CHACHA_BLOCK_SIZE / sizeof(u32)]; | 		u32 entropy_u32[CHACHA_BLOCK_SIZE / sizeof(u32)]; | ||||||
| 	}; | 	}; | ||||||
| 	unsigned int position; | 	unsigned int position; | ||||||
|  | 	spinlock_t batch_lock; | ||||||
| }; | }; | ||||||
| static rwlock_t batched_entropy_reset_lock = __RW_LOCK_UNLOCKED(batched_entropy_reset_lock); |  | ||||||
| 
 | 
 | ||||||
| /*
 | /*
 | ||||||
|  * Get a random word for internal kernel use only. The quality of the random |  * Get a random word for internal kernel use only. The quality of the random | ||||||
|  | @ -2293,12 +2293,14 @@ static rwlock_t batched_entropy_reset_lock = __RW_LOCK_UNLOCKED(batched_entropy_ | ||||||
|  * wait_for_random_bytes() should be called and return 0 at least once |  * wait_for_random_bytes() should be called and return 0 at least once | ||||||
|  * at any point prior. |  * at any point prior. | ||||||
|  */ |  */ | ||||||
| static DEFINE_PER_CPU(struct batched_entropy, batched_entropy_u64); | static DEFINE_PER_CPU(struct batched_entropy, batched_entropy_u64) = { | ||||||
|  | 	.batch_lock	= __SPIN_LOCK_UNLOCKED(batched_entropy_u64.lock), | ||||||
|  | }; | ||||||
|  | 
 | ||||||
| u64 get_random_u64(void) | u64 get_random_u64(void) | ||||||
| { | { | ||||||
| 	u64 ret; | 	u64 ret; | ||||||
| 	bool use_lock; | 	unsigned long flags; | ||||||
| 	unsigned long flags = 0; |  | ||||||
| 	struct batched_entropy *batch; | 	struct batched_entropy *batch; | ||||||
| 	static void *previous; | 	static void *previous; | ||||||
| 
 | 
 | ||||||
|  | @ -2313,28 +2315,25 @@ u64 get_random_u64(void) | ||||||
| 
 | 
 | ||||||
| 	warn_unseeded_randomness(&previous); | 	warn_unseeded_randomness(&previous); | ||||||
| 
 | 
 | ||||||
| 	use_lock = READ_ONCE(crng_init) < 2; | 	batch = raw_cpu_ptr(&batched_entropy_u64); | ||||||
| 	batch = &get_cpu_var(batched_entropy_u64); | 	spin_lock_irqsave(&batch->batch_lock, flags); | ||||||
| 	if (use_lock) |  | ||||||
| 		read_lock_irqsave(&batched_entropy_reset_lock, flags); |  | ||||||
| 	if (batch->position % ARRAY_SIZE(batch->entropy_u64) == 0) { | 	if (batch->position % ARRAY_SIZE(batch->entropy_u64) == 0) { | ||||||
| 		extract_crng((u8 *)batch->entropy_u64); | 		extract_crng((u8 *)batch->entropy_u64); | ||||||
| 		batch->position = 0; | 		batch->position = 0; | ||||||
| 	} | 	} | ||||||
| 	ret = batch->entropy_u64[batch->position++]; | 	ret = batch->entropy_u64[batch->position++]; | ||||||
| 	if (use_lock) | 	spin_unlock_irqrestore(&batch->batch_lock, flags); | ||||||
| 		read_unlock_irqrestore(&batched_entropy_reset_lock, flags); |  | ||||||
| 	put_cpu_var(batched_entropy_u64); |  | ||||||
| 	return ret; | 	return ret; | ||||||
| } | } | ||||||
| EXPORT_SYMBOL(get_random_u64); | EXPORT_SYMBOL(get_random_u64); | ||||||
| 
 | 
 | ||||||
| static DEFINE_PER_CPU(struct batched_entropy, batched_entropy_u32); | static DEFINE_PER_CPU(struct batched_entropy, batched_entropy_u32) = { | ||||||
|  | 	.batch_lock	= __SPIN_LOCK_UNLOCKED(batched_entropy_u32.lock), | ||||||
|  | }; | ||||||
| u32 get_random_u32(void) | u32 get_random_u32(void) | ||||||
| { | { | ||||||
| 	u32 ret; | 	u32 ret; | ||||||
| 	bool use_lock; | 	unsigned long flags; | ||||||
| 	unsigned long flags = 0; |  | ||||||
| 	struct batched_entropy *batch; | 	struct batched_entropy *batch; | ||||||
| 	static void *previous; | 	static void *previous; | ||||||
| 
 | 
 | ||||||
|  | @ -2343,18 +2342,14 @@ u32 get_random_u32(void) | ||||||
| 
 | 
 | ||||||
| 	warn_unseeded_randomness(&previous); | 	warn_unseeded_randomness(&previous); | ||||||
| 
 | 
 | ||||||
| 	use_lock = READ_ONCE(crng_init) < 2; | 	batch = raw_cpu_ptr(&batched_entropy_u32); | ||||||
| 	batch = &get_cpu_var(batched_entropy_u32); | 	spin_lock_irqsave(&batch->batch_lock, flags); | ||||||
| 	if (use_lock) |  | ||||||
| 		read_lock_irqsave(&batched_entropy_reset_lock, flags); |  | ||||||
| 	if (batch->position % ARRAY_SIZE(batch->entropy_u32) == 0) { | 	if (batch->position % ARRAY_SIZE(batch->entropy_u32) == 0) { | ||||||
| 		extract_crng((u8 *)batch->entropy_u32); | 		extract_crng((u8 *)batch->entropy_u32); | ||||||
| 		batch->position = 0; | 		batch->position = 0; | ||||||
| 	} | 	} | ||||||
| 	ret = batch->entropy_u32[batch->position++]; | 	ret = batch->entropy_u32[batch->position++]; | ||||||
| 	if (use_lock) | 	spin_unlock_irqrestore(&batch->batch_lock, flags); | ||||||
| 		read_unlock_irqrestore(&batched_entropy_reset_lock, flags); |  | ||||||
| 	put_cpu_var(batched_entropy_u32); |  | ||||||
| 	return ret; | 	return ret; | ||||||
| } | } | ||||||
| EXPORT_SYMBOL(get_random_u32); | EXPORT_SYMBOL(get_random_u32); | ||||||
|  | @ -2368,12 +2363,19 @@ static void invalidate_batched_entropy(void) | ||||||
| 	int cpu; | 	int cpu; | ||||||
| 	unsigned long flags; | 	unsigned long flags; | ||||||
| 
 | 
 | ||||||
| 	write_lock_irqsave(&batched_entropy_reset_lock, flags); |  | ||||||
| 	for_each_possible_cpu (cpu) { | 	for_each_possible_cpu (cpu) { | ||||||
| 		per_cpu_ptr(&batched_entropy_u32, cpu)->position = 0; | 		struct batched_entropy *batched_entropy; | ||||||
| 		per_cpu_ptr(&batched_entropy_u64, cpu)->position = 0; | 
 | ||||||
|  | 		batched_entropy = per_cpu_ptr(&batched_entropy_u32, cpu); | ||||||
|  | 		spin_lock_irqsave(&batched_entropy->batch_lock, flags); | ||||||
|  | 		batched_entropy->position = 0; | ||||||
|  | 		spin_unlock(&batched_entropy->batch_lock); | ||||||
|  | 
 | ||||||
|  | 		batched_entropy = per_cpu_ptr(&batched_entropy_u64, cpu); | ||||||
|  | 		spin_lock(&batched_entropy->batch_lock); | ||||||
|  | 		batched_entropy->position = 0; | ||||||
|  | 		spin_unlock_irqrestore(&batched_entropy->batch_lock, flags); | ||||||
| 	} | 	} | ||||||
| 	write_unlock_irqrestore(&batched_entropy_reset_lock, flags); |  | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
| /**
 | /**
 | ||||||
|  |  | ||||||
		Loading…
	
		Reference in a new issue
	
	 Sebastian Andrzej Siewior
						Sebastian Andrzej Siewior