Skip to contentSkip to conversation

Allocate unique metrics for icmp6 packets to prevent tainting dst metrics

PATCH Superseded Nick Jones 2 participants 2 messages Mar 12, 2012

Subsystems (3)

v1 of 4 Superseded Latest is v4 →

lore
Share & tools
lore

Nick Jones

author
#0 lore raw

PATCH 1/1 Allocate unique metrics for icmp6 packets to prevent tainting dst metrics

The generation of an icmp6 packet, targeted to a specific desination address, will cause the shared metrics of the ip6_dst and inetpeer of that address to be tainted with the hoplimit value 255. All packets, icmp6 or otherwise, will have this hoplimit value, and if the destination is a router, not even advertisements specifying a new hoplimit value will have any effect due to the way ip6_dst_hoplimit works.

By allocating a unique metrics array for the icmp6 packet, the shared metrics will not be tainted.

---

First follow up after discussion at:
http://www.spinics.net/lists/netdev/msg191052.html

 net/ipv6/route.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)
net/ipv6/route.c +8 -0
index 92be12b..209d156 100644
@@ -1117,6 +1117,14 @@ struct dst_entry *icmp6_dst_alloc(struct net_device *dev,
 	rt->rt6i_dst.addr = fl6->daddr;
 	rt->rt6i_dst.plen = 128;
 	rt->rt6i_idev     = idev;
+
+	u32 *metrics = kzalloc(sizeof(u32) * RTAX_MAX, GFP_ATOMIC);
+	if (unlikely(!metrics)) {
+		in6_dev_put(idev);
+		dst_free(&rt->dst);
+		return ERR_CAST(-ENOMEM);
+	}
+	dst_init_metrics(&rt->dst, metrics, 0);
 	dst_metric_set(&rt->dst, RTAX_HOPLIMIT, 255);
 
 	spin_lock_bh(&icmp6_dst_lock);
Signature · 1 line
1.7.1
maintainer
#1 lore raw

From: Nick Jones <[email protected]>
Date: Mon, 12 Mar 2012 23:16:14 +0800

Quoted 8 lines

The generation of an icmp6 packet, targeted to a specific desination address, will cause the shared metrics of the ip6_dst and inetpeer of that address to be tainted with the hoplimit value 255. All packets, icmp6 or otherwise, will have this hoplimit value, and if the destination is a router, not even advertisements specifying a new hoplimit value will have any effect due to the way ip6_dst_hoplimit works.

Quoted 4 lines

By allocating a unique metrics array for the icmp6 packet, the shared metrics will not be tainted.

You can't just change the allocation side.

You now have to make sure the free'ing side knows that these special routes use kmalloc()'d metrics. On ipv6 this is implemented in ip6_dst_destroy(). Unless DST_HOST will be clear on all of these icmp6 routes, the metrics will be leaked because ip6_dst_destroy() will not invoke dst_destroy_metrics_generic() which would do the kfree().