[v3,4/6] batman-adv: drop unicast packets from other backbone gw

Message ID 1462525107-19750-5-git-send-email-apape@phoenixcontact.com (mailing list archive)
State Superseded, archived
Delegated to: Marek Lindner
Headers

Commit Message

Andreas Pape May 6, 2016, 8:58 a.m. UTC
  Additional dropping of unicast packets received from another backbone gw of
the same backbone network before being forwarded to the same backbone again
is necessary. It was observed in a test setup that in rare cases these
frames lead to looping unicast traffic backbone->mesh->backbone.

Acked-by: Simon Wunderlich <sw@simonwunderlich.de>
Signed-off-by: Andreas Pape <apape@phoenixcontact.com>
---
 net/batman-adv/routing.c |   16 ++++++++++++++++
 1 files changed, 16 insertions(+), 0 deletions(-)

--
1.7.0.4



..................................................................
PHOENIX CONTACT ELECTRONICS GmbH

Sitz der Gesellschaft / registered office of the company: 31812 Bad Pyrmont
USt-Id-Nr.: DE811742156
Amtsgericht Hannover HRB 100528 / district court Hannover HRB 100528
Geschäftsführer / Executive Board: Roland Bent, Dr. Martin Heubeck
  

Comments

Sven Eckelmann May 6, 2016, 10:43 a.m. UTC | #1
On Friday 06 May 2016 10:58:25 Andreas Pape wrote:
[...]
> +		/* If this is a unicast packet from another backgone gw,
> +		 * drop it.
> +		 */
> +		orig_addr = ethhdr->h_source;
> +		orig_node = batadv_orig_hash_find(bat_priv, orig_addr);

I think we already told this before: This is overwriting the previous
orig_node without freeing the reference. It most likely has also other weird
consequences. Here is the relevant code to better understand what it
overwrites:

		if (is4addr) {
			subtype = unicast_4addr_packet->subtype;
			batadv_dat_inc_counter(bat_priv, subtype);

			/* Only payload data should be considered for speedy
			 * join. For example, DAT also uses unicast 4addr
			 * types, but those packets should not be considered
			 * for speedy join, since the clients do not actually
			 * reside at the sending originator.
			 */
			if (subtype == BATADV_P_DATA) {
				orig_addr = unicast_4addr_packet->src;
				orig_node = batadv_orig_hash_find(bat_priv,
								  orig_addr);
			}
		}

		/* If this is a unicast packet from another backgone gw,
		 * drop it.
		 */
		orig_addr = ethhdr->h_source;
		orig_node = batadv_orig_hash_find(bat_priv, orig_addr);
		if (orig_node &&
		    batadv_bla_is_backbone_gw(skb, orig_node, hdr_size)) {
			batadv_dbg(BATADV_DBG_BLA, bat_priv,
				   "recv_unicast_packet(): Dropped unicast pkt received from another backbone gw %pM.\n",
				   orig_addr);
			batadv_orig_node_put(orig_node);
			return NET_RX_DROP;
		}

Kind regards,
	Sven
  

Patch

diff --git a/net/batman-adv/routing.c b/net/batman-adv/routing.c
index ae850f2..1ef0735 100644
--- a/net/batman-adv/routing.c
+++ b/net/batman-adv/routing.c
@@ -864,9 +864,11 @@  int batadv_recv_unicast_packet(struct sk_buff *skb,
 	int check, hdr_size = sizeof(*unicast_packet);
 	enum batadv_subtype subtype;
 	bool is4addr;
+	struct ethhdr *ethhdr;

 	unicast_packet = (struct batadv_unicast_packet *)skb->data;
 	unicast_4addr_packet = (struct batadv_unicast_4addr_packet *)skb->data;
+	ethhdr = eth_hdr(skb);

 	is4addr = unicast_packet->packet_type == BATADV_UNICAST_4ADDR;
 	/* the caller function should have already pulled 2 bytes */
@@ -906,6 +908,20 @@  int batadv_recv_unicast_packet(struct sk_buff *skb,
 			}
 		}

+		/* If this is a unicast packet from another backgone gw,
+		 * drop it.
+		 */
+		orig_addr = ethhdr->h_source;
+		orig_node = batadv_orig_hash_find(bat_priv, orig_addr);
+		if (orig_node &&
+		    batadv_bla_is_backbone_gw(skb, orig_node, hdr_size)) {
+			batadv_dbg(BATADV_DBG_BLA, bat_priv,
+				   "Dropped unicast pkt received from another backbone gw %pM.\n",
+				   orig_addr);
+			batadv_orig_node_put(orig_node);
+			return NET_RX_DROP;
+		}
+
 		if (batadv_dat_snoop_incoming_arp_request(bat_priv, skb,
 							  hdr_size))
 			goto rx_success;