netpoll: Ensure clean state on setup failures
authorBreno Leitao <leitao@debian.org>
Thu, 22 Aug 2024 11:10:47 +0000 (04:10 -0700)
committerJakub Kicinski <kuba@kernel.org>
Mon, 26 Aug 2024 16:25:44 +0000 (09:25 -0700)
Modify netpoll_setup() and __netpoll_setup() to ensure that the netpoll
structure (np) is left in a clean state if setup fails for any reason.
This prevents carrying over misconfigured fields in case of partial
setup success.

Key changes:
- np->dev is now set only after successful setup, ensuring it's always
  NULL if netpoll is not configured or if netpoll_setup() fails.
- np->local_ip is zeroed if netpoll setup doesn't complete successfully.
- Added DEBUG_NET_WARN_ON_ONCE() checks to catch unexpected states.
- Reordered some operations in __netpoll_setup() for better logical flow.

These changes improve the reliability of netpoll configuration, since it
assures that the structure is fully initialized or totally unset.

Suggested-by: Paolo Abeni <pabeni@redhat.com>
Signed-off-by: Breno Leitao <leitao@debian.org>
Link: https://patch.msgid.link/20240822111051.179850-2-leitao@debian.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
net/core/netpoll.c

index cff1f89719b6d6ba66b4687db03596d8362b6571..a1aea86cf98c18081b8e63001e280de11b870075 100644 (file)
@@ -624,12 +624,9 @@ int __netpoll_setup(struct netpoll *np, struct net_device *ndev)
        const struct net_device_ops *ops;
        int err;
 
-       np->dev = ndev;
-       strscpy(np->dev_name, ndev->name, IFNAMSIZ);
-
        if (ndev->priv_flags & IFF_DISABLE_NETPOLL) {
                np_err(np, "%s doesn't support polling, aborting\n",
-                      np->dev_name);
+                      ndev->name);
                err = -ENOTSUPP;
                goto out;
        }
@@ -647,7 +644,7 @@ int __netpoll_setup(struct netpoll *np, struct net_device *ndev)
 
                refcount_set(&npinfo->refcnt, 1);
 
-               ops = np->dev->netdev_ops;
+               ops = ndev->netdev_ops;
                if (ops->ndo_netpoll_setup) {
                        err = ops->ndo_netpoll_setup(ndev, npinfo);
                        if (err)
@@ -658,6 +655,8 @@ int __netpoll_setup(struct netpoll *np, struct net_device *ndev)
                refcount_inc(&npinfo->refcnt);
        }
 
+       np->dev = ndev;
+       strscpy(np->dev_name, ndev->name, IFNAMSIZ);
        npinfo->netpoll = np;
 
        /* last thing to do is link it to the net device structure */
@@ -675,6 +674,7 @@ EXPORT_SYMBOL_GPL(__netpoll_setup);
 int netpoll_setup(struct netpoll *np)
 {
        struct net_device *ndev = NULL;
+       bool ip_overwritten = false;
        struct in_device *in_dev;
        int err;
 
@@ -739,6 +739,7 @@ put_noaddr:
                        }
 
                        np->local_ip.ip = ifa->ifa_local;
+                       ip_overwritten = true;
                        np_info(np, "local IP %pI4\n", &np->local_ip.ip);
                } else {
 #if IS_ENABLED(CONFIG_IPV6)
@@ -755,6 +756,7 @@ put_noaddr:
                                            !!(ipv6_addr_type(&np->remote_ip.in6) & IPV6_ADDR_LINKLOCAL))
                                                continue;
                                        np->local_ip.in6 = ifp->addr;
+                                       ip_overwritten = true;
                                        err = 0;
                                        break;
                                }
@@ -785,6 +787,9 @@ put_noaddr:
        return 0;
 
 put:
+       DEBUG_NET_WARN_ON_ONCE(np->dev);
+       if (ip_overwritten)
+               memset(&np->local_ip, 0, sizeof(np->local_ip));
        netdev_put(ndev, &np->dev_tracker);
 unlock:
        rtnl_unlock();