mirror of
https://github.com/torvalds/linux.git
synced 2026-03-08 03:24:45 +01:00
ovpn: set sk_user_data before overriding callbacks
During initialization, we override socket callbacks and set sk_user_data
to an ovpn_socket instance. Currently, these two operations are
decoupled: callbacks are overridden before sk_user_data is set. While
existing callbacks perform safety checks for NULL or non-ovpn
sk_user_data, this condition causes a "half-formed" state where valid
packets arriving during attachment trigger error logs (e.g., "invoked on
non ovpn socket").
Set sk_user_data before overriding the callbacks so that it can be
accessed safely from them. Since we already check that the socket has no
sk_user_data before setting it, this remains safe even if an interrupt
accesses the socket after sk_user_data is set but before the callbacks
are overridden.
This also requires initializing all protocol-specific fields (such as
tcp_tx_work and peer links) before calling ovpn_socket_attach, ensuring
the ovpn_socket is fully formed before it becomes visible to any
callback.
Fixes: f6226ae7a0 ("ovpn: introduce the ovpn_socket object")
Signed-off-by: Ralf Lici <ralf@mandelbit.com>
Reviewed-by: Sabrina Dubroca <sd@queasysnail.net>
Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
This commit is contained in:
parent
bf9cf80cab
commit
93686c472e
3 changed files with 30 additions and 21 deletions
|
|
@ -200,24 +200,6 @@ struct ovpn_socket *ovpn_socket_new(struct socket *sock, struct ovpn_peer *peer)
|
|||
ovpn_sock->sk = sk;
|
||||
kref_init(&ovpn_sock->refcount);
|
||||
|
||||
/* the newly created ovpn_socket is holding reference to sk,
|
||||
* therefore we increase its refcounter.
|
||||
*
|
||||
* This ovpn_socket instance is referenced by all peers
|
||||
* using the same socket.
|
||||
*
|
||||
* ovpn_socket_release() will take care of dropping the reference.
|
||||
*/
|
||||
sock_hold(sk);
|
||||
|
||||
ret = ovpn_socket_attach(ovpn_sock, sock, peer);
|
||||
if (ret < 0) {
|
||||
sock_put(sk);
|
||||
kfree(ovpn_sock);
|
||||
ovpn_sock = ERR_PTR(ret);
|
||||
goto sock_release;
|
||||
}
|
||||
|
||||
/* TCP sockets are per-peer, therefore they are linked to their unique
|
||||
* peer
|
||||
*/
|
||||
|
|
@ -234,7 +216,28 @@ struct ovpn_socket *ovpn_socket_new(struct socket *sock, struct ovpn_peer *peer)
|
|||
GFP_KERNEL);
|
||||
}
|
||||
|
||||
rcu_assign_sk_user_data(sk, ovpn_sock);
|
||||
/* the newly created ovpn_socket is holding reference to sk,
|
||||
* therefore we increase its refcounter.
|
||||
*
|
||||
* This ovpn_socket instance is referenced by all peers
|
||||
* using the same socket.
|
||||
*
|
||||
* ovpn_socket_release() will take care of dropping the reference.
|
||||
*/
|
||||
sock_hold(sk);
|
||||
|
||||
ret = ovpn_socket_attach(ovpn_sock, sock, peer);
|
||||
if (ret < 0) {
|
||||
if (sk->sk_protocol == IPPROTO_TCP)
|
||||
ovpn_peer_put(peer);
|
||||
else if (sk->sk_protocol == IPPROTO_UDP)
|
||||
netdev_put(peer->ovpn->dev, &ovpn_sock->dev_tracker);
|
||||
|
||||
sock_put(sk);
|
||||
kfree(ovpn_sock);
|
||||
ovpn_sock = ERR_PTR(ret);
|
||||
}
|
||||
|
||||
sock_release:
|
||||
release_sock(sk);
|
||||
return ovpn_sock;
|
||||
|
|
|
|||
|
|
@ -487,6 +487,7 @@ int ovpn_tcp_socket_attach(struct ovpn_socket *ovpn_sock,
|
|||
/* make sure no pre-existing encapsulation handler exists */
|
||||
if (ovpn_sock->sk->sk_user_data)
|
||||
return -EBUSY;
|
||||
rcu_assign_sk_user_data(ovpn_sock->sk, ovpn_sock);
|
||||
|
||||
/* only a fully connected socket is expected. Connection should be
|
||||
* handled in userspace
|
||||
|
|
@ -495,13 +496,14 @@ int ovpn_tcp_socket_attach(struct ovpn_socket *ovpn_sock,
|
|||
net_err_ratelimited("%s: provided TCP socket is not in ESTABLISHED state: %d\n",
|
||||
netdev_name(peer->ovpn->dev),
|
||||
ovpn_sock->sk->sk_state);
|
||||
return -EINVAL;
|
||||
ret = -EINVAL;
|
||||
goto err;
|
||||
}
|
||||
|
||||
ret = strp_init(&peer->tcp.strp, ovpn_sock->sk, &cb);
|
||||
if (ret < 0) {
|
||||
DEBUG_NET_WARN_ON_ONCE(1);
|
||||
return ret;
|
||||
goto err;
|
||||
}
|
||||
|
||||
INIT_WORK(&peer->tcp.defer_del_work, ovpn_tcp_peer_del_work);
|
||||
|
|
@ -536,6 +538,9 @@ int ovpn_tcp_socket_attach(struct ovpn_socket *ovpn_sock,
|
|||
strp_check_rcv(&peer->tcp.strp);
|
||||
|
||||
return 0;
|
||||
err:
|
||||
rcu_assign_sk_user_data(ovpn_sock->sk, NULL);
|
||||
return ret;
|
||||
}
|
||||
|
||||
static void ovpn_tcp_close(struct sock *sk, long timeout)
|
||||
|
|
|
|||
|
|
@ -386,6 +386,7 @@ int ovpn_udp_socket_attach(struct ovpn_socket *ovpn_sock, struct socket *sock,
|
|||
struct ovpn_priv *ovpn)
|
||||
{
|
||||
struct udp_tunnel_sock_cfg cfg = {
|
||||
.sk_user_data = ovpn_sock,
|
||||
.encap_type = UDP_ENCAP_OVPNINUDP,
|
||||
.encap_rcv = ovpn_udp_encap_recv,
|
||||
.encap_destroy = ovpn_udp_encap_destroy,
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue