| 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
| ||
|
Date: Mon, 06 Nov 2017 23:03:02 +0000
From: Ben Hutchings <ben@...adent.org.uk>
To: linux-kernel@...r.kernel.org, stable@...r.kernel.org
CC: akpm@...ux-foundation.org,
"Andrey Konovalov" <andreyknvl@...gle.com>,
"Cong Wang" <xiyou.wangcong@...il.com>,
"Julian Anastasov" <ja@....bg>,
"Eric Dumazet" <edumazet@...gle.com>,
"David S. Miller" <davem@...emloft.net>
Subject: [PATCH 3.16 115/294] ipv4: add reference counting to metrics
3.16.50-rc1 review patch. If anyone has any objections, please let me know.
------------------
From: Eric Dumazet <edumazet@...gle.com>
commit 3fb07daff8e99243366a081e5129560734de4ada upstream.
Andrey Konovalov reported crashes in ipv4_mtu()
I could reproduce the issue with KASAN kernels, between
10.246.7.151 and 10.246.7.152 :
1) 20 concurrent netperf -t TCP_RR -H 10.246.7.152 -l 1000 &
2) At the same time run following loop :
while :
do
ip ro add 10.246.7.152 dev eth0 src 10.246.7.151 mtu 1500
ip ro del 10.246.7.152 dev eth0 src 10.246.7.151 mtu 1500
done
Cong Wang attempted to add back rt->fi in commit
82486aa6f1b9 ("ipv4: restore rt->fi for reference counting")
but this proved to add some issues that were complex to solve.
Instead, I suggested to add a refcount to the metrics themselves,
being a standalone object (in particular, no reference to other objects)
I tried to make this patch as small as possible to ease its backport,
instead of being super clean. Note that we believe that only ipv4 dst
need to take care of the metric refcount. But if this is wrong,
this patch adds the basic infrastructure to extend this to other
families.
Many thanks to Julian Anastasov for reviewing this patch, and Cong Wang
for his efforts on this problem.
Fixes: 2860583fe840 ("ipv4: Kill rt->fi")
Signed-off-by: Eric Dumazet <edumazet@...gle.com>
Reported-by: Andrey Konovalov <andreyknvl@...gle.com>
Reviewed-by: Julian Anastasov <ja@....bg>
Acked-by: Cong Wang <xiyou.wangcong@...il.com>
Signed-off-by: David S. Miller <davem@...emloft.net>
[bwh: Backported to 3.16:
- Give DST_METRICS_REFCOUNTED a value of 4 and struct dst_metrics an
alignment of 8
- Update dst_metrics cache size
- Adjust context]
Signed-off-by: Ben Hutchings <ben@...adent.org.uk>
---
--- a/include/net/dst.h
+++ b/include/net/dst.h
@@ -108,16 +108,22 @@ struct dst_entry {
void *dst_alloc_metrics(gfp_t flags);
void dst_free_metrics(void *metrics);
u32 *dst_cow_metrics_generic(struct dst_entry *dst, unsigned long old);
-extern const u32 dst_default_metrics[];
#define DST_METRICS_READ_ONLY 0x1UL
#define DST_METRICS_FORCE_OVERWRITE 0x2UL
+#define DST_METRICS_REFCOUNTED 0x4UL
#define DST_METRICS_FLAGS 0x7UL
#define DST_METRICS_ALIGNMENT 0x8UL
#define __DST_METRICS_PTR(Y) \
((u32 *)((Y) & ~DST_METRICS_FLAGS))
#define DST_METRICS_PTR(X) __DST_METRICS_PTR((X)->_metrics)
+struct dst_metrics {
+ u32 metrics[RTAX_MAX];
+ atomic_t refcnt;
+} __aligned(DST_METRICS_ALIGNMENT);
+extern const struct dst_metrics dst_default_metrics;
+
static inline bool dst_metrics_read_only(const struct dst_entry *dst)
{
return dst->_metrics & DST_METRICS_READ_ONLY;
--- a/include/net/ip_fib.h
+++ b/include/net/ip_fib.h
@@ -108,11 +108,11 @@ struct fib_info {
unsigned char fib_type;
__be32 fib_prefsrc;
u32 fib_priority;
- u32 *fib_metrics;
-#define fib_mtu fib_metrics[RTAX_MTU-1]
-#define fib_window fib_metrics[RTAX_WINDOW-1]
-#define fib_rtt fib_metrics[RTAX_RTT-1]
-#define fib_advmss fib_metrics[RTAX_ADVMSS-1]
+ struct dst_metrics *fib_metrics;
+#define fib_mtu fib_metrics->metrics[RTAX_MTU-1]
+#define fib_window fib_metrics->metrics[RTAX_WINDOW-1]
+#define fib_rtt fib_metrics->metrics[RTAX_RTT-1]
+#define fib_advmss fib_metrics->metrics[RTAX_ADVMSS-1]
int fib_nhs;
#ifdef CONFIG_IP_ROUTE_MULTIPATH
int fib_power;
--- a/net/core/dst.c
+++ b/net/core/dst.c
@@ -149,13 +149,13 @@ int dst_discard_sk(struct sock *sk, stru
}
EXPORT_SYMBOL(dst_discard_sk);
-const u32 dst_default_metrics[RTAX_MAX + 1] __aligned(DST_METRICS_ALIGNMENT) = {
+const struct dst_metrics dst_default_metrics = {
/* This initializer is needed to force linker to place this variable
* into const section. Otherwise it might end into bss section.
* We really want to avoid false sharing on this variable, and catch
* any writes on it.
*/
- [RTAX_MAX] = 0xdeadbeef,
+ .refcnt = ATOMIC_INIT(1),
};
@@ -176,7 +176,7 @@ void *dst_alloc(struct dst_ops *ops, str
if (dev)
dev_hold(dev);
dst->ops = ops;
- dst_init_metrics(dst, dst_default_metrics, true);
+ dst_init_metrics(dst, dst_default_metrics.metrics, true);
dst->expires = 0UL;
dst->path = dst;
dst->from = NULL;
@@ -308,25 +308,30 @@ EXPORT_SYMBOL(dst_free_metrics);
u32 *dst_cow_metrics_generic(struct dst_entry *dst, unsigned long old)
{
- u32 *p = dst_alloc_metrics(GFP_ATOMIC);
+ struct dst_metrics *p = dst_alloc_metrics(GFP_ATOMIC);
if (p) {
- u32 *old_p = __DST_METRICS_PTR(old);
+ struct dst_metrics *old_p = (struct dst_metrics *)__DST_METRICS_PTR(old);
unsigned long prev, new;
- memcpy(p, old_p, sizeof(u32) * RTAX_MAX);
+ atomic_set(&p->refcnt, 1);
+ memcpy(p->metrics, old_p->metrics, sizeof(p->metrics));
new = (unsigned long) p;
prev = cmpxchg(&dst->_metrics, old, new);
if (prev != old) {
dst_free_metrics(p);
- p = __DST_METRICS_PTR(prev);
+ p = (struct dst_metrics *)__DST_METRICS_PTR(prev);
if (prev & DST_METRICS_READ_ONLY)
p = NULL;
+ } else if (prev & DST_METRICS_REFCOUNTED) {
+ if (atomic_dec_and_test(&old_p->refcnt))
+ dst_free_metrics(old_p);
}
}
- return p;
+ BUILD_BUG_ON(offsetof(struct dst_metrics, metrics) != 0);
+ return (u32 *)p;
}
EXPORT_SYMBOL(dst_cow_metrics_generic);
@@ -335,7 +340,7 @@ void __dst_destroy_metrics_generic(struc
{
unsigned long prev, new;
- new = ((unsigned long) dst_default_metrics) | DST_METRICS_READ_ONLY;
+ new = ((unsigned long) &dst_default_metrics) | DST_METRICS_READ_ONLY;
prev = cmpxchg(&dst->_metrics, old, new);
if (prev == old)
dst_free_metrics(__DST_METRICS_PTR(old));
@@ -434,7 +439,7 @@ void __init dst_init(void)
{
register_netdevice_notifier(&dst_dev_notifier);
metrics_cache = kmem_cache_create("dst_metrics",
- sizeof(u32) * RTAX_MAX,
+ sizeof(struct dst_metrics),
DST_METRICS_ALIGNMENT,
SLAB_PANIC, NULL);
}
--- a/net/ipv4/fib_semantics.c
+++ b/net/ipv4/fib_semantics.c
@@ -201,6 +201,7 @@ static void rt_fibinfo_free_cpus(struct
static void free_fib_info_rcu(struct rcu_head *head)
{
struct fib_info *fi = container_of(head, struct fib_info, rcu);
+ struct dst_metrics *m;
change_nexthops(fi) {
if (nexthop_nh->nh_dev)
@@ -212,8 +213,9 @@ static void free_fib_info_rcu(struct rcu
} endfor_nexthops(fi);
release_net(fi->fib_net);
- if (fi->fib_metrics != (u32 *) dst_default_metrics)
- dst_free_metrics(fi->fib_metrics);
+ m = fi->fib_metrics;
+ if (m != &dst_default_metrics && atomic_dec_and_test(&m->refcnt))
+ dst_free_metrics(m);
kfree(fi);
}
@@ -826,8 +828,9 @@ struct fib_info *fib_create_info(struct
fi->fib_metrics = dst_alloc_metrics(GFP_KERNEL | __GFP_ZERO);
if (!fi->fib_metrics)
goto failure;
+ atomic_set(&fi->fib_metrics->refcnt, 1);
} else
- fi->fib_metrics = (u32 *) dst_default_metrics;
+ fi->fib_metrics = (struct dst_metrics *)&dst_default_metrics;
fi->fib_net = hold_net(net);
fi->fib_protocol = cfg->fc_protocol;
@@ -864,7 +867,7 @@ struct fib_info *fib_create_info(struct
val = 65535 - 15;
if (type == RTAX_HOPLIMIT && val > 255)
val = 255;
- fi->fib_metrics[type - 1] = val;
+ fi->fib_metrics->metrics[type - 1] = val;
}
}
}
@@ -1029,7 +1032,7 @@ int fib_dump_info(struct sk_buff *skb, u
if (fi->fib_priority &&
nla_put_u32(skb, RTA_PRIORITY, fi->fib_priority))
goto nla_put_failure;
- if (rtnetlink_put_metrics(skb, fi->fib_metrics) < 0)
+ if (rtnetlink_put_metrics(skb, fi->fib_metrics->metrics) < 0)
goto nla_put_failure;
if (fi->fib_prefsrc &&
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -1344,8 +1344,12 @@ static void rt_add_uncached_list(struct
static void ipv4_dst_destroy(struct dst_entry *dst)
{
+ struct dst_metrics *p = (struct dst_metrics *)DST_METRICS_PTR(dst);
struct rtable *rt = (struct rtable *) dst;
+ if (p != &dst_default_metrics && atomic_dec_and_test(&p->refcnt))
+ dst_free_metrics(p);
+
if (!list_empty(&rt->rt_uncached)) {
spin_lock_bh(&rt_uncached_lock);
list_del(&rt->rt_uncached);
@@ -1392,7 +1396,11 @@ static void rt_set_nexthop(struct rtable
rt->rt_gateway = nh->nh_gw;
rt->rt_uses_gateway = 1;
}
- dst_init_metrics(&rt->dst, fi->fib_metrics, true);
+ dst_init_metrics(&rt->dst, fi->fib_metrics->metrics, true);
+ if (fi->fib_metrics != &dst_default_metrics) {
+ rt->dst._metrics |= DST_METRICS_REFCOUNTED;
+ atomic_inc(&fi->fib_metrics->refcnt);
+ }
#ifdef CONFIG_IP_ROUTE_CLASSID
rt->dst.tclassid = nh->nh_tclassid;
#endif
Powered by blists - more mailing lists