mirror of
				https://github.com/torvalds/linux.git
				synced 2025-11-04 02:30:34 +02:00 
			
		
		
		
	bpf: sockhash, disallow bpf_tcp_close and update in parallel
After latest lock updates there is no longer anything preventing a
close and recvmsg call running in parallel. Additionally, we can
race update with close if we close a socket and simultaneously update
if via the BPF userspace API (note the cgroup ops are already run
with sock_lock held).
To resolve this take sock_lock in close and update paths.
Reported-by: syzbot+b680e42077a0d7c9a0c4@syzkaller.appspotmail.com
Fixes: e9db4ef6bf ("bpf: sockhash fix omitted bucket lock in sock_close")
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
			
			
This commit is contained in:
		
							parent
							
								
									0c6bc6e531
								
							
						
					
					
						commit
						99ba2b5aba
					
				
					 2 changed files with 18 additions and 1 deletions
				
			
		| 
						 | 
					@ -312,10 +312,12 @@ static void bpf_tcp_close(struct sock *sk, long timeout)
 | 
				
			||||||
	struct smap_psock *psock;
 | 
						struct smap_psock *psock;
 | 
				
			||||||
	struct sock *osk;
 | 
						struct sock *osk;
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
						lock_sock(sk);
 | 
				
			||||||
	rcu_read_lock();
 | 
						rcu_read_lock();
 | 
				
			||||||
	psock = smap_psock_sk(sk);
 | 
						psock = smap_psock_sk(sk);
 | 
				
			||||||
	if (unlikely(!psock)) {
 | 
						if (unlikely(!psock)) {
 | 
				
			||||||
		rcu_read_unlock();
 | 
							rcu_read_unlock();
 | 
				
			||||||
 | 
							release_sock(sk);
 | 
				
			||||||
		return sk->sk_prot->close(sk, timeout);
 | 
							return sk->sk_prot->close(sk, timeout);
 | 
				
			||||||
	}
 | 
						}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
| 
						 | 
					@ -371,6 +373,7 @@ static void bpf_tcp_close(struct sock *sk, long timeout)
 | 
				
			||||||
		e = psock_map_pop(sk, psock);
 | 
							e = psock_map_pop(sk, psock);
 | 
				
			||||||
	}
 | 
						}
 | 
				
			||||||
	rcu_read_unlock();
 | 
						rcu_read_unlock();
 | 
				
			||||||
 | 
						release_sock(sk);
 | 
				
			||||||
	close_fun(sk, timeout);
 | 
						close_fun(sk, timeout);
 | 
				
			||||||
}
 | 
					}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
| 
						 | 
					@ -2069,7 +2072,13 @@ static int sock_map_update_elem(struct bpf_map *map,
 | 
				
			||||||
		return -EOPNOTSUPP;
 | 
							return -EOPNOTSUPP;
 | 
				
			||||||
	}
 | 
						}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
						lock_sock(skops.sk);
 | 
				
			||||||
 | 
						preempt_disable();
 | 
				
			||||||
 | 
						rcu_read_lock();
 | 
				
			||||||
	err = sock_map_ctx_update_elem(&skops, map, key, flags);
 | 
						err = sock_map_ctx_update_elem(&skops, map, key, flags);
 | 
				
			||||||
 | 
						rcu_read_unlock();
 | 
				
			||||||
 | 
						preempt_enable();
 | 
				
			||||||
 | 
						release_sock(skops.sk);
 | 
				
			||||||
	fput(socket->file);
 | 
						fput(socket->file);
 | 
				
			||||||
	return err;
 | 
						return err;
 | 
				
			||||||
}
 | 
					}
 | 
				
			||||||
| 
						 | 
					@ -2410,7 +2419,13 @@ static int sock_hash_update_elem(struct bpf_map *map,
 | 
				
			||||||
		return -EINVAL;
 | 
							return -EINVAL;
 | 
				
			||||||
	}
 | 
						}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
						lock_sock(skops.sk);
 | 
				
			||||||
 | 
						preempt_disable();
 | 
				
			||||||
 | 
						rcu_read_lock();
 | 
				
			||||||
	err = sock_hash_ctx_update_elem(&skops, map, key, flags);
 | 
						err = sock_hash_ctx_update_elem(&skops, map, key, flags);
 | 
				
			||||||
 | 
						rcu_read_unlock();
 | 
				
			||||||
 | 
						preempt_enable();
 | 
				
			||||||
 | 
						release_sock(skops.sk);
 | 
				
			||||||
	fput(socket->file);
 | 
						fput(socket->file);
 | 
				
			||||||
	return err;
 | 
						return err;
 | 
				
			||||||
}
 | 
					}
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -735,7 +735,9 @@ static int map_update_elem(union bpf_attr *attr)
 | 
				
			||||||
	if (bpf_map_is_dev_bound(map)) {
 | 
						if (bpf_map_is_dev_bound(map)) {
 | 
				
			||||||
		err = bpf_map_offload_update_elem(map, key, value, attr->flags);
 | 
							err = bpf_map_offload_update_elem(map, key, value, attr->flags);
 | 
				
			||||||
		goto out;
 | 
							goto out;
 | 
				
			||||||
	} else if (map->map_type == BPF_MAP_TYPE_CPUMAP) {
 | 
						} else if (map->map_type == BPF_MAP_TYPE_CPUMAP ||
 | 
				
			||||||
 | 
							   map->map_type == BPF_MAP_TYPE_SOCKHASH ||
 | 
				
			||||||
 | 
							   map->map_type == BPF_MAP_TYPE_SOCKMAP) {
 | 
				
			||||||
		err = map->ops->map_update_elem(map, key, value, attr->flags);
 | 
							err = map->ops->map_update_elem(map, key, value, attr->flags);
 | 
				
			||||||
		goto out;
 | 
							goto out;
 | 
				
			||||||
	}
 | 
						}
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
		Loading…
	
		Reference in a new issue