bpf: avoid misuse of psock when TCP_ULP_BPF collides with another ULP
Currently we check sk_user_data is non NULL to determine if the sk
exists in a map. However, this is not sufficient to ensure the psock
or the ULP ops are not in use by another user, such as kcm or TLS. To
avoid this when adding a sock to a map also verify it is of the
correct ULP type. Additionally, when releasing a psock verify that
it is the TCP_ULP_BPF type before releasing the ULP. The error case
where we abort an update due to ULP collision can cause this error
path.
For example,
__sock_map_ctx_update_elem()
[...]
err = tcp_set_ulp_id(sock, TCP_ULP_BPF) <- collides with TLS
if (err) <- so err out here
goto out_free
[...]
out_free:
smap_release_sock() <- calling tcp_cleanup_ulp releases the
TLS ULP incorrectly.
Fixes: 2f857d0460
("bpf: sockmap, remove STRPARSER map_flags and add multi-map support")
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
This commit is contained in:
parent
97911e0ccb
commit
597222f72a
|
@ -1462,10 +1462,16 @@ static void smap_destroy_psock(struct rcu_head *rcu)
|
||||||
schedule_work(&psock->gc_work);
|
schedule_work(&psock->gc_work);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
static bool psock_is_smap_sk(struct sock *sk)
|
||||||
|
{
|
||||||
|
return inet_csk(sk)->icsk_ulp_ops == &bpf_tcp_ulp_ops;
|
||||||
|
}
|
||||||
|
|
||||||
static void smap_release_sock(struct smap_psock *psock, struct sock *sock)
|
static void smap_release_sock(struct smap_psock *psock, struct sock *sock)
|
||||||
{
|
{
|
||||||
if (refcount_dec_and_test(&psock->refcnt)) {
|
if (refcount_dec_and_test(&psock->refcnt)) {
|
||||||
tcp_cleanup_ulp(sock);
|
if (psock_is_smap_sk(sock))
|
||||||
|
tcp_cleanup_ulp(sock);
|
||||||
write_lock_bh(&sock->sk_callback_lock);
|
write_lock_bh(&sock->sk_callback_lock);
|
||||||
smap_stop_sock(psock, sock);
|
smap_stop_sock(psock, sock);
|
||||||
write_unlock_bh(&sock->sk_callback_lock);
|
write_unlock_bh(&sock->sk_callback_lock);
|
||||||
|
@ -1892,6 +1898,10 @@ static int __sock_map_ctx_update_elem(struct bpf_map *map,
|
||||||
* doesn't update user data.
|
* doesn't update user data.
|
||||||
*/
|
*/
|
||||||
if (psock) {
|
if (psock) {
|
||||||
|
if (!psock_is_smap_sk(sock)) {
|
||||||
|
err = -EBUSY;
|
||||||
|
goto out_progs;
|
||||||
|
}
|
||||||
if (READ_ONCE(psock->bpf_parse) && parse) {
|
if (READ_ONCE(psock->bpf_parse) && parse) {
|
||||||
err = -EBUSY;
|
err = -EBUSY;
|
||||||
goto out_progs;
|
goto out_progs;
|
||||||
|
|
Loading…
Reference in New Issue