| 94560267 | 12-Feb-2026 |
Antonio Quartulli <antonio@openvpn.net> |
ovpn: tcp - don't deref NULL sk_socket member after tcp_close()
When deleting a peer in case of keepalive expiration, the peer is removed from the OpenVPN hashtable and is temporary inserted in a "r
ovpn: tcp - don't deref NULL sk_socket member after tcp_close()
When deleting a peer in case of keepalive expiration, the peer is removed from the OpenVPN hashtable and is temporary inserted in a "release list" for further processing.
This happens in: ovpn_peer_keepalive_work() unlock_ovpn(release_list)
This processing includes detaching from the socket being used to talk to this peer, by restoring its original proto and socket ops/callbacks.
In case of TCP it may happen that, while the peer is sitting in the release list, userspace decides to close the socket. This will result in a concurrent execution of:
tcp_close(sk) __tcp_close(sk) sock_orphan(sk) sk_set_socket(sk, NULL)
The last function call will set sk->sk_socket to NULL.
When the releasing routine is resumed, ovpn_tcp_socket_detach() will attempt to dereference sk->sk_socket to restore its original ops member. This operation will crash due to sk->sk_socket being NULL.
Fix this race condition by testing-and-accessing sk->sk_socket atomically under sk->sk_callback_lock.
Link: https://lore.kernel.org/netdev/176996279620.3109699.15382994681575380467@eldamar.lan/ Link: https://github.com/OpenVPN/ovpn-net-next/issues/29 Signed-off-by: Antonio Quartulli <antonio@openvpn.net> Fixes: 11851cbd60ea ("ovpn: implement TCP transport") Link: https://patch.msgid.link/20260212213130.11497-1-antonio@openvpn.net Signed-off-by: Paolo Abeni <pabeni@redhat.com>
show more ...
|
| b660b13d | 30-Jan-2026 |
Ralf Lici <ralf@mandelbit.com> |
ovpn: fix VPN TX bytes counting
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
ovpn: fix VPN TX bytes counting
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>
show more ...
|
| a5ec7baa | 30-Jan-2026 |
Ralf Lici <ralf@mandelbit.com> |
ovpn: fix possible use-after-free in ovpn_net_xmit
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 us
ovpn: fix possible use-after-free in ovpn_net_xmit
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>
show more ...
|
| 2022d704 | 01-Jul-2025 |
Ralf Lici <ralf@mandelbit.com> |
ovpn: reset GSO metadata after decapsulation
The ovpn_netdev_write() function is responsible for injecting decapsulated and decrypted packets back into the local network stack.
Prior to this patch,
ovpn: reset GSO metadata after decapsulation
The ovpn_netdev_write() function is responsible for injecting decapsulated and decrypted packets back into the local network stack.
Prior to this patch, the skb could retain GSO metadata from the outer, encrypted tunnel packet. This original GSO metadata, relevant to the sender's transport context, becomes invalid and misleading for the tunnel/data path once the inner packet is exposed.
Leaving this stale metadata intact causes internal GSO validation checks further down the kernel's network stack (validate_xmit_skb()) to fail, leading to packet drops. The reasons for these failures vary by protocol, for example: - for ICMP, no offload handler is registered; - for TCP and UDP, the respective offload handlers return errors when comparing skb->len to the outdated skb_shinfo(skb)->gso_size.
By calling skb_gso_reset(skb) we ensure the inner packet is presented to gro_cells_receive() with a clean state, correctly indicating it is an individual packet from the perspective of the local stack.
This change eliminates the "Driver has suspect GRO implementation, TCP performance may be compromised" warning and improves overall TCP performance by allowing GSO/GRO to function as intended on the decapsulated traffic.
Fixes: 11851cbd60ea ("ovpn: implement TCP transport") Reported-by: Gert Doering <gert@greenie.muc.de> Closes: https://github.com/OpenVPN/ovpn-net-next/issues/4 Tested-by: Gert Doering <gert@greenie.muc.de> Signed-off-by: Ralf Lici <ralf@mandelbit.com> Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
show more ...
|
| af52020f | 25-Jun-2025 |
Antonio Quartulli <antonio@openvpn.net> |
ovpn: reject unexpected netlink attributes
Netlink ops do not expect all attributes to be always set, however this condition is not explicitly coded any where, leading the user to believe that all s
ovpn: reject unexpected netlink attributes
Netlink ops do not expect all attributes to be always set, however this condition is not explicitly coded any where, leading the user to believe that all sent attributes are somewhat processed.
Fix this behaviour by introducing explicit checks.
For CMD_OVPN_PEER_GET and CMD_OVPN_KEY_GET directly open-code the needed condition in the related ops handlers. While for all other ops use attribute subsets in the ovpn.yaml spec file.
Fixes: b7a63391aa98 ("ovpn: add basic netlink support") Reported-by: Ralf Lici <ralf@mandelbit.com> Closes: https://github.com/OpenVPN/ovpn-net-next/issues/19 Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
show more ...
|
| f78c75d8 | 17-Jun-2025 |
Petr Machata <petrm@nvidia.com> |
net: ipv6: Add a flags argument to ip6tunnel_xmit(), udp_tunnel6_xmit_skb()
ip6tunnel_xmit() erases the contents of the SKB control block. In order to be able to set particular IP6CB flags on the SK
net: ipv6: Add a flags argument to ip6tunnel_xmit(), udp_tunnel6_xmit_skb()
ip6tunnel_xmit() erases the contents of the SKB control block. In order to be able to set particular IP6CB flags on the SKB, add a corresponding parameter, and propagate it to udp_tunnel6_xmit_skb() as well.
In one of the following patches, VXLAN driver will use this facility to mark packets as subject to IPv6 multicast routing.
Signed-off-by: Petr Machata <petrm@nvidia.com> Reviewed-by: Ido Schimmel <idosch@nvidia.com> Reviewed-by: Nikolay Aleksandrov <razor@blackwall.org> Link: https://patch.msgid.link/acb4f9f3e40c3a931236c3af08a720b017fbfbfb.1750113335.git.petrm@nvidia.com Signed-off-by: Jakub Kicinski <kuba@kernel.org>
show more ...
|
| a6a5e87b | 28-May-2025 |
Antonio Quartulli <antonio@openvpn.net> |
ovpn: avoid sleep in atomic context in TCP RX error path
Upon error along the TCP data_ready event path, we have the following chain of calls:
strp_data_ready() ovpn_tcp_rcv() ovpn_peer_del()
ovpn: avoid sleep in atomic context in TCP RX error path
Upon error along the TCP data_ready event path, we have the following chain of calls:
strp_data_ready() ovpn_tcp_rcv() ovpn_peer_del() ovpn_socket_release()
Since strp_data_ready() may be invoked from softirq context, and ovpn_socket_release() may sleep, the above sequence may cause a sleep in atomic context like the following:
BUG: sleeping function called from invalid context at ./ovpn-backports-ovpn-net-next-main-6.15.0-rc5-20250522/drivers/net/ovpn/socket.c:71 in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 25, name: ksoftirqd/3 5 locks held by ksoftirqd/3/25: #0: ffffffe000cd0580 (rcu_read_lock){....}-{1:2}, at: netif_receive_skb+0xb8/0x5b0 OpenVPN/ovpn-backports#1: ffffffe000cd0580 (rcu_read_lock){....}-{1:2}, at: netif_receive_skb+0xb8/0x5b0 OpenVPN/ovpn-backports#2: ffffffe000cd0580 (rcu_read_lock){....}-{1:2}, at: ip_local_deliver_finish+0x66/0x1e0 OpenVPN/ovpn-backports#3: ffffffe003ce9818 (slock-AF_INET/1){+.-.}-{2:2}, at: tcp_v4_rcv+0x156e/0x17a0 OpenVPN/ovpn-backports#4: ffffffe000cd0580 (rcu_read_lock){....}-{1:2}, at: ovpn_tcp_data_ready+0x0/0x1b0 [ovpn] CPU: 3 PID: 25 Comm: ksoftirqd/3 Not tainted 5.10.104+ #0 Call Trace: walk_stackframe+0x0/0x1d0 show_stack+0x2e/0x44 dump_stack+0xc2/0x102 ___might_sleep+0x29c/0x2b0 __might_sleep+0x62/0xa0 ovpn_socket_release+0x24/0x2d0 [ovpn] unlock_ovpn+0x6e/0x190 [ovpn] ovpn_peer_del+0x13c/0x390 [ovpn] ovpn_tcp_rcv+0x280/0x560 [ovpn] __strp_recv+0x262/0x940 strp_recv+0x66/0x80 tcp_read_sock+0x122/0x410 strp_data_ready+0x156/0x1f0 ovpn_tcp_data_ready+0x92/0x1b0 [ovpn] tcp_data_ready+0x6c/0x150 tcp_rcv_established+0xb36/0xc50 tcp_v4_do_rcv+0x25e/0x380 tcp_v4_rcv+0x166a/0x17a0 ip_protocol_deliver_rcu+0x8c/0x250 ip_local_deliver_finish+0xf8/0x1e0 ip_local_deliver+0xc2/0x2d0 ip_rcv+0x1f2/0x330 __netif_receive_skb+0xfc/0x290 netif_receive_skb+0x104/0x5b0 br_pass_frame_up+0x190/0x3f0 br_handle_frame_finish+0x3e2/0x7a0 br_handle_frame+0x750/0xab0 __netif_receive_skb_core.constprop.0+0x4c0/0x17f0 __netif_receive_skb+0xc6/0x290 netif_receive_skb+0x104/0x5b0 xgmac_dma_rx+0x962/0xb40 __napi_poll.constprop.0+0x5a/0x350 net_rx_action+0x1fe/0x4b0 __do_softirq+0x1f8/0x85c run_ksoftirqd+0x80/0xd0 smpboot_thread_fn+0x1f0/0x3e0 kthread+0x1e6/0x210 ret_from_kernel_thread+0x8/0xc
Fix this issue by postponing the ovpn_peer_del() call to a scheduled worker, as we already do in ovpn_tcp_send_sock() for the very same reason.
Fixes: 11851cbd60ea ("ovpn: implement TCP transport") Reported-by: Qingfang Deng <dqfext@gmail.com> Closes: https://github.com/OpenVPN/ovpn-net-next/issues/13 Reviewed-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com> Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
show more ...
|
| ba499a07 | 30-Apr-2025 |
Antonio Quartulli <antonio@openvpn.net> |
ovpn: ensure sk is still valid during cleanup
Removing a peer while userspace attempts to close its transport socket triggers a race condition resulting in the following crash:
Oops: general protec
ovpn: ensure sk is still valid during cleanup
Removing a peer while userspace attempts to close its transport socket triggers a race condition resulting in the following crash:
Oops: general protection fault, probably for non-canonical address 0xdffffc0000000077: 0000 [#1] SMP KASAN KASAN: null-ptr-deref in range [0x00000000000003b8-0x00000000000003bf] CPU: 12 UID: 0 PID: 162 Comm: kworker/12:1 Tainted: G O 6.15.0-rc2-00635-g521139ac3840 #272 PREEMPT(full) Tainted: [O]=OOT_MODULE Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-20240910_120124-localhost 04/01/2014 Workqueue: events ovpn_peer_keepalive_work [ovpn] RIP: 0010:ovpn_socket_release+0x23c/0x500 [ovpn] Code: ea 03 80 3c 02 00 0f 85 71 02 00 00 48 b8 00 00 00 00 00 fc ff df 4d 8b 64 24 18 49 8d bc 24 be 03 00 00 48 89 fa 48 c1 ea 03 <0f> b6 14 02 48 89 f8 83 e0 07 83 c0 01 38 d0 7c 08 84 d2 0f 85 30 RSP: 0018:ffffc90000c9fb18 EFLAGS: 00010217 RAX: dffffc0000000000 RBX: ffff8881148d7940 RCX: ffffffff817787bb RDX: 0000000000000077 RSI: 0000000000000008 RDI: 00000000000003be RBP: ffffc90000c9fb30 R08: 0000000000000000 R09: fffffbfff0d3e840 R10: ffffffff869f4207 R11: 0000000000000000 R12: 0000000000000000 R13: ffff888115eb9300 R14: ffffc90000c9fbc8 R15: 000000000000000c FS: 0000000000000000(0000) GS:ffff8882b0151000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 00007f37266b6114 CR3: 00000000054a8000 CR4: 0000000000750ef0 PKRU: 55555554 Call Trace: <TASK> unlock_ovpn+0x8b/0xe0 [ovpn] ovpn_peer_keepalive_work+0xe3/0x540 [ovpn] ? ovpn_peers_free+0x780/0x780 [ovpn] ? lock_acquire+0x56/0x70 ? process_one_work+0x888/0x1740 process_one_work+0x933/0x1740 ? pwq_dec_nr_in_flight+0x10b0/0x10b0 ? move_linked_works+0x12d/0x2c0 ? assign_work+0x163/0x270 worker_thread+0x4d6/0xd90 ? preempt_count_sub+0x4c/0x70 ? process_one_work+0x1740/0x1740 kthread+0x36c/0x710 ? trace_preempt_on+0x8c/0x1e0 ? kthread_is_per_cpu+0xc0/0xc0 ? preempt_count_sub+0x4c/0x70 ? _raw_spin_unlock_irq+0x36/0x60 ? calculate_sigpending+0x7b/0xa0 ? kthread_is_per_cpu+0xc0/0xc0 ret_from_fork+0x3a/0x80 ? kthread_is_per_cpu+0xc0/0xc0 ret_from_fork_asm+0x11/0x20 </TASK> Modules linked in: ovpn(O)
This happens because the peer deletion operation reaches ovpn_socket_release() while ovpn_sock->sock (struct socket *) and its sk member (struct sock *) are still both valid. Here synchronize_rcu() is invoked, after which ovpn_sock->sock->sk becomes NULL, due to the concurrent socket closing triggered from userspace.
After having invoked synchronize_rcu(), ovpn_socket_release() will attempt dereferencing ovpn_sock->sock->sk, triggering the crash reported above.
The reason for accessing sk is that we need to retrieve its protocol and continue the cleanup routine accordingly.
This crash can be easily produced by running openvpn userspace in client mode with `--keepalive 10 20`, while entirely omitting this option on the server side. After 20 seconds ovpn will assume the peer (server) to be dead, will start removing it and will notify userspace. The latter will receive the notification and close the transport socket, thus triggering the crash.
To fix the race condition for good, we need to refactor struct ovpn_socket. Since ovpn is always only interested in the sock->sk member (struct sock *) we can directly hold a reference to it, raher than accessing it via its struct socket container.
This means changing "struct socket *ovpn_socket->sock" to "struct sock *ovpn_socket->sk".
While acquiring a reference to sk, we can increase its refcounter without affecting the socket close()/destroy() notification (which we rely on when userspace closes a socket we are using).
By increasing sk's refcounter we know we can dereference it in ovpn_socket_release() without incurring in any race condition anymore.
ovpn_socket_release() will ultimately decrease the reference counter.
Cc: Oleksandr Natalenko <oleksandr@natalenko.name> Fixes: 11851cbd60ea ("ovpn: implement TCP transport") Reported-by: Qingfang Deng <dqfext@gmail.com> Closes: https://github.com/OpenVPN/ovpn-net-next/issues/1 Tested-by: Gert Doering <gert@greenie.muc.de> Link: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg31575.html Reviewed-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com> Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
show more ...
|
| 40d48527 | 13-May-2025 |
Antonio Quartulli <antonio@openvpn.net> |
ovpn: fix check for skb_to_sgvec_nomark() return value
Depending on the data offset, skb_to_sgvec_nomark() may use less scatterlist elements than what was forecasted by the previous call to skb_cow_
ovpn: fix check for skb_to_sgvec_nomark() return value
Depending on the data offset, skb_to_sgvec_nomark() may use less scatterlist elements than what was forecasted by the previous call to skb_cow_data().
It specifically happens when 'skbheadlen(skb) < offset', because in this case we entirely skip the skb's head, which would have required its own scatterlist element.
For this reason, it doesn't make sense to check that skb_to_sgvec_nomark() returns the same value as skb_cow_data(), but we can rather check for errors only, as it happens in other parts of the kernel.
Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
show more ...
|
| 0ca74dfa | 09-May-2025 |
Antonio Quartulli <antonio@openvpn.net> |
ovpn: improve 'no route to host' debug message
When debugging a 'no route to host' error it can be beneficial to know the address of the unreachable destination. Print it along the debugging text.
ovpn: improve 'no route to host' debug message
When debugging a 'no route to host' error it can be beneficial to know the address of the unreachable destination. Print it along the debugging text.
While at it, add a missing parenthesis in a different debugging message inside ovpn_peer_endpoints_update().
Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
show more ...
|
| adcdaac5 | 30-Apr-2025 |
Antonio Quartulli <antonio@openvpn.net> |
ovpn: drop useless reg_state check in keepalive worker
The keepalive worker is cancelled before calling unregister_netdevice_queue(), therefore it will never hit a situation where the reg_state can
ovpn: drop useless reg_state check in keepalive worker
The keepalive worker is cancelled before calling unregister_netdevice_queue(), therefore it will never hit a situation where the reg_state can be different than NETDEV_REGISTERED.
For this reason, checking reg_state is useless and the condition can be removed.
Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
show more ...
|
| 47e8e9d2 | 07-May-2025 |
Antonio Quartulli <antonio@openvpn.net> |
ovpn: fix ndo_start_xmit return value on error
ndo_start_xmit is basically expected to always return NETDEV_TX_OK. However, in case of error, it was currently returning NET_XMIT_DROP, which is not a
ovpn: fix ndo_start_xmit return value on error
ndo_start_xmit is basically expected to always return NETDEV_TX_OK. However, in case of error, it was currently returning NET_XMIT_DROP, which is not a valid netdev_tx_t return value, leading to misinterpretation.
Change ndo_start_xmit to always return NETDEV_TX_OK to signal back to the caller that the packet was handled (even if dropped).
Effects of this bug can be seen when sending IPv6 packets having no peer to forward them to:
$ ip netns exec ovpn-server oping -c20 fd00:abcd:220:201::1 PING fd00:abcd:220:201::1 (fd00:abcd:220:201::1) 56 bytes of data.00:abcd:220:201 :1 ping_send failed: No buffer space available ping_sendto: No buffer space available ping_send failed: No buffer space available ping_sendto: No buffer space available ...
Fixes: c2d950c4672a ("ovpn: add basic interface creation/destruction/management routines") Reported-by: Gert Doering <gert@greenie.muc.de> Closes: https://github.com/OpenVPN/ovpn-net-next/issues/5 Tested-by: Gert Doering <gert@greenie.muc.de> Acked-by: Gert Doering <gert@greenie.muc.de> Link: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg31591.html Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
show more ...
|