Skip to content
This repository has been archived by the owner on Sep 24, 2020. It is now read-only.

Commit

Permalink
bpf: sockhash, disallow bpf_tcp_close and update in parallel
Browse files Browse the repository at this point in the history
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: e9db4ef ("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>
  • Loading branch information
jrfastab authored and Alexei Starovoitov committed Jul 7, 2018
1 parent 0c6bc6e commit 99ba2b5
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 1 deletion.
15 changes: 15 additions & 0 deletions kernel/bpf/sockmap.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}
Expand Down
4 changes: 3 additions & 1 deletion kernel/bpf/syscall.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down

0 comments on commit 99ba2b5

Please sign in to comment.