| Message ID | 20260202132309.567382-1-ralf@mandelbit.com |
|---|---|
| State | New |
| Headers | show |
| Series | [Openvpn-devel,ovpn,net] ovpn: detach TCP socket before invoking close | expand |
This patch is actually fixing the same issue reported here: https://lore.kernel.org/netdev/176996279620.3109699.15382994681575380467@eldamar.lan/ Cheers, On 02/02/2026 14:23, Ralf Lici wrote: > Currently, when a connected peer expires (no packets received within the > keepalive interval), we remove the peer and notify userspace of the > deletion. We then insert the peer in the release list and proceed to > detach and release the socket and the peer. This can be problematic with > TCP because, as soon as we send the notification, openvpn will close the > peer's socket and if ovpn_tcp_close is invoked before > ovpn_tcp_socket_detach we incurr in a NULL pointer dereference when > trying to access sk->sk_socket. > > Enforce correct ordering by calling ovpn_sock_release before invoking > the original socket close callback. This avoids potential race > conditions and guarantees that we completely detach from the socket once > userspace issues the close command. > > Signed-off-by: Ralf Lici <ralf@mandelbit.com> > --- > drivers/net/ovpn/tcp.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/net/ovpn/tcp.c b/drivers/net/ovpn/tcp.c > index 0d7f30360d87..13d2a8069695 100644 > --- a/drivers/net/ovpn/tcp.c > +++ b/drivers/net/ovpn/tcp.c > @@ -553,6 +553,7 @@ static void ovpn_tcp_close(struct sock *sk, long timeout) > rcu_read_unlock(); > > ovpn_peer_del(sock->peer, OVPN_DEL_PEER_REASON_TRANSPORT_DISCONNECT); > + ovpn_socket_release(peer); > peer->tcp.sk_cb.prot->close(sk, timeout); > ovpn_peer_put(peer); > }
On Mon, 2026-02-02 at 14:23 +0100, Ralf Lici wrote: > Currently, when a connected peer expires (no packets received within > the > keepalive interval), we remove the peer and notify userspace of the > deletion. We then insert the peer in the release list and proceed to > detach and release the socket and the peer. This can be problematic > with > TCP because, as soon as we send the notification, openvpn will close > the > peer's socket and if ovpn_tcp_close is invoked before > ovpn_tcp_socket_detach we incurr in a NULL pointer dereference when > trying to access sk->sk_socket. > > Enforce correct ordering by calling ovpn_sock_release before invoking > the original socket close callback. This avoids potential race > conditions and guarantees that we completely detach from the socket > once > userspace issues the close command. > Fixes: 11851cbd60ea ("ovpn: implement TCP transport") > Signed-off-by: Ralf Lici <ralf@mandelbit.com> > --- > drivers/net/ovpn/tcp.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/net/ovpn/tcp.c b/drivers/net/ovpn/tcp.c > index 0d7f30360d87..13d2a8069695 100644 > --- a/drivers/net/ovpn/tcp.c > +++ b/drivers/net/ovpn/tcp.c > @@ -553,6 +553,7 @@ static void ovpn_tcp_close(struct sock *sk, long > timeout) > rcu_read_unlock(); > > ovpn_peer_del(sock->peer, > OVPN_DEL_PEER_REASON_TRANSPORT_DISCONNECT); > + ovpn_socket_release(peer); > peer->tcp.sk_cb.prot->close(sk, timeout); > ovpn_peer_put(peer); > }
diff --git a/drivers/net/ovpn/tcp.c b/drivers/net/ovpn/tcp.c index 0d7f30360d87..13d2a8069695 100644 --- a/drivers/net/ovpn/tcp.c +++ b/drivers/net/ovpn/tcp.c @@ -553,6 +553,7 @@ static void ovpn_tcp_close(struct sock *sk, long timeout) rcu_read_unlock(); ovpn_peer_del(sock->peer, OVPN_DEL_PEER_REASON_TRANSPORT_DISCONNECT); + ovpn_socket_release(peer); peer->tcp.sk_cb.prot->close(sk, timeout); ovpn_peer_put(peer); }
Currently, when a connected peer expires (no packets received within the keepalive interval), we remove the peer and notify userspace of the deletion. We then insert the peer in the release list and proceed to detach and release the socket and the peer. This can be problematic with TCP because, as soon as we send the notification, openvpn will close the peer's socket and if ovpn_tcp_close is invoked before ovpn_tcp_socket_detach we incurr in a NULL pointer dereference when trying to access sk->sk_socket. Enforce correct ordering by calling ovpn_sock_release before invoking the original socket close callback. This avoids potential race conditions and guarantees that we completely detach from the socket once userspace issues the close command. Signed-off-by: Ralf Lici <ralf@mandelbit.com> --- drivers/net/ovpn/tcp.c | 1 + 1 file changed, 1 insertion(+)