public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH net 0/1] pull request: fixes for ovpn 2026-04-22
@ 2026-04-22 12:32 Antonio Quartulli
  2026-04-22 12:32 ` [PATCH net 1/1] ovpn: fix race between deleting interface and adding new peer Antonio Quartulli
  0 siblings, 1 reply; 2+ messages in thread
From: Antonio Quartulli @ 2026-04-22 12:32 UTC (permalink / raw)
  To: netdev
  Cc: Antonio Quartulli, Sabrina Dubroca, Ralf Lici, Jakub Kicinski,
	Paolo Abeni, Andrew Lunn, David S. Miller, Eric Dumazet

Hello netdev team,

This PR is a new version of the one sent on March 20th.
We are trying to address a remote, yet possible, race condition between
tearing down an ovpn interface and adding new peers via netlink.

After further discussion including Jakub and Sabrina, the following
patch came up.

It should address all raised concerns.
Sorry for taking so long to get back to this.

Please pull or let mw know of any issue!

Thanks a lot,
	Antonio


The following changes since commit 478ed6b7d2577439c610f91fa8759a4c878a4264:

  net/sched: sch_dualpi2: drain both C-queue and L-queue in dualpi2_change() (2026-04-21 15:00:39 +0200)

are available in the Git repository at:

  https://github.com/OpenVPN/ovpn-net-next.git tags/ovpn-net-20260422

for you to fetch changes up to eac0dfc41d179d5f78d6d7ee401c63f48ba04b5a:

  ovpn: fix race between deleting interface and adding new peer (2026-04-22 14:30:17 +0200)

----------------------------------------------------------------
Included change:
* fix race condition between interface teardown and new peer being
  added via netlink

----------------------------------------------------------------
Antonio Quartulli (1):
      ovpn: fix race between deleting interface and adding new peer

 drivers/net/ovpn/main.c | 12 ++----------
 drivers/net/ovpn/peer.c |  8 ++++++++
 2 files changed, 10 insertions(+), 10 deletions(-)

^ permalink raw reply	[flat|nested] 2+ messages in thread

* [PATCH net 1/1] ovpn: fix race between deleting interface and adding new peer
  2026-04-22 12:32 [PATCH net 0/1] pull request: fixes for ovpn 2026-04-22 Antonio Quartulli
@ 2026-04-22 12:32 ` Antonio Quartulli
  0 siblings, 0 replies; 2+ messages in thread
From: Antonio Quartulli @ 2026-04-22 12:32 UTC (permalink / raw)
  To: netdev
  Cc: Antonio Quartulli, Sabrina Dubroca, Ralf Lici, Jakub Kicinski,
	Paolo Abeni, Andrew Lunn, David S. Miller, Eric Dumazet,
	Hyunwoo Kim

While deleting an existing ovpn interface, there is a very
narrow window where adding a new peer via netlink may cause
the netdevice to hang and prevent its unregistration.

It may happen during ovpn_dellink(), when all existing peers are
freed and the device is queued for deregistration, but a
CMD_PEER_NEW message comes in adding a new peer that takes again
a reference to the netdev.

At this point there is no way to release the device because we are
under the assumption that all peers were already released.

Fix the race condition by releasing all peers in ndo_uninit(),
when the netdevice has already been removed from the netdev
list. Also ovpn_peer_add() has now an extra check that
forces the function to bail out if the device reg_state is
UNREGISTERING (or later state).

This way any incoming CMD_PEER_NEW racing with the interface
deletion routine will simply stop before adding the peer.

At this point ovpn_dellink() becomes empty and can just be
removed.

Reported-by: Hyunwoo Kim <imv4bel@gmail.com>
Closes: https://lore.kernel.org/netdev/aaVgJ16edTfQkYbx@v4bel/
Suggested-by: Sabrina Dubroca <sd@queasysnail.net>
Fixes: 80747caef33d ("ovpn: introduce the ovpn_peer object")
Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
---
 drivers/net/ovpn/main.c | 12 ++----------
 drivers/net/ovpn/peer.c |  8 ++++++++
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ovpn/main.c b/drivers/net/ovpn/main.c
index 2e0420febda0..0eab305780c7 100644
--- a/drivers/net/ovpn/main.c
+++ b/drivers/net/ovpn/main.c
@@ -92,6 +92,8 @@ static void ovpn_net_uninit(struct net_device *dev)
 {
 	struct ovpn_priv *ovpn = netdev_priv(dev);
 
+	cancel_delayed_work_sync(&ovpn->keepalive_work);
+	ovpn_peers_free(ovpn, NULL, OVPN_DEL_PEER_REASON_TEARDOWN);
 	gro_cells_destroy(&ovpn->gro_cells);
 }
 
@@ -208,15 +210,6 @@ static int ovpn_newlink(struct net_device *dev,
 	return register_netdevice(dev);
 }
 
-static void ovpn_dellink(struct net_device *dev, struct list_head *head)
-{
-	struct ovpn_priv *ovpn = netdev_priv(dev);
-
-	cancel_delayed_work_sync(&ovpn->keepalive_work);
-	ovpn_peers_free(ovpn, NULL, OVPN_DEL_PEER_REASON_TEARDOWN);
-	unregister_netdevice_queue(dev, head);
-}
-
 static int ovpn_fill_info(struct sk_buff *skb, const struct net_device *dev)
 {
 	struct ovpn_priv *ovpn = netdev_priv(dev);
@@ -235,7 +228,6 @@ static struct rtnl_link_ops ovpn_link_ops = {
 	.policy = ovpn_policy,
 	.maxtype = IFLA_OVPN_MAX,
 	.newlink = ovpn_newlink,
-	.dellink = ovpn_dellink,
 	.fill_info = ovpn_fill_info,
 };
 
diff --git a/drivers/net/ovpn/peer.c b/drivers/net/ovpn/peer.c
index c02dfab51a6e..8ef485a8c851 100644
--- a/drivers/net/ovpn/peer.c
+++ b/drivers/net/ovpn/peer.c
@@ -1034,6 +1034,14 @@ static int ovpn_peer_add_p2p(struct ovpn_priv *ovpn, struct ovpn_peer *peer)
  */
 int ovpn_peer_add(struct ovpn_priv *ovpn, struct ovpn_peer *peer)
 {
+	/* Prevent adding new peers while destroying the ovpn interface.
+	 * Failing to do so would end up holding the device reference
+	 * endlessly hostage of the new peer object with no chance of
+	 * release..
+	 */
+	if (ovpn->dev->reg_state >= NETREG_UNREGISTERING)
+		return -ENODEV;
+
 	switch (ovpn->mode) {
 	case OVPN_MODE_MP:
 		return ovpn_peer_add_mp(ovpn, peer);
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-04-22 12:33 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-22 12:32 [PATCH net 0/1] pull request: fixes for ovpn 2026-04-22 Antonio Quartulli
2026-04-22 12:32 ` [PATCH net 1/1] ovpn: fix race between deleting interface and adding new peer Antonio Quartulli

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox