Skip to content

Commit 99ba2b5

Browse files
jrfastabAlexei Starovoitov
authored andcommitted
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: 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>
1 parent 0c6bc6e commit 99ba2b5

File tree

2 files changed

+18
-1
lines changed

2 files changed

+18
-1
lines changed

kernel/bpf/sockmap.c

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -312,10 +312,12 @@ static void bpf_tcp_close(struct sock *sk, long timeout)
312312
struct smap_psock *psock;
313313
struct sock *osk;
314314

315+
lock_sock(sk);
315316
rcu_read_lock();
316317
psock = smap_psock_sk(sk);
317318
if (unlikely(!psock)) {
318319
rcu_read_unlock();
320+
release_sock(sk);
319321
return sk->sk_prot->close(sk, timeout);
320322
}
321323

@@ -371,6 +373,7 @@ static void bpf_tcp_close(struct sock *sk, long timeout)
371373
e = psock_map_pop(sk, psock);
372374
}
373375
rcu_read_unlock();
376+
release_sock(sk);
374377
close_fun(sk, timeout);
375378
}
376379

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

2075+
lock_sock(skops.sk);
2076+
preempt_disable();
2077+
rcu_read_lock();
20722078
err = sock_map_ctx_update_elem(&skops, map, key, flags);
2079+
rcu_read_unlock();
2080+
preempt_enable();
2081+
release_sock(skops.sk);
20732082
fput(socket->file);
20742083
return err;
20752084
}
@@ -2410,7 +2419,13 @@ static int sock_hash_update_elem(struct bpf_map *map,
24102419
return -EINVAL;
24112420
}
24122421

2422+
lock_sock(skops.sk);
2423+
preempt_disable();
2424+
rcu_read_lock();
24132425
err = sock_hash_ctx_update_elem(&skops, map, key, flags);
2426+
rcu_read_unlock();
2427+
preempt_enable();
2428+
release_sock(skops.sk);
24142429
fput(socket->file);
24152430
return err;
24162431
}

kernel/bpf/syscall.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -735,7 +735,9 @@ static int map_update_elem(union bpf_attr *attr)
735735
if (bpf_map_is_dev_bound(map)) {
736736
err = bpf_map_offload_update_elem(map, key, value, attr->flags);
737737
goto out;
738-
} else if (map->map_type == BPF_MAP_TYPE_CPUMAP) {
738+
} else if (map->map_type == BPF_MAP_TYPE_CPUMAP ||
739+
map->map_type == BPF_MAP_TYPE_SOCKHASH ||
740+
map->map_type == BPF_MAP_TYPE_SOCKMAP) {
739741
err = map->ops->map_update_elem(map, key, value, attr->flags);
740742
goto out;
741743
}

0 commit comments

Comments
 (0)