netdev - [PATCH v2 2/2] tipc: pskb_copy() buffers when sending on more than one bearer

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <1367244820-29881-2-git-send-email-gerlando.falauto@keymile.com>
Date:	Mon, 29 Apr 2013 16:13:40 +0200
From:	Gerlando Falauto <gerlando.falauto@...mile.com>
To:	netdev@...r.kernel.org
Cc:	jon.maloy@...csson.com, erik.hugne@...csson.com,
	ying.xue@...driver.com, holger.brunck@...mile.com,
	Gerlando Falauto <gerlando.falauto@...mile.com>
Subject: [PATCH v2 2/2] tipc: pskb_copy() buffers when sending on more than one bearer

When sending packets, TIPC bearers use skb_clone() before writing their
hardware header. This will however NOT copy the data buffer.
So when the same packet is sent over multiple bearers (to reach multiple
nodes), the same socket buffer data will be treated by multiple
tipc_media drivers which will write their own hardware header through
dev_hard_header().
Most of the time this is not a problem, because by the time the
packet is processed by the second media, it has already been sent over
the first one. However, when the first transmission is delayed (e.g.
because of insufficient bandwidth or through a shaper), the next bearer
will overwrite the hardware header, resulting in the packet being sent:
a) with the wrong source address, when bearers of the same type,
e.g. ethernet, are involved
b) with a completely corrupt header, or even dropped, when bearers of
different types are involved.

So when the same socket buffer is to be sent multiple times, send a
pskb_copy() instead (from the second instance on), and release it
afterwards (the bearer will skb_clone() it anyway).

Signed-off-by: Gerlando Falauto <gerlando.falauto@...mile.com>
---
 net/tipc/bcast.c |   13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/net/tipc/bcast.c b/net/tipc/bcast.c
index be1f945..7f57f76 100644
--- a/net/tipc/bcast.c
+++ b/net/tipc/bcast.c
@@ -611,6 +611,7 @@ static int tipc_bcbearer_send(struct sk_buff *buf,
 		struct tipc_bearer *p = bcbearer->bpairs[bp_index].primary;
 		struct tipc_bearer *b = p;
 		struct tipc_bearer *s = bcbearer->bpairs[bp_index].secondary;
+		struct sk_buff *tbuf;

 		if (!p)
 			break;	/* No more bearers to try */
@@ -626,7 +627,17 @@ static int tipc_bcbearer_send(struct sk_buff *buf,
 		if (bcbearer->remains_new.count == bcbearer->remains.count)
 			continue;  /* Nothing added by bearer pair */

-		tipc_bearer_send(b, buf, &s->media->bcast_addr);
+		if (bp_index == 0) {
+			/* Use original buffer for first bearer */
+			tipc_bearer_send(b, buf, &b->media->bcast_addr);
+		} else {
+			/* Avoid concurrent buffer access */
+			tbuf = pskb_copy(buf, GFP_ATOMIC);
+			if (!tbuf)
+				break;
+			tipc_bearer_send(b, tbuf, &b->media->bcast_addr);
+			kfree_skb(tbuf); /* Bearer keeps a clone */
+		}

 		/* Swap bearers for next packet */
 		if (s) {
-- 
1.7.10.1

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html