@@ -282,7 +282,9 @@ void ovpn_encrypt_post(void *data, int ret)
switch (sock->sk->sk_protocol) {
case IPPROTO_UDP:
- ovpn_udp_send_skb(peer, sock->sk, skb);
+ ret = ovpn_udp_send_skb(peer, sock->sk, skb);
+ if (unlikely(ret < 0))
+ goto err_unlock;
break;
case IPPROTO_TCP:
ovpn_tcp_send_skb(peer, sock->sk, skb);
@@ -149,13 +149,20 @@ static int ovpn_udp4_output(struct ovpn_peer *peer, struct ovpn_bind *bind,
struct flowi4 fl = {
.saddr = bind->local.ipv4.s_addr,
.daddr = bind->remote.in4.sin_addr.s_addr,
- .fl4_sport = inet_sk(sk)->inet_sport,
+ .fl4_sport = READ_ONCE(inet_sk(sk)->inet_sport),
.fl4_dport = bind->remote.in4.sin_port,
.flowi4_proto = sk->sk_protocol,
.flowi4_mark = sk->sk_mark,
};
int ret;
+ /* an uninitialized socket or connect(AF_UNSPEC) can cause this */
+ if (unlikely(!fl.fl4_sport)) {
+ net_warn_ratelimited("%s: peer %u: UDP source port is 0\n",
+ netdev_name(peer->ovpn->dev), peer->id);
+ return -EADDRNOTAVAIL;
+ }
+
local_bh_disable();
rt = dst_cache_get_ip4(cache, &fl.saddr);
if (rt)
@@ -226,13 +233,20 @@ static int ovpn_udp6_output(struct ovpn_peer *peer, struct ovpn_bind *bind,
struct flowi6 fl = {
.saddr = bind->local.ipv6,
.daddr = bind->remote.in6.sin6_addr,
- .fl6_sport = inet_sk(sk)->inet_sport,
+ .fl6_sport = READ_ONCE(inet_sk(sk)->inet_sport),
.fl6_dport = bind->remote.in6.sin6_port,
.flowi6_proto = sk->sk_protocol,
.flowi6_mark = sk->sk_mark,
.flowi6_oif = bind->remote.in6.sin6_scope_id,
};
+ /* an uninitialized socket or connect(AF_UNSPEC) can cause this */
+ if (unlikely(!fl.fl6_sport)) {
+ net_warn_ratelimited("%s: peer %u: UDP source port is 0\n",
+ netdev_name(peer->ovpn->dev), peer->id);
+ return -EADDRNOTAVAIL;
+ }
+
local_bh_disable();
dst = dst_cache_get_ip6(cache, &fl.saddr);
if (dst)
@@ -289,7 +303,8 @@ static int ovpn_udp6_output(struct ovpn_peer *peer, struct ovpn_bind *bind,
* @skb: the packet to send
*
* rcu_read_lock should be held on entry.
- * On return, the skb is consumed.
+ * On success, the skb is passed to the transport stack and consumed. On
+ * error, ownership remains with the caller.
*
* Return: 0 on success or a negative error code otherwise
*/
@@ -336,21 +351,19 @@ static int ovpn_udp_output(struct ovpn_peer *peer, struct dst_cache *cache,
* @peer: the destination peer
* @sk: peer socket
* @skb: the packet to send
+ *
+ * Return: 0 on success or a negative error code otherwise
*/
-void ovpn_udp_send_skb(struct ovpn_peer *peer, struct sock *sk,
- struct sk_buff *skb)
+int ovpn_udp_send_skb(struct ovpn_peer *peer, struct sock *sk,
+ struct sk_buff *skb)
{
- int ret;
-
skb->dev = peer->ovpn->dev;
skb->mark = READ_ONCE(sk->sk_mark);
/* no checksum performed at this layer */
skb->ip_summed = CHECKSUM_NONE;
/* crypto layer -> transport (UDP) */
- ret = ovpn_udp_output(peer, &peer->dst_cache, sk, skb);
- if (unlikely(ret < 0))
- kfree_skb(skb);
+ return ovpn_udp_output(peer, &peer->dst_cache, sk, skb);
}
static void ovpn_udp_encap_destroy(struct sock *sk)
@@ -19,7 +19,7 @@ int ovpn_udp_socket_attach(struct ovpn_socket *ovpn_sock, struct socket *sock,
struct ovpn_priv *ovpn);
void ovpn_udp_socket_detach(struct ovpn_socket *ovpn_sock);
-void ovpn_udp_send_skb(struct ovpn_peer *peer, struct sock *sk,
- struct sk_buff *skb);
+int ovpn_udp_send_skb(struct ovpn_peer *peer, struct sock *sk,
+ struct sk_buff *skb);
#endif /* _NET_OVPN_UDP_H_ */
ovpn operates on a userspace-owned UDP socket, which may be manipulated in various ways by userspace. If the socket is never bound, connected, or used for communication, it may not have a source port assigned. Similarly, if the socket was connect()'ed to AF_INET or AF_INET6, it can be disconnected by connect() with AF_UNSPEC, which resets the source port unless the socket was explicitly bound. Since we must not transmit packets with source port 0, gate UDP TX on the presence of a valid source port and drop packets otherwise. To avoid ambiguity, sample the current source port once before route lookup and header build and enforce the check on that value. Emit a ratelimited warning when this drop path is hit so the broken socket state is visible. Return local UDP output errors to the common TX completion path so locally dropped packets are not counted as successful transport TX and do not refresh the peer keepalive timer. Fixes: 08857b5ec5d9 ("ovpn: implement basic TX path (UDP)") Signed-off-by: Ralf Lici <ralf@mandelbit.com> --- Changes since v1 https://lore.kernel.org/openvpn-devel/20260526124544.425791-1-ralf@mandelbit.com/ - Emit a ratelimited warning when UDP TX sees source port 0. - Make ovpn_udp_send_skb return local UDP output errors instead of freeing the skb internally. - Propagate local UDP TX errors to ovpn_encrypt_post so failed local drops do not update link TX stats or last_sent. - Update UDP TX kdoc to document skb ownership on success/error. drivers/net/ovpn/io.c | 4 +++- drivers/net/ovpn/udp.c | 33 +++++++++++++++++++++++---------- drivers/net/ovpn/udp.h | 4 ++-- 3 files changed, 28 insertions(+), 13 deletions(-)