mirror of
				https://github.com/torvalds/linux.git
				synced 2025-11-04 10:40:15 +02:00 
			
		
		
		
	bpf: Fix syscall's stackmap lookup potential deadlock
The map_lookup_elem used to not acquiring spinlock in order to optimize the reader. It was true until commit557c0c6e7d("bpf: convert stackmap to pre-allocation") The syscall's map_lookup_elem(stackmap) calls bpf_stackmap_copy(). bpf_stackmap_copy() may find the elem no longer needed after the copy is done. If that is the case, pcpu_freelist_push() saves this elem for reuse later. This push requires a spinlock. If a tracing bpf_prog got run in the middle of the syscall's map_lookup_elem(stackmap) and this tracing bpf_prog is calling bpf_get_stackid(stackmap) which also requires the same pcpu_freelist's spinlock, it may end up with a dead lock situation as reported by Eric Dumazet in https://patchwork.ozlabs.org/patch/1030266/ The situation is the same as the syscall's map_update_elem() which needs to acquire the pcpu_freelist's spinlock and could race with tracing bpf_prog. Hence, this patch fixes it by protecting bpf_stackmap_copy() with this_cpu_inc(bpf_prog_active) to prevent tracing bpf_prog from running. A later syscall's map_lookup_elem commitf1a2e44a3a("bpf: add queue and stack maps") also acquires a spinlock and races with tracing bpf_prog similarly. Hence, this patch is forward looking and protects the majority of the map lookups. bpf_map_offload_lookup_elem() is the exception since it is for network bpf_prog only (i.e. never called by tracing bpf_prog). Fixes:557c0c6e7d("bpf: convert stackmap to pre-allocation") Reported-by: Eric Dumazet <eric.dumazet@gmail.com> Acked-by: Alexei Starovoitov <ast@kernel.org> Signed-off-by: Martin KaFai Lau <kafai@fb.com> Signed-off-by: Alexei Starovoitov <ast@kernel.org> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
This commit is contained in:
		
							parent
							
								
									e16ec34039
								
							
						
					
					
						commit
						7c4cd051ad
					
				
					 1 changed files with 10 additions and 2 deletions
				
			
		| 
						 | 
				
			
			@ -713,7 +713,12 @@ static int map_lookup_elem(union bpf_attr *attr)
 | 
			
		|||
 | 
			
		||||
	if (bpf_map_is_dev_bound(map)) {
 | 
			
		||||
		err = bpf_map_offload_lookup_elem(map, key, value);
 | 
			
		||||
	} else if (map->map_type == BPF_MAP_TYPE_PERCPU_HASH ||
 | 
			
		||||
		goto done;
 | 
			
		||||
	}
 | 
			
		||||
 | 
			
		||||
	preempt_disable();
 | 
			
		||||
	this_cpu_inc(bpf_prog_active);
 | 
			
		||||
	if (map->map_type == BPF_MAP_TYPE_PERCPU_HASH ||
 | 
			
		||||
	    map->map_type == BPF_MAP_TYPE_LRU_PERCPU_HASH) {
 | 
			
		||||
		err = bpf_percpu_hash_copy(map, key, value);
 | 
			
		||||
	} else if (map->map_type == BPF_MAP_TYPE_PERCPU_ARRAY) {
 | 
			
		||||
| 
						 | 
				
			
			@ -744,7 +749,10 @@ static int map_lookup_elem(union bpf_attr *attr)
 | 
			
		|||
		}
 | 
			
		||||
		rcu_read_unlock();
 | 
			
		||||
	}
 | 
			
		||||
	this_cpu_dec(bpf_prog_active);
 | 
			
		||||
	preempt_enable();
 | 
			
		||||
 | 
			
		||||
done:
 | 
			
		||||
	if (err)
 | 
			
		||||
		goto free_value;
 | 
			
		||||
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
		Loading…
	
		Reference in a new issue