diff mbox series

[net-next] net: annotate data-races around dev->if_port

Message ID 20240507184144.1230469-1-edumazet@google.com (mailing list archive)
State Accepted
Commit 8d8b1a422c4644891e1f8a3ea10b544b65cd0cc6
Delegated to: Netdev Maintainers
Headers show
Series [net-next] net: annotate data-races around dev->if_port | expand

Checks

Context Check Description
netdev/series_format success Single patches do not need cover letters
netdev/tree_selection success Clearly marked for net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 929 this patch: 929
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers warning 7 maintainers not CCed: bagasdotme@gmail.com colin.i.king@gmail.com linux@armlinux.org.uk linux-arm-kernel@lists.infradead.org christophe.jaillet@wanadoo.fr horms@kernel.org venza@brownhat.org
netdev/build_clang success Errors and warnings before: 939 this patch: 939
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 940 this patch: 940
netdev/checkpatch warning WARNING: drivers/net/ethernet/8390/etherh.c is marked as 'obsolete' in the MAINTAINERS hierarchy. No unnecessary modifications please. WARNING: drivers/net/ethernet/8390/pcnet_cs.c is marked as 'obsolete' in the MAINTAINERS hierarchy. No unnecessary modifications please. WARNING: please, no spaces at the start of a line WARNING: suspect code indent for conditional statements (4, 6)
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 19 this patch: 19
netdev/source_inline success Was 0 now: 0
netdev/contest success net-next-2024-05-08--03-00 (tests: 1005)

Commit Message

Eric Dumazet May 7, 2024, 6:41 p.m. UTC 
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>
---
 drivers/net/ethernet/3com/3c589_cs.c     | 2 +-
 drivers/net/ethernet/8390/etherh.c       | 2 +-
 drivers/net/ethernet/8390/pcnet_cs.c     | 2 +-
 drivers/net/ethernet/amd/nmclan_cs.c     | 2 +-
 drivers/net/ethernet/sis/sis900.c        | 6 +++---
 drivers/net/ethernet/smsc/smc91c92_cs.c  | 2 +-
 drivers/net/ethernet/xircom/xirc2ps_cs.c | 4 ++--
 7 files changed, 10 insertions(+), 10 deletions(-)

Comments

Simon Horman May 8, 2024, 7:55 p.m. UTC | #1 
On Tue, May 07, 2024 at 06:41:44PM +0000, Eric Dumazet wrote:
> 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>
patchwork-bot+netdevbpf@kernel.org May 9, 2024, 2 a.m. UTC | #2 
Hello:

This patch was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Tue,  7 May 2024 18:41:44 +0000 you wrote:
> 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.
> 
> [...]

Here is the summary with links:
  - [net-next] net: annotate data-races around dev->if_port
    https://git.kernel.org/netdev/net-next/c/8d8b1a422c46

You are awesome, thank you!
diff mbox series

Patch

diff --git a/drivers/net/ethernet/3com/3c589_cs.c b/drivers/net/ethernet/3com/3c589_cs.c
index 5267e9dcd87ef90927e7806206dba7d108e96ddc..be58dac0502a6f777010e4005ef8537b830074d6 100644
--- a/drivers/net/ethernet/3com/3c589_cs.c
+++ b/drivers/net/ethernet/3com/3c589_cs.c
@@ -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 {
diff --git a/drivers/net/ethernet/8390/etherh.c b/drivers/net/ethernet/8390/etherh.c
index 05d39ecb97ffeeeb95c5fa84cbfc487ee6a61b73..e876fe52399a0a0b853724b60e3dd0416a3f5320 100644
--- a/drivers/net/ethernet/8390/etherh.c
+++ b/drivers/net/ethernet/8390/etherh.c
@@ -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:
diff --git a/drivers/net/ethernet/8390/pcnet_cs.c b/drivers/net/ethernet/8390/pcnet_cs.c
index 9bd5e991f1e52be30a5285196779d725ae9b481e..780fb4afb6af0d907d70dbfb01268a74c4fd9d7b 100644
--- a/drivers/net/ethernet/8390/pcnet_cs.c
+++ b/drivers/net/ethernet/8390/pcnet_cs.c
@@ -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);
     }
diff --git a/drivers/net/ethernet/amd/nmclan_cs.c b/drivers/net/ethernet/amd/nmclan_cs.c
index 0dd391c84c1387e35a6eab6952cb242fd5ee7c5a..37054a670407bb072ae9bfcb373df4b3ab078257 100644
--- a/drivers/net/ethernet/amd/nmclan_cs.c
+++ b/drivers/net/ethernet/amd/nmclan_cs.c
@@ -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;
diff --git a/drivers/net/ethernet/sis/sis900.c b/drivers/net/ethernet/sis/sis900.c
index cb7fec226cab63451f1c5e31287d67181f4f8a95..85b850372efee01c142f3f63bc2d495450512110 100644
--- a/drivers/net/ethernet/sis/sis900.c
+++ b/drivers/net/ethernet/sis/sis900.c
@@ -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
diff --git a/drivers/net/ethernet/smsc/smc91c92_cs.c b/drivers/net/ethernet/smsc/smc91c92_cs.c
index 29bb19f42de9f58661d5af3550ef78603fa373b0..86e3ec25df0799b243e87d0c30c9b6c06127aae2 100644
--- a/drivers/net/ethernet/smsc/smc91c92_cs.c
+++ b/drivers/net/ethernet/smsc/smc91c92_cs.c
@@ -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);
     }
diff --git a/drivers/net/ethernet/xircom/xirc2ps_cs.c b/drivers/net/ethernet/xircom/xirc2ps_cs.c
index e9bc38fd20257b96b9fb11a5cce19c5fd5d17aaf..a31d5d5e65936d88615bf28d5b5e3a650b8cc8a7 100644
--- a/drivers/net/ethernet/xircom/xirc2ps_cs.c
+++ b/drivers/net/ethernet/xircom/xirc2ps_cs.c
@@ -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 :-) */