[tipc-discussion] [PATCH net-next v3 03/12] tipc: make bearer variable in media structure synchronous

[Ying X. <yin...@wi...>]

When TIPC media is enabled by bearer, the "bearer" variable in the
media instance is first initialized with the bearer pointer under
tipc_net_lock protection; likewise, when media is disabled,
tipc_net_lock is also first held and its "bearer" variable is then
set to NULL. But, when the "bearer" variable is accessed in
recv_msg() and recv_notification(), tipc_net_lock is not taken. As
the context under which recv_msg() and recv_notification() run is
different with media enable/disable functions, a stale "bearer"
might be got by recv_msg() and recv_notification() occasionally.
Therefore, before the bearer variable is read, tipc_net_lock should
be held to get rid of the synchronous issue.

Signed-off-by: Ying Xue <yin...@wi...>
---
 net/tipc/bearer.c    |    2 --
 net/tipc/eth_media.c |   13 ++++++++++++-
 net/tipc/ib_media.c  |   13 ++++++++++++-
 net/tipc/link.c      |    2 --
 4 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/net/tipc/bearer.c b/net/tipc/bearer.c
index 35e65b5..9f03a93 100644
--- a/net/tipc/bearer.c
+++ b/net/tipc/bearer.c
@@ -421,7 +421,6 @@ int tipc_block_bearer(struct tipc_bearer *b_ptr)
 	struct tipc_link *l_ptr;
 	struct tipc_link *temp_l_ptr;
 
-	read_lock_bh(&tipc_net_lock);
 	pr_info("Blocking bearer <%s>\n", b_ptr->name);
 	atomic_add_unless(&b_ptr->blocked, 1, 1);
 	spin_lock_bh(&b_ptr->lock);
@@ -433,7 +432,6 @@ int tipc_block_bearer(struct tipc_bearer *b_ptr)
 		spin_unlock_bh(&n_ptr->lock);
 	}
 	spin_unlock_bh(&b_ptr->lock);
-	read_unlock_bh(&tipc_net_lock);
 	return 0;
 }
 
diff --git a/net/tipc/eth_media.c b/net/tipc/eth_media.c
index 5ba58b0..f7ddbd7 100644
--- a/net/tipc/eth_media.c
+++ b/net/tipc/eth_media.c
@@ -36,6 +36,7 @@
 
 #include "core.h"
 #include "bearer.h"
+#include "net.h"
 
 #define MAX_ETH_MEDIA		MAX_BEARERS
 
@@ -135,13 +136,16 @@ static int recv_msg(struct sk_buff *buf, struct net_device *dev,
 		return NET_RX_DROP;
 	}
 
+	read_lock_bh(&tipc_net_lock);
 	if (likely(eb_ptr->bearer)) {
 		if (likely(buf->pkt_type <= PACKET_BROADCAST)) {
 			buf->next = NULL;
 			tipc_recv_msg(buf, eb_ptr->bearer);
+			read_unlock_bh(&tipc_net_lock);
 			return NET_RX_SUCCESS;
 		}
 	}
+	read_unlock_bh(&tipc_net_lock);
 	kfree_skb(buf);
 	return NET_RX_DROP;
 }
@@ -254,8 +258,12 @@ static int recv_notification(struct notifier_block *nb, unsigned long evt,
 		if (++eb_ptr == stop)
 			return NOTIFY_DONE;	/* couldn't find device */
 	}
-	if (!eb_ptr->bearer)
+
+	read_lock_bh(&tipc_net_lock);
+	if (!eb_ptr->bearer) {
+		read_unlock_bh(&tipc_net_lock);
 		return NOTIFY_DONE;		/* bearer had been disabled */
+	}
 
 	eb_ptr->bearer->mtu = dev->mtu;
 
@@ -279,9 +287,12 @@ static int recv_notification(struct notifier_block *nb, unsigned long evt,
 		break;
 	case NETDEV_UNREGISTER:
 	case NETDEV_CHANGENAME:
+		read_unlock_bh(&tipc_net_lock);
 		tipc_disable_bearer(eb_ptr->bearer->name);
+		read_lock_bh(&tipc_net_lock);
 		break;
 	}
+	read_unlock_bh(&tipc_net_lock);
 	return NOTIFY_OK;
 }
 
diff --git a/net/tipc/ib_media.c b/net/tipc/ib_media.c
index 34b9710..1d1e2f5 100644
--- a/net/tipc/ib_media.c
+++ b/net/tipc/ib_media.c
@@ -41,6 +41,7 @@
 #include <linux/if_infiniband.h>
 #include "core.h"
 #include "bearer.h"
+#include "net.h"
 
 #define MAX_IB_MEDIA		MAX_BEARERS
 
@@ -128,13 +129,16 @@ static int recv_msg(struct sk_buff *buf, struct net_device *dev,
 		return NET_RX_DROP;
 	}
 
+	read_lock_bh(&tipc_net_lock);
 	if (likely(ib_ptr->bearer)) {
 		if (likely(buf->pkt_type <= PACKET_BROADCAST)) {
 			buf->next = NULL;
 			tipc_recv_msg(buf, ib_ptr->bearer);
+			read_unlock_bh(&tipc_net_lock);
 			return NET_RX_SUCCESS;
 		}
 	}
+	read_unlock_bh(&tipc_net_lock);
 	kfree_skb(buf);
 	return NET_RX_DROP;
 }
@@ -247,8 +251,12 @@ static int recv_notification(struct notifier_block *nb, unsigned long evt,
 		if (++ib_ptr == stop)
 			return NOTIFY_DONE;	/* couldn't find device */
 	}
-	if (!ib_ptr->bearer)
+
+	read_lock_bh(&tipc_net_lock);
+	if (!ib_ptr->bearer) {
+		read_unlock_bh(&tipc_net_lock);
 		return NOTIFY_DONE;		/* bearer had been disabled */
+	}
 
 	ib_ptr->bearer->mtu = dev->mtu;
 
@@ -272,9 +280,12 @@ static int recv_notification(struct notifier_block *nb, unsigned long evt,
 		break;
 	case NETDEV_UNREGISTER:
 	case NETDEV_CHANGENAME:
+		read_unlock_bh(&tipc_net_lock);
 		tipc_disable_bearer(ib_ptr->bearer->name);
+		read_lock_bh(&tipc_net_lock);
 		break;
 	}
+	read_unlock_bh(&tipc_net_lock);
 	return NOTIFY_OK;
 }
 
diff --git a/net/tipc/link.c b/net/tipc/link.c
index 8c0e9a3..1069874 100644
--- a/net/tipc/link.c
+++ b/net/tipc/link.c
@@ -1491,7 +1491,6 @@ static int link_recv_buf_validate(struct sk_buff *buf)
  */
 void tipc_recv_msg(struct sk_buff *head, struct tipc_bearer *b_ptr)
 {
-	read_lock_bh(&tipc_net_lock);
 	while (head) {
 		struct tipc_node *n_ptr;
 		struct tipc_link *l_ptr;
@@ -1687,7 +1686,6 @@ deliver:
 cont:
 		kfree_skb(buf);
 	}
-	read_unlock_bh(&tipc_net_lock);
 }
 
 /**
-- 
1.7.9.5