mirror of
				https://github.com/torvalds/linux.git
				synced 2025-11-04 02:30:34 +02:00 
			
		
		
		
	bpf, sockmap: Prevent lock inversion deadlock in map delete elem
syzkaller started using corpuses where a BPF tracing program deletes
elements from a sockmap/sockhash map. Because BPF tracing programs can be
invoked from any interrupt context, locks taken during a map_delete_elem
operation must be hardirq-safe. Otherwise a deadlock due to lock inversion
is possible, as reported by lockdep:
       CPU0                    CPU1
       ----                    ----
  lock(&htab->buckets[i].lock);
                               local_irq_disable();
                               lock(&host->lock);
                               lock(&htab->buckets[i].lock);
  <Interrupt>
    lock(&host->lock);
Locks in sockmap are hardirq-unsafe by design. We expects elements to be
deleted from sockmap/sockhash only in task (normal) context with interrupts
enabled, or in softirq context.
Detect when map_delete_elem operation is invoked from a context which is
_not_ hardirq-unsafe, that is interrupts are disabled, and bail out with an
error.
Note that map updates are not affected by this issue. BPF verifier does not
allow updating sockmap/sockhash from a BPF tracing program today.
Fixes: 604326b41a ("bpf, sockmap: convert to generic sk_msg interface")
Reported-by: xingwei lee <xrivendell7@gmail.com>
Reported-by: yue sun <samsun1006219@gmail.com>
Reported-by: syzbot+bc922f476bd65abbd466@syzkaller.appspotmail.com
Reported-by: syzbot+d4066896495db380182e@syzkaller.appspotmail.com
Signed-off-by: Jakub Sitnicki <jakub@cloudflare.com>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Tested-by: syzbot+d4066896495db380182e@syzkaller.appspotmail.com
Acked-by: John Fastabend <john.fastabend@gmail.com>
Closes: https://syzkaller.appspot.com/bug?extid=d4066896495db380182e
Closes: https://syzkaller.appspot.com/bug?extid=bc922f476bd65abbd466
Link: https://lore.kernel.org/bpf/20240402104621.1050319-1-jakub@cloudflare.com
			
			
This commit is contained in:
		
							parent
							
								
									8c3fe029d7
								
							
						
					
					
						commit
						ff91059932
					
				
					 1 changed files with 6 additions and 0 deletions
				
			
		| 
						 | 
				
			
			@ -411,6 +411,9 @@ static int __sock_map_delete(struct bpf_stab *stab, struct sock *sk_test,
 | 
			
		|||
	struct sock *sk;
 | 
			
		||||
	int err = 0;
 | 
			
		||||
 | 
			
		||||
	if (irqs_disabled())
 | 
			
		||||
		return -EOPNOTSUPP; /* locks here are hardirq-unsafe */
 | 
			
		||||
 | 
			
		||||
	spin_lock_bh(&stab->lock);
 | 
			
		||||
	sk = *psk;
 | 
			
		||||
	if (!sk_test || sk_test == sk)
 | 
			
		||||
| 
						 | 
				
			
			@ -933,6 +936,9 @@ static long sock_hash_delete_elem(struct bpf_map *map, void *key)
 | 
			
		|||
	struct bpf_shtab_elem *elem;
 | 
			
		||||
	int ret = -ENOENT;
 | 
			
		||||
 | 
			
		||||
	if (irqs_disabled())
 | 
			
		||||
		return -EOPNOTSUPP; /* locks here are hardirq-unsafe */
 | 
			
		||||
 | 
			
		||||
	hash = sock_hash_bucket_hash(key, key_size);
 | 
			
		||||
	bucket = sock_hash_select_bucket(htab, hash);
 | 
			
		||||
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
		Loading…
	
		Reference in a new issue