tipc: clean up connection protocol reception function

We simplify the code for receiving connection probes, leveraging the recently introduced tipc_msg_reverse() function. We also stick to the principle of sending a possible response message directly from the calling (tipc_sk_rcv or backlog_rcv) functions, hence making the call chain shallower and easier to follow. We make one small protocol change here, allowed according to the spec. If a protocol message arrives from a remote socket that is not the one we are connected to, we are currently generating a connection abort message and send it to the source. This behavior is unnecessary, and might even be a security risk, so instead we now choose to only ignore the message. The consequnce for the sender is that he will need longer time to discover his mistake (until the next timeout), but this is an extreme corner case, and may happen anyway under other circumstances, so we deem this change acceptable. Signed-off-by: Jon Maloy <jon.maloy@ericsson.com> Reviewed-by: Erik Hugne <erik.hugne@ericsson.com> Reviewed-by: Ying Xue <ying.xue@windriver.com> Signed-off-by: David S. Miller <davem@davemloft.net>

tipc: clean up connection protocol reception function
We simplify the code for receiving connection probes, leveraging the recently introduced tipc_msg_reverse() function. We also stick to the principle of sending a possible response message directly from the calling (tipc_sk_rcv or backlog_rcv) functions, hence making the call chain shallower and easier to follow. We make one small protocol change here, allowed according to the spec. If a protocol message arrives from a remote socket that is not the one we are connected to, we are currently generating a connection abort message and send it to the source. This behavior is unnecessary, and might even be a security risk, so instead we now choose to only ignore the message. The consequnce for the sender is that he will need longer time to discover his mistake (until the next timeout), but this is an extreme corner case, and may happen anyway under other circumstances, so we deem this change acceptable. Signed-off-by: Jon Maloy <jon.maloy@ericsson.com> Reviewed-by: Erik Hugne <erik.hugne@ericsson.com> Reviewed-by: Ying Xue <ying.xue@windriver.com> Signed-off-by: David S. Miller <davem@davemloft.net>
Jon Paul Maloy · David S. Miller
1 parent ec8a2e5621
Showing 5 changed files with 68 additions and 63 deletions Side-by-side Diff
net/tipc/msg.c
net/tipc/port.c
net/tipc/port.h
net/tipc/socket.c
net/tipc/socket.h
@@ -348,13 +348,17 @@
 	struct tipc_msg ohdr;
 	uint rdsz = min_t(uint, msg_data_sz(msg), MAX_FORWARD_SIZE);
  
-	if (skb_linearize(buf) || !msg_isdata(msg))
+	if (skb_linearize(buf))
 		goto exit;
-	if (msg_dest_droppable(msg) || msg_errcode(msg))
+	if (msg_dest_droppable(msg))
 		goto exit;
+	if (msg_errcode(msg))
+		goto exit;
  
 	memcpy(&ohdr, msg, msg_hdr_sz(msg));
-	msg_set_importance(msg, min_t(uint, ++imp, TIPC_CRITICAL_IMPORTANCE));
+	imp = min_t(uint, imp + 1, TIPC_CRITICAL_IMPORTANCE);
+	if (msg_isdata(msg))
+		msg_set_importance(msg, imp);
 	msg_set_errcode(msg, err);
 	msg_set_origport(msg, msg_destport(&ohdr));
 	msg_set_destport(msg, msg_origport(&ohdr));
@@ -42,8 +42,6 @@
  
 /* Connection management: */
 #define PROBING_INTERVAL 3600000	/* [ms] => 1 h */
-#define CONFIRMED 0
-#define PROBING 1
  
 #define MAX_REJECT_SIZE 1024
  
  
@@ -299,11 +297,11 @@
 	}
  
 	/* Last probe answered ? */
-	if (p_ptr->probing_state == PROBING) {
+	if (p_ptr->probing_state == TIPC_CONN_PROBING) {
 		buf = port_build_self_abort_msg(p_ptr, TIPC_ERR_NO_PORT);
 	} else {
 		buf = port_build_proto_msg(p_ptr, CONN_PROBE, 0);
-		p_ptr->probing_state = PROBING;
+		p_ptr->probing_state = TIPC_CONN_PROBING;
 		k_start_timer(&p_ptr->timer, p_ptr->probing_interval);
 	}
 	tipc_port_unlock(p_ptr);
@@ -365,51 +363,6 @@
 	return buf;
 }
  
-void tipc_port_proto_rcv(struct tipc_port *p_ptr, struct sk_buff *buf)
-{
-	struct tipc_msg *msg = buf_msg(buf);
-	struct sk_buff *r_buf = NULL;
-	u32 destport = msg_destport(msg);
-	int wakeable;
-
-	/* Validate connection */
-	if (!p_ptr || !p_ptr->connected || !tipc_port_peer_msg(p_ptr, msg)) {
-		r_buf = tipc_buf_acquire(BASIC_H_SIZE);
-		if (r_buf) {
-			msg = buf_msg(r_buf);
-			tipc_msg_init(msg, TIPC_HIGH_IMPORTANCE, TIPC_CONN_MSG,
-				      BASIC_H_SIZE, msg_orignode(msg));
-			msg_set_errcode(msg, TIPC_ERR_NO_PORT);
-			msg_set_origport(msg, destport);
-			msg_set_destport(msg, msg_origport(msg));
-		}
-		goto exit;
-	}
-
-	/* Process protocol message sent by peer */
-	switch (msg_type(msg)) {
-	case CONN_ACK:
-		wakeable = tipc_port_congested(p_ptr) && p_ptr->congested;
-		p_ptr->acked += msg_msgcnt(msg);
-		if (!tipc_port_congested(p_ptr)) {
-			p_ptr->congested = 0;
-			if (wakeable)
-				tipc_port_wakeup(p_ptr);
-		}
-		break;
-	case CONN_PROBE:
-		r_buf = port_build_proto_msg(p_ptr, CONN_PROBE_REPLY, 0);
-		break;
-	default:
-		/* CONN_PROBE_REPLY or unrecognized - no action required */
-		break;
-	}
-	p_ptr->probing_state = CONFIRMED;
-exit:
-	tipc_link_xmit2(r_buf, msg_destnode(msg), msg_link_selector(msg));
-	kfree_skb(buf);
-}
-
 static int port_print(struct tipc_port *p_ptr, char *buf, int len, int full_id)
 {
 	struct publication *publ;
@@ -613,7 +566,7 @@
 	msg_set_hdr_sz(msg, SHORT_H_SIZE);
  
 	p_ptr->probing_interval = PROBING_INTERVAL;
-	p_ptr->probing_state = CONFIRMED;
+	p_ptr->probing_state = TIPC_CONN_OK;
 	p_ptr->connected = 1;
 	k_start_timer(&p_ptr->timer, p_ptr->probing_interval);
  
@@ -140,7 +140,6 @@
 			 unsigned int len);
  
 struct sk_buff *tipc_port_get_ports(void);
-void tipc_port_proto_rcv(struct tipc_port *port, struct sk_buff *buf);
 void tipc_port_mcast_rcv(struct sk_buff *buf, struct tipc_port_list *dp);
 void tipc_port_reinit(void);
  
@@ -46,6 +46,7 @@
 #define SS_READY	-2	/* socket is connectionless */
  
 #define CONN_TIMEOUT_DEFAULT	8000	/* default connect timeout = 8s */
+#define TIPC_FWD_MSG	        1
  
 static int tipc_backlog_rcv(struct sock *sk, struct sk_buff *skb);
 static void tipc_data_ready(struct sock *sk);
@@ -534,6 +535,46 @@
 }
  
 /**
+ * tipc_sk_proto_rcv - receive a connection mng protocol message
+ * @tsk: receiving socket
+ * @dnode: node to send response message to, if any
+ * @buf: buffer containing protocol message
+ * Returns 0 (TIPC_OK) if message was consumed, 1 (TIPC_FWD_MSG) if
+ * (CONN_PROBE_REPLY) message should be forwarded.
+ */
+int tipc_sk_proto_rcv(struct tipc_sock *tsk, u32 *dnode, struct sk_buff *buf)
+{
+	struct tipc_msg *msg = buf_msg(buf);
+	struct tipc_port *port = &tsk->port;
+	int wakeable;
+
+	/* Ignore if connection cannot be validated: */
+	if (!port->connected || !tipc_port_peer_msg(port, msg))
+		goto exit;
+
+	port->probing_state = TIPC_CONN_OK;
+
+	if (msg_type(msg) == CONN_ACK) {
+		wakeable = tipc_port_congested(port) && port->congested;
+		port->acked += msg_msgcnt(msg);
+		if (!tipc_port_congested(port)) {
+			port->congested = 0;
+			if (wakeable)
+				tipc_port_wakeup(port);
+		}
+	} else if (msg_type(msg) == CONN_PROBE) {
+		if (!tipc_msg_reverse(buf, dnode, TIPC_OK))
+			return TIPC_OK;
+		msg_set_type(msg, CONN_PROBE_REPLY);
+		return TIPC_FWD_MSG;
+	}
+	/* Do nothing if msg_type() == CONN_PROBE_REPLY */
+exit:
+	kfree_skb(buf);
+	return TIPC_OK;
+}
+
+/**
  * dest_name_check - verify user is permitted to send to specified port name
  * @dest: destination address
  * @m: descriptor for message to be sent
@@ -1406,7 +1447,7 @@
  * Called with socket lock already taken; port lock may also be taken.
  *
  * Returns 0 (TIPC_OK) if message was consumed, -TIPC error code if message
- * to be rejected.
+ * to be rejected, 1 (TIPC_FWD_MSG) if (CONN_MANAGER) message to be forwarded
  */
 static int filter_rcv(struct sock *sk, struct sk_buff *buf)
 {
  
@@ -1414,12 +1455,11 @@
 	struct tipc_sock *tsk = tipc_sk(sk);
 	struct tipc_msg *msg = buf_msg(buf);
 	unsigned int limit = rcvbuf_limit(sk, buf);
+	u32 onode;
 	int rc = TIPC_OK;
  
-	if (unlikely(msg_user(msg) == CONN_MANAGER)) {
-		tipc_port_proto_rcv(&tsk->port, buf);
-		return TIPC_OK;
-	}
+	if (unlikely(msg_user(msg) == CONN_MANAGER))
+		return tipc_sk_proto_rcv(tsk, &onode, buf);
  
 	/* Reject message if it is wrong sort of message for socket */
 	if (msg_type(msg) > TIPC_DIRECT_MSG)
@@ -1465,10 +1505,16 @@
  
 	rc = filter_rcv(sk, buf);
  
-	if (unlikely(rc && tipc_msg_reverse(buf, &onode, -rc)))
-		tipc_link_xmit2(buf, onode, 0);
-	else if (atomic_read(&tsk->dupl_rcvcnt) < TIPC_CONN_OVERLOAD_LIMIT)
-		atomic_add(truesize, &tsk->dupl_rcvcnt);
+	if (likely(!rc)) {
+		if (atomic_read(&tsk->dupl_rcvcnt) < TIPC_CONN_OVERLOAD_LIMIT)
+			atomic_add(truesize, &tsk->dupl_rcvcnt);
+		return 0;
+	}
+
+	if ((rc < 0) && !tipc_msg_reverse(buf, &onode, -rc))
+		return 0;
+
+	tipc_link_xmit2(buf, onode, 0);
  
 	return 0;
 }
@@ -38,6 +38,9 @@
 #include "port.h"
 #include <net/sock.h>
  
+#define TIPC_CONN_OK      0
+#define TIPC_CONN_PROBING 1
+
 /**
  * struct tipc_sock - TIPC socket structure
  * @sk: socket - interacts with 'port' and with user via the socket API
...	...	@@ -348,13 +348,17 @@
348	348	struct tipc_msg ohdr;
349	349	uint rdsz = min_t(uint, msg_data_sz(msg), MAX_FORWARD_SIZE);
350	350
351		- if (skb_linearize(buf) \|\| !msg_isdata(msg))
	351	+ if (skb_linearize(buf))
352	352	goto exit;
353		- if (msg_dest_droppable(msg) \|\| msg_errcode(msg))
	353	+ if (msg_dest_droppable(msg))
354	354	goto exit;
	355	+ if (msg_errcode(msg))
	356	+ goto exit;
355	357
356	358	memcpy(&ohdr, msg, msg_hdr_sz(msg));
357		- msg_set_importance(msg, min_t(uint, ++imp, TIPC_CRITICAL_IMPORTANCE));
	359	+ imp = min_t(uint, imp + 1, TIPC_CRITICAL_IMPORTANCE);
	360	+ if (msg_isdata(msg))
	361	+ msg_set_importance(msg, imp);
358	362	msg_set_errcode(msg, err);
359	363	msg_set_origport(msg, msg_destport(&ohdr));
360	364	msg_set_destport(msg, msg_origport(&ohdr));
...	...	@@ -42,8 +42,6 @@
42	42
43	43	/* Connection management: */
44	44	#define PROBING_INTERVAL 3600000 /* [ms] => 1 h */
45		-#define CONFIRMED 0
46		-#define PROBING 1
47	45
48	46	#define MAX_REJECT_SIZE 1024
49	47
50	48
...	...	@@ -299,11 +297,11 @@
299	297	}
300	298
301	299	/* Last probe answered ? */
302		- if (p_ptr->probing_state == PROBING) {
	300	+ if (p_ptr->probing_state == TIPC_CONN_PROBING) {
303	301	buf = port_build_self_abort_msg(p_ptr, TIPC_ERR_NO_PORT);
304	302	} else {
305	303	buf = port_build_proto_msg(p_ptr, CONN_PROBE, 0);
306		- p_ptr->probing_state = PROBING;
	304	+ p_ptr->probing_state = TIPC_CONN_PROBING;
307	305	k_start_timer(&p_ptr->timer, p_ptr->probing_interval);
308	306	}
309	307	tipc_port_unlock(p_ptr);
...	...	@@ -365,51 +363,6 @@
365	363	return buf;
366	364	}
367	365
368		-void tipc_port_proto_rcv(struct tipc_port p_ptr, struct sk_buff buf)
369		-{
370		- struct tipc_msg *msg = buf_msg(buf);
371		- struct sk_buff *r_buf = NULL;
372		- u32 destport = msg_destport(msg);
373		- int wakeable;
374		-
375		- /* Validate connection */
376		- if (!p_ptr \|\| !p_ptr->connected \|\| !tipc_port_peer_msg(p_ptr, msg)) {
377		- r_buf = tipc_buf_acquire(BASIC_H_SIZE);
378		- if (r_buf) {
379		- msg = buf_msg(r_buf);
380		- tipc_msg_init(msg, TIPC_HIGH_IMPORTANCE, TIPC_CONN_MSG,
381		- BASIC_H_SIZE, msg_orignode(msg));
382		- msg_set_errcode(msg, TIPC_ERR_NO_PORT);
383		- msg_set_origport(msg, destport);
384		- msg_set_destport(msg, msg_origport(msg));
385		- }
386		- goto exit;
387		- }
388		-
389		- /* Process protocol message sent by peer */
390		- switch (msg_type(msg)) {
391		- case CONN_ACK:
392		- wakeable = tipc_port_congested(p_ptr) && p_ptr->congested;
393		- p_ptr->acked += msg_msgcnt(msg);
394		- if (!tipc_port_congested(p_ptr)) {
395		- p_ptr->congested = 0;
396		- if (wakeable)
397		- tipc_port_wakeup(p_ptr);
398		- }
399		- break;
400		- case CONN_PROBE:
401		- r_buf = port_build_proto_msg(p_ptr, CONN_PROBE_REPLY, 0);
402		- break;
403		- default:
404		- /* CONN_PROBE_REPLY or unrecognized - no action required */
405		- break;
406		- }
407		- p_ptr->probing_state = CONFIRMED;
408		-exit:
409		- tipc_link_xmit2(r_buf, msg_destnode(msg), msg_link_selector(msg));
410		- kfree_skb(buf);
411		-}
412		-
413	366	static int port_print(struct tipc_port p_ptr, char buf, int len, int full_id)
414	367	{
415	368	struct publication *publ;
...	...	@@ -613,7 +566,7 @@
613	566	msg_set_hdr_sz(msg, SHORT_H_SIZE);
614	567
615	568	p_ptr->probing_interval = PROBING_INTERVAL;
616		- p_ptr->probing_state = CONFIRMED;
	569	+ p_ptr->probing_state = TIPC_CONN_OK;
617	570	p_ptr->connected = 1;
618	571	k_start_timer(&p_ptr->timer, p_ptr->probing_interval);
619	572
...	...	@@ -140,7 +140,6 @@
140	140	unsigned int len);
141	141
142	142	struct sk_buff *tipc_port_get_ports(void);
143		-void tipc_port_proto_rcv(struct tipc_port port, struct sk_buff buf);
144	143	void tipc_port_mcast_rcv(struct sk_buff buf, struct tipc_port_list dp);
145	144	void tipc_port_reinit(void);
146	145
...	...	@@ -46,6 +46,7 @@
46	46	#define SS_READY -2 /* socket is connectionless */
47	47
48	48	#define CONN_TIMEOUT_DEFAULT 8000 /* default connect timeout = 8s */
	49	+#define TIPC_FWD_MSG 1
49	50
50	51	static int tipc_backlog_rcv(struct sock sk, struct sk_buff skb);
51	52	static void tipc_data_ready(struct sock *sk);
...	...	@@ -534,6 +535,46 @@
534	535	}
535	536
536	537	/**
	538	+ * tipc_sk_proto_rcv - receive a connection mng protocol message
	539	+ * @tsk: receiving socket
	540	+ * @dnode: node to send response message to, if any
	541	+ * @buf: buffer containing protocol message
	542	+ * Returns 0 (TIPC_OK) if message was consumed, 1 (TIPC_FWD_MSG) if
	543	+ * (CONN_PROBE_REPLY) message should be forwarded.
	544	+ */
	545	+int tipc_sk_proto_rcv(struct tipc_sock tsk, u32 dnode, struct sk_buff *buf)
	546	+{
	547	+ struct tipc_msg *msg = buf_msg(buf);
	548	+ struct tipc_port *port = &tsk->port;
	549	+ int wakeable;
	550	+
	551	+ /* Ignore if connection cannot be validated: */
	552	+ if (!port->connected \|\| !tipc_port_peer_msg(port, msg))
	553	+ goto exit;
	554	+
	555	+ port->probing_state = TIPC_CONN_OK;
	556	+
	557	+ if (msg_type(msg) == CONN_ACK) {
	558	+ wakeable = tipc_port_congested(port) && port->congested;
	559	+ port->acked += msg_msgcnt(msg);
	560	+ if (!tipc_port_congested(port)) {
	561	+ port->congested = 0;
	562	+ if (wakeable)
	563	+ tipc_port_wakeup(port);
	564	+ }
	565	+ } else if (msg_type(msg) == CONN_PROBE) {
	566	+ if (!tipc_msg_reverse(buf, dnode, TIPC_OK))
	567	+ return TIPC_OK;
	568	+ msg_set_type(msg, CONN_PROBE_REPLY);
	569	+ return TIPC_FWD_MSG;
	570	+ }
	571	+ /* Do nothing if msg_type() == CONN_PROBE_REPLY */
	572	+exit:
	573	+ kfree_skb(buf);
	574	+ return TIPC_OK;
	575	+}
	576	+
	577	+/**
537	578	* dest_name_check - verify user is permitted to send to specified port name
538	579	* @dest: destination address
539	580	* @m: descriptor for message to be sent
...	...	@@ -1406,7 +1447,7 @@
1406	1447	* Called with socket lock already taken; port lock may also be taken.
1407	1448	*
1408	1449	* Returns 0 (TIPC_OK) if message was consumed, -TIPC error code if message
1409		- * to be rejected.
	1450	+ * to be rejected, 1 (TIPC_FWD_MSG) if (CONN_MANAGER) message to be forwarded
1410	1451	*/
1411	1452	static int filter_rcv(struct sock sk, struct sk_buff buf)
1412	1453	{
1413	1454
...	...	@@ -1414,12 +1455,11 @@
1414	1455	struct tipc_sock *tsk = tipc_sk(sk);
1415	1456	struct tipc_msg *msg = buf_msg(buf);
1416	1457	unsigned int limit = rcvbuf_limit(sk, buf);
	1458	+ u32 onode;
1417	1459	int rc = TIPC_OK;
1418	1460
1419		- if (unlikely(msg_user(msg) == CONN_MANAGER)) {
1420		- tipc_port_proto_rcv(&tsk->port, buf);
1421		- return TIPC_OK;
1422		- }
	1461	+ if (unlikely(msg_user(msg) == CONN_MANAGER))
	1462	+ return tipc_sk_proto_rcv(tsk, &onode, buf);
1423	1463
1424	1464	/* Reject message if it is wrong sort of message for socket */
1425	1465	if (msg_type(msg) > TIPC_DIRECT_MSG)
...	...	@@ -1465,10 +1505,16 @@
1465	1505
1466	1506	rc = filter_rcv(sk, buf);
1467	1507
1468		- if (unlikely(rc && tipc_msg_reverse(buf, &onode, -rc)))
1469		- tipc_link_xmit2(buf, onode, 0);
1470		- else if (atomic_read(&tsk->dupl_rcvcnt) < TIPC_CONN_OVERLOAD_LIMIT)
1471		- atomic_add(truesize, &tsk->dupl_rcvcnt);
	1508	+ if (likely(!rc)) {
	1509	+ if (atomic_read(&tsk->dupl_rcvcnt) < TIPC_CONN_OVERLOAD_LIMIT)
	1510	+ atomic_add(truesize, &tsk->dupl_rcvcnt);
	1511	+ return 0;
	1512	+ }
	1513	+
	1514	+ if ((rc < 0) && !tipc_msg_reverse(buf, &onode, -rc))
	1515	+ return 0;
	1516	+
	1517	+ tipc_link_xmit2(buf, onode, 0);
1472	1518
1473	1519	return 0;
1474	1520	}
...	...	@@ -38,6 +38,9 @@
38	38	#include "port.h"
39	39	#include <net/sock.h>
40	40
	41	+#define TIPC_CONN_OK 0
	42	+#define TIPC_CONN_PROBING 1
	43	+
41	44	/**
42	45	* struct tipc_sock - TIPC socket structure
43	46	* @sk: socket - interacts with 'port' and with user via the socket API