[06/18] batman-adv: Add length check for (received) tracker packets

Message ID 1296362366-3852-7-git-send-email-linus.luessing@saxnet.de (mailing list archive)
State Superseded, archived
Headers

Commit Message

Linus Lüssing Jan. 30, 2011, 4:39 a.m. UTC
  Before/while a tracker packet is being searched for next hops for its
destination entries, it will also be checked if the number of
destination and mcast entries might exceed the tracker_packet_len.
Otherwise we might read/write in unallocated memory.
Such a broken tracker packet could potentially occure when we are going
to reuse route_mcast_tracker_packet for tracker packets received from a
neighbour node.

In such a case, we are just reducing the stated mcast / dest numbers in
the tracker packet to fit the size of the allocated buffer.

Signed-off-by: Linus Lüssing <linus.luessing@saxnet.de>
---
 multicast.c |   25 +++++++++++++++++++++++--
 1 files changed, 23 insertions(+), 2 deletions(-)
  

Patch

diff --git a/batman-adv/multicast.c b/batman-adv/multicast.c
index f414394..ffeb800 100644
--- a/batman-adv/multicast.c
+++ b/batman-adv/multicast.c
@@ -403,17 +403,38 @@  out:
 	return ret;
 }
 
-/* Collect nexthops for all dest entries specified in this tracker packet */
+/* Collect nexthops for all dest entries specified in this tracker packet.
+ * It also reduces the number of elements in the tracker packet if they exceed
+ * the buffers length (e.g. because of a received, broken tracker packet) to
+ * avoid writing in unallocated memory. */
 static int tracker_next_hops(struct mcast_tracker_packet *tracker_packet,
+			     int tracker_packet_len,
 			     struct dest_entries_list *next_hops,
 			     struct bat_priv *bat_priv)
 {
 	int num_next_hops = 0, ret;
 	struct tracker_packet_state state;
+	uint8_t *tail = (uint8_t *)tracker_packet + tracker_packet_len;
 
 	INIT_LIST_HEAD(&next_hops->list);
 
 	tracker_packet_for_each_dest(&state, tracker_packet) {
+		/* avoid writing outside of unallocated memory later */
+		if (state.dest_entry + ETH_ALEN > tail) {
+			bat_dbg(DBG_BATMAN, bat_priv,
+				"mcast tracker packet is broken, too many "
+				"entries claimed for its length, repairing");
+
+			tracker_packet->num_mcast_entries = state.mcast_num;
+
+			if (state.dest_num) {
+				tracker_packet->num_mcast_entries++;
+				state.mcast_entry->num_dest = state.dest_num;
+			}
+
+			break;
+		}
+
 		ret = add_router_of_dest(next_hops, state.dest_entry,
 					 bat_priv);
 		if (!ret)
@@ -521,7 +542,7 @@  void route_mcast_tracker_packet(struct sk_buff *skb,
 	int num_next_hops;
 
 	num_next_hops = tracker_next_hops((struct mcast_tracker_packet*)
-				skb->data, &next_hops, bat_priv);
+				skb->data, skb->len, &next_hops, bat_priv);
 	if (!num_next_hops)
 		return;