| 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
| ||
|
Message-Id: <20130226235537.466394657@linuxfoundation.org>
Date: Tue, 26 Feb 2013 15:56:25 -0800
From: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To: linux-kernel@...r.kernel.org
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
stable@...r.kernel.org, Eric Dumazet <eric.dumazet@...il.com>,
Neil Horman <nhorman@...driver.com>,
Gao Feng <gaofeng@...fujitsu.com>,
YOSHIFUJI Hideaki <yoshfuji@...ux-ipv6.org>,
Eric Dumazet <edumazet@...gle.com>,
"Steinar H. Gunderson" <sesse@...gle.com>,
"David S. Miller" <davem@...emloft.net>
Subject: [ 128/150] ipv6: fix race condition regarding dst->expires and dst->from.
3.8-stable review patch. If anyone has any objections, please let me know.
------------------
From: YOSHIFUJI Hideaki <yoshfuji@...ux-ipv6.org>
[ Upstream commit ecd9883724b78cc72ed92c98bcb1a46c764fff21 ]
Eric Dumazet wrote:
| Some strange crashes happen in rt6_check_expired(), with access
| to random addresses.
|
| At first glance, it looks like the RTF_EXPIRES and
| stuff added in commit 1716a96101c49186b
| (ipv6: fix problem with expired dst cache)
| are racy : same dst could be manipulated at the same time
| on different cpus.
|
| At some point, our stack believes rt->dst.from contains a dst pointer,
| while its really a jiffie value (as rt->dst.expires shares the same area
| of memory)
|
| rt6_update_expires() should be fixed, or am I missing something ?
|
| CC Neil because of https://bugzilla.redhat.com/show_bug.cgi?id=892060
Because we do not have any locks for dst_entry, we cannot change
essential structure in the entry; e.g., we cannot change reference
to other entity.
To fix this issue, split 'from' and 'expires' field in dst_entry
out of union. Once it is 'from' is assigned in the constructor,
keep the reference until the very last stage of the life time of
the object.
Of course, it is unsafe to change 'from', so make rt6_set_from simple
just for fresh entries.
Reported-by: Eric Dumazet <eric.dumazet@...il.com>
Reported-by: Neil Horman <nhorman@...driver.com>
CC: Gao Feng <gaofeng@...fujitsu.com>
Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@...ux-ipv6.org>
Reviewed-by: Eric Dumazet <edumazet@...gle.com>
Reported-by: Steinar H. Gunderson <sesse@...gle.com>
Reviewed-by: Neil Horman <nhorman@...driver.com>
Signed-off-by: David S. Miller <davem@...emloft.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
---
include/net/dst.h | 8 ++------
include/net/ip6_fib.h | 37 +++++++++++--------------------------
net/core/dst.c | 1 +
net/ipv6/route.c | 8 +++-----
4 files changed, 17 insertions(+), 37 deletions(-)
--- a/include/net/dst.h
+++ b/include/net/dst.h
@@ -36,13 +36,9 @@ struct dst_entry {
struct net_device *dev;
struct dst_ops *ops;
unsigned long _metrics;
- union {
- unsigned long expires;
- /* point to where the dst_entry copied from */
- struct dst_entry *from;
- };
+ unsigned long expires;
struct dst_entry *path;
- void *__pad0;
+ struct dst_entry *from;
#ifdef CONFIG_XFRM
struct xfrm_state *xfrm;
#else
--- a/include/net/ip6_fib.h
+++ b/include/net/ip6_fib.h
@@ -166,50 +166,35 @@ static inline struct inet6_dev *ip6_dst_
static inline void rt6_clean_expires(struct rt6_info *rt)
{
- if (!(rt->rt6i_flags & RTF_EXPIRES) && rt->dst.from)
- dst_release(rt->dst.from);
-
rt->rt6i_flags &= ~RTF_EXPIRES;
- rt->dst.from = NULL;
}
static inline void rt6_set_expires(struct rt6_info *rt, unsigned long expires)
{
- if (!(rt->rt6i_flags & RTF_EXPIRES) && rt->dst.from)
- dst_release(rt->dst.from);
-
- rt->rt6i_flags |= RTF_EXPIRES;
rt->dst.expires = expires;
+ rt->rt6i_flags |= RTF_EXPIRES;
}
-static inline void rt6_update_expires(struct rt6_info *rt, int timeout)
+static inline void rt6_update_expires(struct rt6_info *rt0, int timeout)
{
- if (!(rt->rt6i_flags & RTF_EXPIRES)) {
- if (rt->dst.from)
- dst_release(rt->dst.from);
- /* dst_set_expires relies on expires == 0
- * if it has not been set previously.
- */
- rt->dst.expires = 0;
- }
+ struct rt6_info *rt;
- dst_set_expires(&rt->dst, timeout);
- rt->rt6i_flags |= RTF_EXPIRES;
+ for (rt = rt0; rt && !(rt->rt6i_flags & RTF_EXPIRES);
+ rt = (struct rt6_info *)rt->dst.from);
+ if (rt && rt != rt0)
+ rt0->dst.expires = rt->dst.expires;
+
+ dst_set_expires(&rt0->dst, timeout);
+ rt0->rt6i_flags |= RTF_EXPIRES;
}
static inline void rt6_set_from(struct rt6_info *rt, struct rt6_info *from)
{
struct dst_entry *new = (struct dst_entry *) from;
- if (!(rt->rt6i_flags & RTF_EXPIRES) && rt->dst.from) {
- if (new == rt->dst.from)
- return;
- dst_release(rt->dst.from);
- }
-
rt->rt6i_flags &= ~RTF_EXPIRES;
- rt->dst.from = new;
dst_hold(new);
+ rt->dst.from = new;
}
static inline void ip6_rt_put(struct rt6_info *rt)
--- a/net/core/dst.c
+++ b/net/core/dst.c
@@ -179,6 +179,7 @@ void *dst_alloc(struct dst_ops *ops, str
dst_init_metrics(dst, dst_default_metrics, true);
dst->expires = 0UL;
dst->path = dst;
+ dst->from = NULL;
#ifdef CONFIG_XFRM
dst->xfrm = NULL;
#endif
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -300,6 +300,7 @@ static void ip6_dst_destroy(struct dst_e
{
struct rt6_info *rt = (struct rt6_info *)dst;
struct inet6_dev *idev = rt->rt6i_idev;
+ struct dst_entry *from = dst->from;
if (rt->n)
neigh_release(rt->n);
@@ -312,8 +313,8 @@ static void ip6_dst_destroy(struct dst_e
in6_dev_put(idev);
}
- if (!(rt->rt6i_flags & RTF_EXPIRES) && dst->from)
- dst_release(dst->from);
+ dst->from = NULL;
+ dst_release(from);
if (rt6_has_peer(rt)) {
struct inet_peer *peer = rt6_peer_ptr(rt);
@@ -1054,7 +1055,6 @@ struct dst_entry *ip6_blackhole_route(st
rt->rt6i_gateway = ort->rt6i_gateway;
rt->rt6i_flags = ort->rt6i_flags;
- rt6_clean_expires(rt);
rt->rt6i_metric = 0;
memcpy(&rt->rt6i_dst, &ort->rt6i_dst, sizeof(struct rt6key));
@@ -1859,8 +1859,6 @@ static struct rt6_info *ip6_rt_copy(stru
if ((ort->rt6i_flags & (RTF_DEFAULT | RTF_ADDRCONF)) ==
(RTF_DEFAULT | RTF_ADDRCONF))
rt6_set_from(rt, ort);
- else
- rt6_clean_expires(rt);
rt->rt6i_metric = 0;
#ifdef CONFIG_IPV6_SUBTREES
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists