ovpn: disable IPv4 redirects on MP interfaces

ovpn_mp_alloc() tried to disable SEND_REDIRECTS on a multipeer
interface, but it runs from ovpn_net_init() (->ndo_init), which
register_netdevice() invokes before the NETDEV_REGISTER notifier
chain. The IPv4 in_device is only created when that notifier reaches
inetdev_event() -> inetdev_init(), so __in_dev_get_rtnl() always
returned NULL at ndo_init time and the whole redirect-disabling block
(both the per-device and the per-netns IPV4_DEVCONF_ALL write) was
dead. MP interfaces therefore kept emitting ICMP redirects.

Disabling redirects only once is not enough either: the IPv4
in_device is destroyed and recreated when the interface is moved to a
different network namespace (NETDEV_UNREGISTER/NETDEV_REGISTER), and
the newly created in_device inherits the destination namespace
defaults, silently re-enabling SEND_REDIRECTS.

Disable redirects from ovpn_net_open() (->ndo_open) instead: it runs
every time the interface is brought up, including after the in_device
has been recreated, so the setting is always re-applied. This mirrors
what wireguard does in wg_open(). RTNL is held on the ndo_open() path,
so __in_dev_get_rtnl() is safe.

Fixes: 05003b408c20 ("ovpn: implement multi-peer support")
Signed-off-by: Antonio Quartulli <antonio@openvpn.net>

diff --git a/drivers/net/ovpn/main.c b/drivers/net/ovpn/main.c
index 9d9a0ff690d662d6596d610afbe2440fd734e5f1..3a04757d5c31e1dddde026878ce7925d6f3a903f 100644 (file)
--- a/drivers/net/ovpn/main.c
+++ b/drivers/net/ovpn/main.c
@@ -35,25 +35,11 @@ static void ovpn_priv_free(struct net_device *net)
 
 static int ovpn_mp_alloc(struct ovpn_priv *ovpn)
 {
-       struct in_device *dev_v4;
        int i;
 
        if (ovpn->mode != OVPN_MODE_MP)
                return 0;
 
-       dev_v4 = __in_dev_get_rtnl(ovpn->dev);
-       if (dev_v4) {
-               /* disable redirects as Linux gets confused by ovpn
-                * handling same-LAN routing.
-                * This happens because a multipeer interface is used as
-                * relay point between hosts in the same subnet, while
-                * in a classic LAN this would not be needed because the
-                * two hosts would be able to talk directly.
-                */
-               IN_DEV_CONF_SET(dev_v4, SEND_REDIRECTS, false);
-               IPV4_DEVCONF_ALL(dev_net(ovpn->dev), SEND_REDIRECTS) = false;
-       }
-
        /* the peer container is fairly large, therefore we allocate it only in
         * MP mode
         */
@@ -97,9 +83,38 @@ static void ovpn_net_uninit(struct net_device *dev)
        gro_cells_destroy(&ovpn->gro_cells);
 }
 
+static int ovpn_net_open(struct net_device *dev)
+{
+       struct ovpn_priv *ovpn = netdev_priv(dev);
+       struct in_device *dev_v4;
+
+       /* the IPv4 in_device (and thus its config) is recreated whenever the
+        * interface is moved to a new netns, so redirects must be disabled on
+        * every bring-up rather than once at creation time, otherwise the
+        * setting is silently lost after such a move
+        */
+       if (ovpn->mode == OVPN_MODE_MP) {
+               dev_v4 = __in_dev_get_rtnl(dev);
+               if (dev_v4) {
+                       /* disable redirects as Linux gets confused by ovpn
+                        * handling same-LAN routing.
+                        * This happens because a multipeer interface is used as
+                        * relay point between hosts in the same subnet, while
+                        * in a classic LAN this would not be needed because the
+                        * two hosts would be able to talk directly.
+                        */
+                       IN_DEV_CONF_SET(dev_v4, SEND_REDIRECTS, false);
+                       IPV4_DEVCONF_ALL(dev_net(dev), SEND_REDIRECTS) = false;
+               }
+       }
+
+       return 0;
+}
+
 static const struct net_device_ops ovpn_netdev_ops = {
        .ndo_init               = ovpn_net_init,
        .ndo_uninit             = ovpn_net_uninit,
+       .ndo_open               = ovpn_net_open,
        .ndo_start_xmit         = ovpn_net_xmit,
 };
 
@@ -183,6 +198,7 @@ static int ovpn_newlink(struct net_device *dev,
        struct ovpn_priv *ovpn = netdev_priv(dev);
        struct nlattr **data = params->data;
        enum ovpn_mode mode = OVPN_MODE_P2P;
+       int ret;
 
        if (data && data[IFLA_OVPN_MODE]) {
                mode = nla_get_u8(data[IFLA_OVPN_MODE]);
@@ -207,7 +223,11 @@ static int ovpn_newlink(struct net_device *dev,
        else
                netif_carrier_off(dev);
 
-       return register_netdevice(dev);
+       ret = register_netdevice(dev);
+       if (ret < 0)
+               return ret;
+
+       return 0;
 }
 
 static size_t ovpn_get_size(const struct net_device *dev)