net: annotate data-races around dev->if_port
authorEric Dumazet <edumazet@google.com>
Tue, 7 May 2024 18:41:44 +0000 (18:41 +0000)
committerJakub Kicinski <kuba@kernel.org>
Thu, 9 May 2024 01:51:30 +0000 (18:51 -0700)
Various ndo_set_config() methods can change dev->if_port

dev->if_port is going to be read locklessly from
rtnl_fill_link_ifmap().

Add corresponding WRITE_ONCE() on writer sides.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Reviewed-by: Simon Horman <horms@kernel.org>
Link: https://lore.kernel.org/r/20240507184144.1230469-1-edumazet@google.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
drivers/net/ethernet/3com/3c589_cs.c
drivers/net/ethernet/8390/etherh.c
drivers/net/ethernet/8390/pcnet_cs.c
drivers/net/ethernet/amd/nmclan_cs.c
drivers/net/ethernet/sis/sis900.c
drivers/net/ethernet/smsc/smc91c92_cs.c
drivers/net/ethernet/xircom/xirc2ps_cs.c

index 5267e9d..be58dac 100644 (file)
@@ -502,7 +502,7 @@ static int el3_config(struct net_device *dev, struct ifmap *map)
 {
        if ((map->port != (u_char)(-1)) && (map->port != dev->if_port)) {
                if (map->port <= 3) {
-                       dev->if_port = map->port;
+                       WRITE_ONCE(dev->if_port, map->port);
                        netdev_info(dev, "switched to %s port\n", if_names[dev->if_port]);
                        tc589_set_xcvr(dev, dev->if_port);
                } else {
index 05d39ec..e876fe5 100644 (file)
@@ -258,7 +258,7 @@ static int etherh_set_config(struct net_device *dev, struct ifmap *map)
                 * media type, turn off automedia detection.
                 */
                dev->flags &= ~IFF_AUTOMEDIA;
-               dev->if_port = map->port;
+               WRITE_ONCE(dev->if_port, map->port);
                break;
 
        default:
index 9bd5e99..780fb4a 100644 (file)
@@ -994,7 +994,7 @@ static int set_config(struct net_device *dev, struct ifmap *map)
            return -EOPNOTSUPP;
        else if ((map->port < 1) || (map->port > 2))
            return -EINVAL;
-       dev->if_port = map->port;
+       WRITE_ONCE(dev->if_port, map->port);
        netdev_info(dev, "switched to %s port\n", if_names[dev->if_port]);
        NS8390_init(dev, 1);
     }
index 0dd391c..37054a6 100644 (file)
@@ -760,7 +760,7 @@ static int mace_config(struct net_device *dev, struct ifmap *map)
 {
   if ((map->port != (u_char)(-1)) && (map->port != dev->if_port)) {
     if (map->port <= 2) {
-      dev->if_port = map->port;
+      WRITE_ONCE(dev->if_port, map->port);
       netdev_info(dev, "switched to %s port\n", if_names[dev->if_port]);
     } else
       return -EINVAL;
index cb7fec2..85b8503 100644 (file)
@@ -2273,7 +2273,7 @@ static int sis900_set_config(struct net_device *dev, struct ifmap *map)
                 * (which seems to be different from the ifport(pcmcia) definition) */
                switch(map->port){
                case IF_PORT_UNKNOWN: /* use auto here */
-                       dev->if_port = map->port;
+                       WRITE_ONCE(dev->if_port, map->port);
                        /* we are going to change the media type, so the Link
                         * will be temporary down and we need to reflect that
                         * here. When the Link comes up again, it will be
@@ -2294,7 +2294,7 @@ static int sis900_set_config(struct net_device *dev, struct ifmap *map)
                        break;
 
                case IF_PORT_10BASET: /* 10BaseT */
-                       dev->if_port = map->port;
+                       WRITE_ONCE(dev->if_port, map->port);
 
                        /* we are going to change the media type, so the Link
                         * will be temporary down and we need to reflect that
@@ -2315,7 +2315,7 @@ static int sis900_set_config(struct net_device *dev, struct ifmap *map)
 
                case IF_PORT_100BASET: /* 100BaseT */
                case IF_PORT_100BASETX: /* 100BaseTx */
-                       dev->if_port = map->port;
+                       WRITE_ONCE(dev->if_port, map->port);
 
                        /* we are going to change the media type, so the Link
                         * will be temporary down and we need to reflect that
index 29bb19f..86e3ec2 100644 (file)
@@ -1595,7 +1595,7 @@ static int s9k_config(struct net_device *dev, struct ifmap *map)
            return -EOPNOTSUPP;
        else if (map->port > 2)
            return -EINVAL;
-       dev->if_port = map->port;
+       WRITE_ONCE(dev->if_port, map->port);
        netdev_info(dev, "switched to %s port\n", if_names[dev->if_port]);
        smc_reset(dev);
     }
index e9bc38f..a31d5d5 100644 (file)
@@ -1366,10 +1366,10 @@ do_config(struct net_device *dev, struct ifmap *map)
            return -EINVAL;
        if (!map->port) {
            local->probe_port = 1;
-           dev->if_port = 1;
+           WRITE_ONCE(dev->if_port, 1);
        } else {
            local->probe_port = 0;
-           dev->if_port = map->port;
+           WRITE_ONCE(dev->if_port, map->port);
        }
        netdev_info(dev, "switching to %s port\n", if_names[dev->if_port]);
        do_reset(dev,1);  /* not the fine way :-) */