Commit 99ba2b5a authored by John Fastabend's avatar John Fastabend Committed by Alexei Starovoitov
Browse files

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: default avatar <syzbot+b680e42077a0d7c9a0c4@syzkaller.appspotmail.com>
Fixes: e9db4ef6 ("bpf: sockhash fix omitted bucket lock in sock_close")
Signed-off-by: default avatarJohn Fastabend <john.fastabend@gmail.com>
Signed-off-by: default avatarAlexei Starovoitov <ast@kernel.org>
parent 0c6bc6e5

kernel/bpf/sockmap.c

+15 −0
Original line number Diff line number Diff line
@@ -312,10 +312,12 @@ static void bpf_tcp_close(struct sock *sk, long timeout) 
	struct smap_psock *psock;

	struct sock *osk;



	lock_sock(sk);

	rcu_read_lock();

	psock = smap_psock_sk(sk);

	if (unlikely(!psock)) {

		rcu_read_unlock();

		release_sock(sk);

		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);

	}

	rcu_read_unlock();

	release_sock(sk);

	close_fun(sk, timeout);

}
@@ -2069,7 +2072,13 @@ static int sock_map_update_elem(struct bpf_map *map, 
		return -EOPNOTSUPP;

	}



	lock_sock(skops.sk);

	preempt_disable();

	rcu_read_lock();

	err = sock_map_ctx_update_elem(&skops, map, key, flags);

	rcu_read_unlock();

	preempt_enable();

	release_sock(skops.sk);

	fput(socket->file);

	return err;

}
@@ -2410,7 +2419,13 @@ static int sock_hash_update_elem(struct bpf_map *map, 
		return -EINVAL;

	}



	lock_sock(skops.sk);

	preempt_disable();

	rcu_read_lock();

	err = sock_hash_ctx_update_elem(&skops, map, key, flags);

	rcu_read_unlock();

	preempt_enable();

	release_sock(skops.sk);

	fput(socket->file);

	return err;

}

kernel/bpf/syscall.c

+3 −1
Original line number Diff line number Diff line
@@ -735,7 +735,9 @@ static int map_update_elem(union bpf_attr *attr) 
	if (bpf_map_is_dev_bound(map)) {

		err = bpf_map_offload_update_elem(map, key, value, attr->flags);

		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);

		goto out;

	}

CRA Git | Maintained and supported by SUSTech CRA and CCSE