* [PATCH net 1/3] ovpn: set sk_user_data before overriding callbacks
2026-02-12 21:03 [PATCH net 0/3] pull request: ovpn for net 2026-02-12 Antonio Quartulli
@ 2026-02-12 21:03 ` Antonio Quartulli
2026-02-17 10:30 ` patchwork-bot+netdevbpf
2026-02-12 21:03 ` [PATCH net 2/3] ovpn: fix possible use-after-free in ovpn_net_xmit Antonio Quartulli
` (2 subsequent siblings)
3 siblings, 1 reply; 6+ messages in thread
From: Antonio Quartulli @ 2026-02-12 21:03 UTC (permalink / raw)
To: netdev
Cc: Ralf Lici, Sabrina Dubroca, Jakub Kicinski, Paolo Abeni,
Antonio Quartulli
From: Ralf Lici <ralf@mandelbit.com>
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: f6226ae7a0cd ("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>
---
drivers/net/ovpn/socket.c | 39 +++++++++++++++++++++------------------
drivers/net/ovpn/tcp.c | 9 +++++++--
drivers/net/ovpn/udp.c | 1 +
3 files changed, 29 insertions(+), 20 deletions(-)
diff --git a/drivers/net/ovpn/socket.c b/drivers/net/ovpn/socket.c
index 9750871ab65c..448cee3b3f9f 100644
--- a/drivers/net/ovpn/socket.c
+++ b/drivers/net/ovpn/socket.c
@@ -200,6 +200,22 @@ struct ovpn_socket *ovpn_socket_new(struct socket *sock, struct ovpn_peer *peer)
ovpn_sock->sk = sk;
kref_init(&ovpn_sock->refcount);
+ /* TCP sockets are per-peer, therefore they are linked to their unique
+ * peer
+ */
+ if (sk->sk_protocol == IPPROTO_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) {
+ /* in UDP we only link the ovpn instance since the socket is
+ * shared among multiple peers
+ */
+ ovpn_sock->ovpn = peer->ovpn;
+ netdev_hold(peer->ovpn->dev, &ovpn_sock->dev_tracker,
+ GFP_KERNEL);
+ }
+
/* the newly created ovpn_socket is holding reference to sk,
* therefore we increase its refcounter.
*
@@ -212,29 +228,16 @@ 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)
+ 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);
- goto sock_release;
- }
-
- /* TCP sockets are per-peer, therefore they are linked to their unique
- * peer
- */
- if (sk->sk_protocol == IPPROTO_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) {
- /* in UDP we only link the ovpn instance since the socket is
- * shared among multiple peers
- */
- ovpn_sock->ovpn = peer->ovpn;
- netdev_hold(peer->ovpn->dev, &ovpn_sock->dev_tracker,
- GFP_KERNEL);
}
- rcu_assign_sk_user_data(sk, ovpn_sock);
sock_release:
release_sock(sk);
return ovpn_sock;
diff --git a/drivers/net/ovpn/tcp.c b/drivers/net/ovpn/tcp.c
index 0d7f30360d87..f0b4e07ba924 100644
--- a/drivers/net/ovpn/tcp.c
+++ b/drivers/net/ovpn/tcp.c
@@ -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)
diff --git a/drivers/net/ovpn/udp.c b/drivers/net/ovpn/udp.c
index d6a0f7a0b75d..272b535ecaad 100644
--- a/drivers/net/ovpn/udp.c
+++ b/drivers/net/ovpn/udp.c
@@ -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,
--
2.52.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH net 1/3] ovpn: set sk_user_data before overriding callbacks
2026-02-12 21:03 ` [PATCH net 1/3] ovpn: set sk_user_data before overriding callbacks Antonio Quartulli
@ 2026-02-17 10:30 ` patchwork-bot+netdevbpf
0 siblings, 0 replies; 6+ messages in thread
From: patchwork-bot+netdevbpf @ 2026-02-17 10:30 UTC (permalink / raw)
To: Antonio Quartulli; +Cc: netdev, ralf, sd, kuba, pabeni
Hello:
This series was applied to netdev/net.git (main)
by Antonio Quartulli <antonio@openvpn.net>:
On Thu, 12 Feb 2026 22:03:27 +0100 you wrote:
> From: Ralf Lici <ralf@mandelbit.com>
>
> 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").
>
> [...]
Here is the summary with links:
- [net,1/3] ovpn: set sk_user_data before overriding callbacks
https://git.kernel.org/netdev/net/c/93686c472eb7
- [net,2/3] ovpn: fix possible use-after-free in ovpn_net_xmit
https://git.kernel.org/netdev/net/c/a5ec7baa44ea
- [net,3/3] ovpn: fix VPN TX bytes counting
https://git.kernel.org/netdev/net/c/b660b13d4c63
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH net 2/3] ovpn: fix possible use-after-free in ovpn_net_xmit
2026-02-12 21:03 [PATCH net 0/3] pull request: ovpn for net 2026-02-12 Antonio Quartulli
2026-02-12 21:03 ` [PATCH net 1/3] ovpn: set sk_user_data before overriding callbacks Antonio Quartulli
@ 2026-02-12 21:03 ` Antonio Quartulli
2026-02-12 21:03 ` [PATCH net 3/3] ovpn: fix VPN TX bytes counting Antonio Quartulli
2026-02-17 14:35 ` [PATCH net 0/3] pull request: ovpn for net 2026-02-12 Paolo Abeni
3 siblings, 0 replies; 6+ messages in thread
From: Antonio Quartulli @ 2026-02-12 21:03 UTC (permalink / raw)
To: netdev
Cc: Ralf Lici, Sabrina Dubroca, Jakub Kicinski, Paolo Abeni,
Antonio Quartulli
From: Ralf Lici <ralf@mandelbit.com>
When building the skb_list in ovpn_net_xmit, skb_share_check will free
the original skb if it is shared. The current implementation continues
to use the stale skb pointer for subsequent operations:
- peer lookup,
- skb_dst_drop (even though all segments produced by skb_gso_segment
will have a dst attached),
- ovpn_peer_stats_increment_tx.
Fix this by moving the peer lookup and skb_dst_drop before segmentation
so that the original skb is still valid when used. Return early if all
segments fail skb_share_check and the list ends up empty.
Also switch ovpn_peer_stats_increment_tx to use skb_list.next; the next
patch fixes the stats logic.
Fixes: 08857b5ec5d9 ("ovpn: implement basic TX path (UDP)")
Signed-off-by: Ralf Lici <ralf@mandelbit.com>
Reviewed-by: Sabrina Dubroca <sd@queasysnail.net>
Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
---
drivers/net/ovpn/io.c | 52 ++++++++++++++++++++++++++-----------------
1 file changed, 31 insertions(+), 21 deletions(-)
diff --git a/drivers/net/ovpn/io.c b/drivers/net/ovpn/io.c
index 3e9e7f8444b3..f70c58b10599 100644
--- a/drivers/net/ovpn/io.c
+++ b/drivers/net/ovpn/io.c
@@ -365,7 +365,27 @@ netdev_tx_t ovpn_net_xmit(struct sk_buff *skb, struct net_device *dev)
/* verify IP header size in network packet */
proto = ovpn_ip_check_protocol(skb);
if (unlikely(!proto || skb->protocol != proto))
- goto drop;
+ goto drop_no_peer;
+
+ /* retrieve peer serving the destination IP of this packet */
+ peer = ovpn_peer_get_by_dst(ovpn, skb);
+ if (unlikely(!peer)) {
+ switch (skb->protocol) {
+ case htons(ETH_P_IP):
+ net_dbg_ratelimited("%s: no peer to send data to dst=%pI4\n",
+ netdev_name(ovpn->dev),
+ &ip_hdr(skb)->daddr);
+ break;
+ case htons(ETH_P_IPV6):
+ net_dbg_ratelimited("%s: no peer to send data to dst=%pI6c\n",
+ netdev_name(ovpn->dev),
+ &ipv6_hdr(skb)->daddr);
+ break;
+ }
+ goto drop_no_peer;
+ }
+ /* dst was needed for peer selection - it can now be dropped */
+ skb_dst_drop(skb);
if (skb_is_gso(skb)) {
segments = skb_gso_segment(skb, 0);
@@ -396,34 +416,24 @@ netdev_tx_t ovpn_net_xmit(struct sk_buff *skb, struct net_device *dev)
__skb_queue_tail(&skb_list, curr);
}
- skb_list.prev->next = NULL;
- /* retrieve peer serving the destination IP of this packet */
- peer = ovpn_peer_get_by_dst(ovpn, skb);
- if (unlikely(!peer)) {
- switch (skb->protocol) {
- case htons(ETH_P_IP):
- net_dbg_ratelimited("%s: no peer to send data to dst=%pI4\n",
- netdev_name(ovpn->dev),
- &ip_hdr(skb)->daddr);
- break;
- case htons(ETH_P_IPV6):
- net_dbg_ratelimited("%s: no peer to send data to dst=%pI6c\n",
- netdev_name(ovpn->dev),
- &ipv6_hdr(skb)->daddr);
- break;
- }
- goto drop;
+ /* no segments survived: don't jump to 'drop' because we already
+ * incremented the counter for each failure in the loop
+ */
+ if (unlikely(skb_queue_empty(&skb_list))) {
+ ovpn_peer_put(peer);
+ return NETDEV_TX_OK;
}
- /* dst was needed for peer selection - it can now be dropped */
- skb_dst_drop(skb);
+ skb_list.prev->next = NULL;
- ovpn_peer_stats_increment_tx(&peer->vpn_stats, skb->len);
+ ovpn_peer_stats_increment_tx(&peer->vpn_stats, skb_list.next->len);
ovpn_send(ovpn, skb_list.next, peer);
return NETDEV_TX_OK;
drop:
+ ovpn_peer_put(peer);
+drop_no_peer:
dev_dstats_tx_dropped(ovpn->dev);
skb_tx_error(skb);
kfree_skb_list(skb);
--
2.52.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH net 3/3] ovpn: fix VPN TX bytes counting
2026-02-12 21:03 [PATCH net 0/3] pull request: ovpn for net 2026-02-12 Antonio Quartulli
2026-02-12 21:03 ` [PATCH net 1/3] ovpn: set sk_user_data before overriding callbacks Antonio Quartulli
2026-02-12 21:03 ` [PATCH net 2/3] ovpn: fix possible use-after-free in ovpn_net_xmit Antonio Quartulli
@ 2026-02-12 21:03 ` Antonio Quartulli
2026-02-17 14:35 ` [PATCH net 0/3] pull request: ovpn for net 2026-02-12 Paolo Abeni
3 siblings, 0 replies; 6+ messages in thread
From: Antonio Quartulli @ 2026-02-12 21:03 UTC (permalink / raw)
To: netdev
Cc: Ralf Lici, Sabrina Dubroca, Jakub Kicinski, Paolo Abeni,
Antonio Quartulli
From: Ralf Lici <ralf@mandelbit.com>
In ovpn_net_xmit, after GSO segmentation and segment processing, the
first segment on the list is used to increment VPN TX statistics, which
fails to account for any subsequent segments in the chain.
Fix this by accumulating the length of every segment that successfully
passes skb_share_check into a tx_bytes variable. This ensures the peer
statistics accurately reflect the total data volume sent, regardless of
whether the original packet was segmented.
Fixes: 04ca14955f9a ("ovpn: store tunnel and transport statistics")
Signed-off-by: Ralf Lici <ralf@mandelbit.com>
Reviewed-by: Sabrina Dubroca <sd@queasysnail.net>
Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
---
drivers/net/ovpn/io.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ovpn/io.c b/drivers/net/ovpn/io.c
index f70c58b10599..955c9a37e1f8 100644
--- a/drivers/net/ovpn/io.c
+++ b/drivers/net/ovpn/io.c
@@ -355,6 +355,7 @@ netdev_tx_t ovpn_net_xmit(struct sk_buff *skb, struct net_device *dev)
struct ovpn_priv *ovpn = netdev_priv(dev);
struct sk_buff *segments, *curr, *next;
struct sk_buff_head skb_list;
+ unsigned int tx_bytes = 0;
struct ovpn_peer *peer;
__be16 proto;
int ret;
@@ -414,6 +415,8 @@ netdev_tx_t ovpn_net_xmit(struct sk_buff *skb, struct net_device *dev)
continue;
}
+ /* only count what we actually send */
+ tx_bytes += curr->len;
__skb_queue_tail(&skb_list, curr);
}
@@ -426,7 +429,7 @@ netdev_tx_t ovpn_net_xmit(struct sk_buff *skb, struct net_device *dev)
}
skb_list.prev->next = NULL;
- ovpn_peer_stats_increment_tx(&peer->vpn_stats, skb_list.next->len);
+ ovpn_peer_stats_increment_tx(&peer->vpn_stats, tx_bytes);
ovpn_send(ovpn, skb_list.next, peer);
return NETDEV_TX_OK;
--
2.52.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH net 0/3] pull request: ovpn for net 2026-02-12
2026-02-12 21:03 [PATCH net 0/3] pull request: ovpn for net 2026-02-12 Antonio Quartulli
` (2 preceding siblings ...)
2026-02-12 21:03 ` [PATCH net 3/3] ovpn: fix VPN TX bytes counting Antonio Quartulli
@ 2026-02-17 14:35 ` Paolo Abeni
3 siblings, 0 replies; 6+ messages in thread
From: Paolo Abeni @ 2026-02-17 14:35 UTC (permalink / raw)
To: Antonio Quartulli, netdev; +Cc: Sabrina Dubroca, Jakub Kicinski
On 2/12/26 10:03 PM, Antonio Quartulli wrote:
> In this batch you can three fixes:
>
> 1) ensure that sk_user_data is set before configuring socket callbacks.
> This way we avoid dropping early packets received by the CB when the
> ovpn data is not yet configured.
>
> 2) fix use-after-free in ovpn_net_xmit by not blindly assuming that the
> first skb segment will always be valid. It may actually be released by
> skb_share_check.
>
> 3) properly increase TX stats, by counting bytes of all segments that
> have been properly processed, instead of just counting the first segment
> only.
>
> Please pull or let me know of any issue!
Pulled, thanks!
Paolo
^ permalink raw reply [flat|nested] 6+ messages in thread