@@ -400,10 +400,21 @@ int ovpn_nl_peer_new_doit(struct sk_buff *skb, struct genl_info *info)
goto peer_release;
}
+ /* sk_protocol is not enough to determine if this is a real UDP or TCP
+ * socket
+ */
+ if (!sk_is_udp(sock->sk) && !sk_is_tcp(sock->sk)) {
+ NL_SET_ERR_MSG_FMT_MOD(info->extack,
+ "socket is not TCP or UDP");
+ sockfd_put(sock);
+ ret = -EOPNOTSUPP;
+ goto peer_release;
+ }
+
/* Only when using UDP as transport protocol the remote endpoint
* can be configured so that ovpn knows where to send packets to.
*/
- if (sock->sk->sk_protocol == IPPROTO_UDP &&
+ if (sk_is_udp(sock->sk) &&
!attrs[OVPN_A_PEER_REMOTE_IPV4] &&
!attrs[OVPN_A_PEER_REMOTE_IPV6]) {
NL_SET_ERR_MSG_FMT_MOD(info->extack,
@@ -417,7 +428,7 @@ int ovpn_nl_peer_new_doit(struct sk_buff *skb, struct genl_info *info)
* will just send bytes over it, without the need to specify a
* destination.
*/
- if (sock->sk->sk_protocol == IPPROTO_TCP &&
+ if (sk_is_tcp(sock->sk) &&
(attrs[OVPN_A_PEER_REMOTE_IPV4] ||
attrs[OVPN_A_PEER_REMOTE_IPV6])) {
NL_SET_ERR_MSG_FMT_MOD(info->extack,
@@ -126,13 +126,15 @@ static int ovpn_socket_attach(struct ovpn_socket *ovpn_sock,
/**
* ovpn_socket_new - create a new socket and initialize it
- * @sock: the kernel socket to embed
+ * @sock: the kernel socket to embed; must be a real UDP or TCP socket
* @peer: the peer reachable via this socket
*
* Return: an openvpn socket on success or a negative error code otherwise
*/
struct ovpn_socket *ovpn_socket_new(struct socket *sock, struct ovpn_peer *peer)
{
+ const bool tcp = sk_is_tcp(sock->sk);
+ const bool udp = sk_is_udp(sock->sk);
struct ovpn_socket *ovpn_sock;
struct sock *sk = sock->sk;
int ret;
@@ -142,7 +144,7 @@ struct ovpn_socket *ovpn_socket_new(struct socket *sock, struct ovpn_peer *peer)
/* a TCP socket can only be owned by a single peer, therefore there
* can't be any other user
*/
- if (sk->sk_protocol == IPPROTO_TCP && sk->sk_user_data) {
+ if (tcp && sk->sk_user_data) {
ovpn_sock = ERR_PTR(-EBUSY);
goto sock_release;
}
@@ -150,7 +152,7 @@ struct ovpn_socket *ovpn_socket_new(struct socket *sock, struct ovpn_peer *peer)
/* a UDP socket can be shared across multiple peers, but we must make
* sure it is not owned by something else
*/
- if (sk->sk_protocol == IPPROTO_UDP) {
+ if (udp) {
u8 type = READ_ONCE(udp_sk(sk)->encap_type);
/* socket owned by other encapsulation module */
@@ -203,11 +205,11 @@ struct ovpn_socket *ovpn_socket_new(struct socket *sock, struct ovpn_peer *peer)
/* TCP sockets are per-peer, therefore they are linked to their unique
* peer
*/
- if (sk->sk_protocol == IPPROTO_TCP) {
+ if (tcp) {
INIT_WORK(&ovpn_sock->tcp_tx_work, ovpn_tcp_tx_work);
ovpn_sock->peer = peer;
ovpn_peer_hold(peer);
- } else if (sk->sk_protocol == IPPROTO_UDP) {
+ } else if (udp) {
/* in UDP we only link the ovpn instance since the socket is
* shared among multiple peers
*/
@@ -228,9 +230,9 @@ struct ovpn_socket *ovpn_socket_new(struct socket *sock, struct ovpn_peer *peer)
ret = ovpn_socket_attach(ovpn_sock, sock, peer);
if (ret < 0) {
- if (sk->sk_protocol == IPPROTO_TCP)
+ if (tcp)
ovpn_peer_put(peer);
- else if (sk->sk_protocol == IPPROTO_UDP)
+ else if (udp)
netdev_put(peer->ovpn->dev, &ovpn_sock->dev_tracker);
sock_put(sk);
ovpn accepts a userspace-provided socket and attaches transport-specific state to it. The current checks use sk_protocol to select the UDP or TCP attach path, but sk_protocol alone does not identify the socket layout. For example, a raw socket can have sk_protocol set to IPPROTO_UDP while its storage is not a struct udp_sock. Passing such a socket to the UDP attach path would make ovpn read and write udp_sock fields on the wrong object, potentially accessing memory beyond the actual socket storage. Reject sockets unless they are real UDP datagram or TCP stream sockets before attaching them to ovpn in the peer creation path. This lets netlink report a clear error before calling the socket attach helper. Also switch ovpn_socket_new to sk_is_tcp and sk_is_udp, matching the netlink validation performed before the helper is called. This does not change the accepted socket types, but makes the helper's assumptions explicit. Fixes: f6226ae7a0cd ("ovpn: introduce the ovpn_socket object") Fixes: 1d36a36f6d53 ("ovpn: implement peer add/get/dump/delete via netlink") Signed-off-by: Ralf Lici <ralf@mandelbit.com> --- drivers/net/ovpn/netlink.c | 15 +++++++++++++-- drivers/net/ovpn/socket.c | 16 +++++++++------- 2 files changed, 22 insertions(+), 9 deletions(-)