caif: Fixes freeze on Link layer removal.

CAIF Socket layer - caif_socket.c: - Plug mem-leak at reconnect. - Always call disconnect to cleanup CAIF stack. - Disconnect will always report success. CAIF configuration layer - cfcnfg.c - Disconnect must dismantle the caif stack correctly - Protect against faulty removals (check on id zero) CAIF mux layer - cfmuxl.c - When inserting new service layer in the MUX remove any old entries with the same ID. - When removing CAIF Link layer, remove the associated service layers before notifying service layers. Signed-off-by: Sjur Brændeland <sjur.brandeland@stericsson.com> Signed-off-by: David S. Miller <davem@davemloft.net>

caif: Fixes freeze on Link layer removal.
CAIF Socket layer - caif_socket.c: - Plug mem-leak at reconnect. - Always call disconnect to cleanup CAIF stack. - Disconnect will always report success. CAIF configuration layer - cfcnfg.c - Disconnect must dismantle the caif stack correctly - Protect against faulty removals (check on id zero) CAIF mux layer - cfmuxl.c - When inserting new service layer in the MUX remove any old entries with the same ID. - When removing CAIF Link layer, remove the associated service layers before notifying service layers. Signed-off-by: Sjur Brændeland <sjur.brandeland@stericsson.com> Signed-off-by: David S. Miller <davem@davemloft.net>
sjur.brandeland@stericsson.com · David S. Miller
1 parent 0e5a117441
Showing 3 changed files with 62 additions and 44 deletions Side-by-side Diff
net/caif/caif_socket.c
net/caif/cfcnfg.c
net/caif/cfmuxl.c
@@ -19,7 +19,7 @@
 #include <linux/uaccess.h>
 #include <linux/debugfs.h>
 #include <linux/caif/caif_socket.h>
-#include <asm/atomic.h>
+#include <linux/atomic.h>
 #include <net/sock.h>
 #include <net/tcp_states.h>
 #include <net/caif/caif_layer.h>
@@ -816,6 +816,7 @@
 		if (sk->sk_shutdown & SHUTDOWN_MASK) {
 			/* Allow re-connect after SHUTDOWN_IND */
 			caif_disconnect_client(sock_net(sk), &cf_sk->layer);
+			caif_free_client(&cf_sk->layer);
 			break;
 		}
 		/* No reconnect on a seqpacket socket */
@@ -926,7 +927,6 @@
 {
 	struct sock *sk = sock->sk;
 	struct caifsock *cf_sk = container_of(sk, struct caifsock, sk);
-	int res = 0;
  
 	if (!sk)
 		return 0;
@@ -953,10 +953,7 @@
 	sk->sk_state = CAIF_DISCONNECTED;
 	sk->sk_shutdown = SHUTDOWN_MASK;
  
-	if (cf_sk->sk.sk_socket->state == SS_CONNECTED ||
-		cf_sk->sk.sk_socket->state == SS_CONNECTING)
-		res = caif_disconnect_client(sock_net(sk), &cf_sk->layer);
-
+	caif_disconnect_client(sock_net(sk), &cf_sk->layer);
 	cf_sk->sk.sk_socket->state = SS_DISCONNECTING;
 	wake_up_interruptible_poll(sk_sleep(sk), POLLERR|POLLHUP);
  
@@ -964,7 +961,7 @@
 	sk_stream_kill_queues(&cf_sk->sk);
 	release_sock(sk);
 	sock_put(sk);
-	return res;
+	return 0;
 }
  
 /* Copied from af_unix.c:unix_poll(), added CAIF tx_flow handling */
@@ -1120,7 +1117,7 @@
 	set_rx_flow_on(cf_sk);
  
 	/* Set default options on configuration */
-	cf_sk->sk.sk_priority= CAIF_PRIO_NORMAL;
+	cf_sk->sk.sk_priority = CAIF_PRIO_NORMAL;
 	cf_sk->conn_req.link_selector = CAIF_LINK_LOW_LATENCY;
 	cf_sk->conn_req.protocol = protocol;
 	/* Increase the number of sockets created. */
@@ -182,39 +182,26 @@
  
 int caif_disconnect_client(struct net *net, struct cflayer *adap_layer)
 {
-	u8 channel_id = 0;
-	int ret = 0;
-	struct cflayer *servl = NULL;
+	u8 channel_id;
 	struct cfcnfg *cfg = get_cfcnfg(net);
  
 	caif_assert(adap_layer != NULL);
-
+	cfctrl_cancel_req(cfg->ctrl, adap_layer);
 	channel_id = adap_layer->id;
-	if (adap_layer->dn == NULL || channel_id == 0) {
-		pr_err("adap_layer->dn == NULL or adap_layer->id is 0\n");
-		ret = -ENOTCONN;
-		goto end;
-	}
+	if (channel_id != 0) {
+		struct cflayer *servl;
+		servl = cfmuxl_remove_uplayer(cfg->mux, channel_id);
+		if (servl != NULL)
+			layer_set_up(servl, NULL);
+	} else
+		pr_debug("nothing to disconnect\n");
+	cfctrl_linkdown_req(cfg->ctrl, channel_id, adap_layer);
  
-	servl = cfmuxl_remove_uplayer(cfg->mux, channel_id);
-	if (servl == NULL) {
-		pr_err("PROTOCOL ERROR - "
-				"Error removing service_layer Channel_Id(%d)",
-				channel_id);
-		ret = -EINVAL;
-		goto end;
-	}
-
-	ret = cfctrl_linkdown_req(cfg->ctrl, channel_id, adap_layer);
-
-end:
-	cfctrl_cancel_req(cfg->ctrl, adap_layer);
-
 	/* Do RCU sync before initiating cleanup */
 	synchronize_rcu();
 	if (adap_layer->ctrlcmd != NULL)
 		adap_layer->ctrlcmd(adap_layer, CAIF_CTRLCMD_DEINIT_RSP, 0);
-	return ret;
+	return 0;
  
 }
 EXPORT_SYMBOL(caif_disconnect_client);
@@ -400,6 +387,14 @@
 	struct cfcnfg_phyinfo *phyinfo;
 	struct net_device *netdev;
  
+	if (channel_id == 0) {
+		pr_warn("received channel_id zero\n");
+		if (adapt_layer != NULL && adapt_layer->ctrlcmd != NULL)
+			adapt_layer->ctrlcmd(adapt_layer,
+						CAIF_CTRLCMD_INIT_FAIL_RSP, 0);
+		return;
+	}
+
 	rcu_read_lock();
  
 	if (adapt_layer == NULL) {
@@ -523,7 +518,6 @@
 	phyinfo->use_stx = stx;
 	phyinfo->use_fcs = fcs;
  
-	phy_layer->type = phy_type;
 	frml = cffrml_create(phyid, fcs);
  
 	if (!frml) {
@@ -62,16 +62,6 @@
 	return &this->layer;
 }
  
-int cfmuxl_set_uplayer(struct cflayer *layr, struct cflayer *up, u8 linkid)
-{
-	struct cfmuxl *muxl = container_obj(layr);
-
-	spin_lock_bh(&muxl->receive_lock);
-	list_add_rcu(&up->node, &muxl->srvl_list);
-	spin_unlock_bh(&muxl->receive_lock);
-	return 0;
-}
-
 int cfmuxl_set_dnlayer(struct cflayer *layr, struct cflayer *dn, u8 phyid)
 {
 	struct cfmuxl *muxl = (struct cfmuxl *) layr;
@@ -93,6 +83,24 @@
 	return NULL;
 }
  
+int cfmuxl_set_uplayer(struct cflayer *layr, struct cflayer *up, u8 linkid)
+{
+	struct cfmuxl *muxl = container_obj(layr);
+	struct cflayer *old;
+
+	spin_lock_bh(&muxl->receive_lock);
+
+	/* Two entries with same id is wrong, so remove old layer from mux */
+	old = get_from_id(&muxl->srvl_list, linkid);
+	if (old != NULL)
+		list_del_rcu(&old->node);
+
+	list_add_rcu(&up->node, &muxl->srvl_list);
+	spin_unlock_bh(&muxl->receive_lock);
+
+	return 0;
+}
+
 struct cflayer *cfmuxl_remove_dnlayer(struct cflayer *layr, u8 phyid)
 {
 	struct cfmuxl *muxl = container_obj(layr);
@@ -146,6 +154,11 @@
 	struct cfmuxl *muxl = container_obj(layr);
 	int idx = id % UP_CACHE_SIZE;
  
+	if (id == 0) {
+		pr_warn("Trying to remove control layer\n");
+		return NULL;
+	}
+
 	spin_lock_bh(&muxl->receive_lock);
 	up = get_from_id(&muxl->srvl_list, id);
 	if (up == NULL)
  
  
@@ -235,12 +248,26 @@
 {
 	struct cfmuxl *muxl = container_obj(layr);
 	struct cflayer *layer;
+	int idx;
  
 	rcu_read_lock();
 	list_for_each_entry_rcu(layer, &muxl->srvl_list, node) {
-		if (cfsrvl_phyid_match(layer, phyid) && layer->ctrlcmd)
+
+		if (cfsrvl_phyid_match(layer, phyid) && layer->ctrlcmd) {
+
+			if ((ctrl == _CAIF_CTRLCMD_PHYIF_FLOW_OFF_IND ||
+				ctrl == CAIF_CTRLCMD_REMOTE_SHUTDOWN_IND) &&
+					layer->id != 0) {
+
+				idx = layer->id % UP_CACHE_SIZE;
+				spin_lock_bh(&muxl->receive_lock);
+				rcu_assign_pointer(muxl->up_cache[idx], NULL);
+				list_del_rcu(&layer->node);
+				spin_unlock_bh(&muxl->receive_lock);
+			}
 			/* NOTE: ctrlcmd is not allowed to block */
 			layer->ctrlcmd(layer, ctrl, phyid);
+		}
 	}
 	rcu_read_unlock();
 }
...	...	@@ -19,7 +19,7 @@
19	19	#include <linux/uaccess.h>
20	20	#include <linux/debugfs.h>
21	21	#include <linux/caif/caif_socket.h>
22		-#include <asm/atomic.h>
	22	+#include <linux/atomic.h>
23	23	#include <net/sock.h>
24	24	#include <net/tcp_states.h>
25	25	#include <net/caif/caif_layer.h>
...	...	@@ -816,6 +816,7 @@
816	816	if (sk->sk_shutdown & SHUTDOWN_MASK) {
817	817	/* Allow re-connect after SHUTDOWN_IND */
818	818	caif_disconnect_client(sock_net(sk), &cf_sk->layer);
	819	+ caif_free_client(&cf_sk->layer);
819	820	break;
820	821	}
821	822	/* No reconnect on a seqpacket socket */
...	...	@@ -926,7 +927,6 @@
926	927	{
927	928	struct sock *sk = sock->sk;
928	929	struct caifsock *cf_sk = container_of(sk, struct caifsock, sk);
929		- int res = 0;
930	930
931	931	if (!sk)
932	932	return 0;
...	...	@@ -953,10 +953,7 @@
953	953	sk->sk_state = CAIF_DISCONNECTED;
954	954	sk->sk_shutdown = SHUTDOWN_MASK;
955	955
956		- if (cf_sk->sk.sk_socket->state == SS_CONNECTED \|\|
957		- cf_sk->sk.sk_socket->state == SS_CONNECTING)
958		- res = caif_disconnect_client(sock_net(sk), &cf_sk->layer);
959		-
	956	+ caif_disconnect_client(sock_net(sk), &cf_sk->layer);
960	957	cf_sk->sk.sk_socket->state = SS_DISCONNECTING;
961	958	wake_up_interruptible_poll(sk_sleep(sk), POLLERR\|POLLHUP);
962	959
...	...	@@ -964,7 +961,7 @@
964	961	sk_stream_kill_queues(&cf_sk->sk);
965	962	release_sock(sk);
966	963	sock_put(sk);
967		- return res;
	964	+ return 0;
968	965	}
969	966
970	967	/* Copied from af_unix.c:unix_poll(), added CAIF tx_flow handling */
...	...	@@ -1120,7 +1117,7 @@
1120	1117	set_rx_flow_on(cf_sk);
1121	1118
1122	1119	/* Set default options on configuration */
1123		- cf_sk->sk.sk_priority= CAIF_PRIO_NORMAL;
	1120	+ cf_sk->sk.sk_priority = CAIF_PRIO_NORMAL;
1124	1121	cf_sk->conn_req.link_selector = CAIF_LINK_LOW_LATENCY;
1125	1122	cf_sk->conn_req.protocol = protocol;
1126	1123	/* Increase the number of sockets created. */
...	...	@@ -182,39 +182,26 @@
182	182
183	183	int caif_disconnect_client(struct net net, struct cflayer adap_layer)
184	184	{
185		- u8 channel_id = 0;
186		- int ret = 0;
187		- struct cflayer *servl = NULL;
	185	+ u8 channel_id;
188	186	struct cfcnfg *cfg = get_cfcnfg(net);
189	187
190	188	caif_assert(adap_layer != NULL);
191		-
	189	+ cfctrl_cancel_req(cfg->ctrl, adap_layer);
192	190	channel_id = adap_layer->id;
193		- if (adap_layer->dn == NULL \|\| channel_id == 0) {
194		- pr_err("adap_layer->dn == NULL or adap_layer->id is 0\n");
195		- ret = -ENOTCONN;
196		- goto end;
197		- }
	191	+ if (channel_id != 0) {
	192	+ struct cflayer *servl;
	193	+ servl = cfmuxl_remove_uplayer(cfg->mux, channel_id);
	194	+ if (servl != NULL)
	195	+ layer_set_up(servl, NULL);
	196	+ } else
	197	+ pr_debug("nothing to disconnect\n");
	198	+ cfctrl_linkdown_req(cfg->ctrl, channel_id, adap_layer);
198	199
199		- servl = cfmuxl_remove_uplayer(cfg->mux, channel_id);
200		- if (servl == NULL) {
201		- pr_err("PROTOCOL ERROR - "
202		- "Error removing service_layer Channel_Id(%d)",
203		- channel_id);
204		- ret = -EINVAL;
205		- goto end;
206		- }
207		-
208		- ret = cfctrl_linkdown_req(cfg->ctrl, channel_id, adap_layer);
209		-
210		-end:
211		- cfctrl_cancel_req(cfg->ctrl, adap_layer);
212		-
213	200	/* Do RCU sync before initiating cleanup */
214	201	synchronize_rcu();
215	202	if (adap_layer->ctrlcmd != NULL)
216	203	adap_layer->ctrlcmd(adap_layer, CAIF_CTRLCMD_DEINIT_RSP, 0);
217		- return ret;
	204	+ return 0;
218	205
219	206	}
220	207	EXPORT_SYMBOL(caif_disconnect_client);
...	...	@@ -400,6 +387,14 @@
400	387	struct cfcnfg_phyinfo *phyinfo;
401	388	struct net_device *netdev;
402	389
	390	+ if (channel_id == 0) {
	391	+ pr_warn("received channel_id zero\n");
	392	+ if (adapt_layer != NULL && adapt_layer->ctrlcmd != NULL)
	393	+ adapt_layer->ctrlcmd(adapt_layer,
	394	+ CAIF_CTRLCMD_INIT_FAIL_RSP, 0);
	395	+ return;
	396	+ }
	397	+
403	398	rcu_read_lock();
404	399
405	400	if (adapt_layer == NULL) {
...	...	@@ -523,7 +518,6 @@
523	518	phyinfo->use_stx = stx;
524	519	phyinfo->use_fcs = fcs;
525	520
526		- phy_layer->type = phy_type;
527	521	frml = cffrml_create(phyid, fcs);
528	522
529	523	if (!frml) {
...	...	@@ -62,16 +62,6 @@
62	62	return &this->layer;
63	63	}
64	64
65		-int cfmuxl_set_uplayer(struct cflayer layr, struct cflayer up, u8 linkid)
66		-{
67		- struct cfmuxl *muxl = container_obj(layr);
68		-
69		- spin_lock_bh(&muxl->receive_lock);
70		- list_add_rcu(&up->node, &muxl->srvl_list);
71		- spin_unlock_bh(&muxl->receive_lock);
72		- return 0;
73		-}
74		-
75	65	int cfmuxl_set_dnlayer(struct cflayer layr, struct cflayer dn, u8 phyid)
76	66	{
77	67	struct cfmuxl muxl = (struct cfmuxl ) layr;
...	...	@@ -93,6 +83,24 @@
93	83	return NULL;
94	84	}
95	85
	86	+int cfmuxl_set_uplayer(struct cflayer layr, struct cflayer up, u8 linkid)
	87	+{
	88	+ struct cfmuxl *muxl = container_obj(layr);
	89	+ struct cflayer *old;
	90	+
	91	+ spin_lock_bh(&muxl->receive_lock);
	92	+
	93	+ /* Two entries with same id is wrong, so remove old layer from mux */
	94	+ old = get_from_id(&muxl->srvl_list, linkid);
	95	+ if (old != NULL)
	96	+ list_del_rcu(&old->node);
	97	+
	98	+ list_add_rcu(&up->node, &muxl->srvl_list);
	99	+ spin_unlock_bh(&muxl->receive_lock);
	100	+
	101	+ return 0;
	102	+}
	103	+
96	104	struct cflayer cfmuxl_remove_dnlayer(struct cflayer layr, u8 phyid)
97	105	{
98	106	struct cfmuxl *muxl = container_obj(layr);
...	...	@@ -146,6 +154,11 @@
146	154	struct cfmuxl *muxl = container_obj(layr);
147	155	int idx = id % UP_CACHE_SIZE;
148	156
	157	+ if (id == 0) {
	158	+ pr_warn("Trying to remove control layer\n");
	159	+ return NULL;
	160	+ }
	161	+
149	162	spin_lock_bh(&muxl->receive_lock);
150	163	up = get_from_id(&muxl->srvl_list, id);
151	164	if (up == NULL)
152	165
153	166
...	...	@@ -235,12 +248,26 @@
235	248	{
236	249	struct cfmuxl *muxl = container_obj(layr);
237	250	struct cflayer *layer;
	251	+ int idx;
238	252
239	253	rcu_read_lock();
240	254	list_for_each_entry_rcu(layer, &muxl->srvl_list, node) {
241		- if (cfsrvl_phyid_match(layer, phyid) && layer->ctrlcmd)
	255	+
	256	+ if (cfsrvl_phyid_match(layer, phyid) && layer->ctrlcmd) {
	257	+
	258	+ if ((ctrl == _CAIF_CTRLCMD_PHYIF_FLOW_OFF_IND \|\|
	259	+ ctrl == CAIF_CTRLCMD_REMOTE_SHUTDOWN_IND) &&
	260	+ layer->id != 0) {
	261	+
	262	+ idx = layer->id % UP_CACHE_SIZE;
	263	+ spin_lock_bh(&muxl->receive_lock);
	264	+ rcu_assign_pointer(muxl->up_cache[idx], NULL);
	265	+ list_del_rcu(&layer->node);
	266	+ spin_unlock_bh(&muxl->receive_lock);
	267	+ }
242	268	/* NOTE: ctrlcmd is not allowed to block */
243	269	layer->ctrlcmd(layer, ctrl, phyid);
	270	+ }
244	271	}
245	272	rcu_read_unlock();
246	273	}