On Thu, Aug 07, 2008 at 07:00:56PM +0200, John Gumb wrote:
Scenario: no ipv6 default route set.
# ip -f inet6 route get fec0::1
BUG: unable to handle kernel NULL pointer dereference at 00000000
IP: [<c0369b85>] rt6_fill_node+0x175/0x3b0
EIP is at rt6_fill_node+0x175/0x3b0
ip6_route_output+0x50/0xa0
ip6_pol_route_output+0x0/0x20
inet6_rtm_getroute+0x16e/0x200
inet6_rtm_getroute+0x0/0x200
rtnetlink_rcv_msg+0x1b9/0x1f0
rtnetlink_rcv_msg+0x0/0x1f0
netlink_rcv_skb+0x8d/0xb0
rtnetlink_rcv+0x17/0x20
netlink_unicast+0x23d/0x270
memcpy_fromiovec+0x4a/0x70
netlink_sendmsg+0x1c1/0x290
sock_sendmsg+0xc5/0xf0
autoremove_wake_function+0x0/0x50
autoremove_wake_function+0x0/0x50
sock_sendmsg+0xc5/0xf0
copy_from_user+0x37/0x70
verify_iovec+0x2c/0x90
sys_sendmsg+0x10a/0x220
0xffffffff80424dd3 is in rt6_fill_node (net/ipv6/route.c:2191).
2186 } else
2187 #endif
2188 NLA_PUT_U32(skb, RTA_IIF, iif);
2189 } else if (dst) {
2190 struct in6_addr saddr_buf;
2191 ====> if (ipv6_dev_get_saddr(ip6_dst_idev(&rt->u.dst)->dev,
^^^^^^^^^^^^^^^^^^^^^^^^
NULL
2192 dst, 0, &saddr_buf) == 0)
2193 NLA_PUT(skb, RTA_PREFSRC, 16, &saddr_buf);
2194 }
Hi,
<quote sender="Brian Haley">
Alexey Dobriyan wrote:
quoted
On Thu, Aug 07, 2008 at 07:00:56PM +0200, John Gumb wrote:
[...]
The commit that changed this can't be reverted easily, but the patch
below works for me.
Fix NULL de-reference in rt6_fill_node() when there's no IPv6 input
device present in the dst entry.
Signed-off-by: Brian Haley <redacted>
I think it's better to use a helper routine like ipv6_get_saddr to make
sure that both dst and rt6i_idev arguments are checked for NULL.
I have compiled, and tested the patch.
Thanks,
Eugene
---
Fix NULL pointer dereference in rt6_fill_node().
# ip -f inet6 route get fec0::1
BUG: unable to handle kernel NULL pointer dereference at 00000000
IP: [<c0369b85>] rt6_fill_node+0x175/0x3b0
EIP is at rt6_fill_node+0x175/0x3b0
Cc: Stable <stable@kernel.org>
Signed-off-by: Eugene Teo <redacted>
---
include/net/addrconf.h | 4 ++++
include/net/ip6_fib.h | 2 +-
net/ipv6/addrconf.c | 8 ++++++++
net/ipv6/fib6_rules.c | 3 +--
net/ipv6/ip6_output.c | 3 +--
net/ipv6/route.c | 3 +--
net/ipv6/xfrm6_policy.c | 3 +--
net/sctp/ipv6.c | 3 +--
8 files changed, 18 insertions(+), 11 deletions(-)
diff --git a/include/net/addrconf.h b/include/net/addrconf.h
index 06b2814..c43ad8e 100644
--- a/include/net/addrconf.h
+++ b/include/net/addrconf.h
@@ -80,6 +80,10 @@ extern struct inet6_ifaddr *ipv6_get_ifaddr(struct net *net,
struct net_device *dev,
int strict);
+extern int ipv6_get_saddr(struct dst_entry *dst,
+ const struct in6_addr *daddr,
+ unsigned int srcprefs,
+ struct in6_addr *saddr);
extern int ipv6_dev_get_saddr(struct net_device *dev,
const struct in6_addr *daddr,
unsigned int srcprefs,
diff --git a/include/net/ip6_fib.h b/include/net/ip6_fib.h
index 7c5c0f7..73f7f6b 100644
--- a/include/net/ip6_fib.h
+++ b/include/net/ip6_fib.h
@@ -118,7 +118,7 @@ struct rt6_info
static inline struct inet6_dev *ip6_dst_idev(struct dst_entry *dst)
{
- return ((struct rt6_info *)dst)->rt6i_idev;
+ return dst ? ((struct rt6_info *)dst)->rt6i_idev : NULL;
}
struct fib6_walker_tdiff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index a7842c5..55647bd 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -1239,6 +1239,14 @@ try_nextdev:
return 0;
}
+int ipv6_get_saddr(struct dst_entry *dst, const struct in6_addr *daddr,
+ unsigned int srcprefs, struct in6_addr *saddr)
+{
+ struct inet6_dev *idev = ip6_dst_idev(dst);
+ return ipv6_dev_get_saddr(idev ? idev->dev : NULL, daddr, srcprefs, saddr);
+
+}
+EXPORT_SYMBOL(ipv6_get_saddr);
EXPORT_SYMBOL(ipv6_dev_get_saddr);
int ipv6_get_lladdr(struct net_device *dev, struct in6_addr *addr,diff --git a/net/ipv6/fib6_rules.c b/net/ipv6/fib6_rules.c
index 8d05527..27c859c 100644
--- a/net/ipv6/fib6_rules.c
+++ b/net/ipv6/fib6_rules.c
@@ -93,8 +93,7 @@ static int fib6_rule_action(struct fib_rule *rule, struct flowi *flp,
if (flags & RT6_LOOKUP_F_SRCPREF_COA)
srcprefs |= IPV6_PREFER_SRC_COA;
- if (ipv6_dev_get_saddr(ip6_dst_idev(&rt->u.dst)->dev,
- &flp->fl6_dst, srcprefs,
+ if (ipv6_get_saddr(&rt->u.dst, &flp->fl6_dst, srcprefs,
&saddr))
goto again;
if (!ipv6_prefix_equal(&saddr, &r->src.addr,
diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index a4402de..fc5f4f4 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -934,8 +934,7 @@ static int ip6_dst_lookup_tail(struct sock *sk,
goto out_err_release;
if (ipv6_addr_any(&fl->fl6_src)) {
- err = ipv6_dev_get_saddr(ip6_dst_idev(*dst)->dev,
- &fl->fl6_dst,
+ err = ipv6_get_saddr(*dst, &fl->fl6_dst,
sk ? inet6_sk(sk)->srcprefs : 0,
&fl->fl6_src);
if (err)diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 5a3e87e..950d709 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -2188,8 +2188,7 @@ static int rt6_fill_node(struct sk_buff *skb, struct rt6_info *rt,
NLA_PUT_U32(skb, RTA_IIF, iif);
} else if (dst) {
struct in6_addr saddr_buf;
- if (ipv6_dev_get_saddr(ip6_dst_idev(&rt->u.dst)->dev,
- dst, 0, &saddr_buf) == 0)
+ if (ipv6_get_saddr(&rt->u.dst, dst, 0, &saddr_buf) == 0)
NLA_PUT(skb, RTA_PREFSRC, 16, &saddr_buf);
}
diff --git a/net/ipv6/xfrm6_policy.c b/net/ipv6/xfrm6_policy.c
index 8f1e054..ad582bd 100644
--- a/net/ipv6/xfrm6_policy.c
+++ b/net/ipv6/xfrm6_policy.c
@@ -57,8 +57,7 @@ static int xfrm6_get_saddr(xfrm_address_t *saddr, xfrm_address_t *daddr)
if (IS_ERR(dst))
return -EHOSTUNREACH;
- ipv6_dev_get_saddr(ip6_dst_idev(dst)->dev,
- (struct in6_addr *)&daddr->a6, 0,
+ ipv6_get_saddr(dst, (struct in6_addr *)&daddr->a6, 0,
(struct in6_addr *)&saddr->a6);
dst_release(dst);
return 0;
diff --git a/net/sctp/ipv6.c b/net/sctp/ipv6.c
index 483a01d..0fa9411 100644
--- a/net/sctp/ipv6.c
+++ b/net/sctp/ipv6.c
@@ -319,8 +319,7 @@ static void sctp_v6_get_saddr(struct sctp_sock *sk,
__func__, asoc, dst, NIP6(daddr->v6.sin6_addr));
if (!asoc) {
- ipv6_dev_get_saddr(dst ? ip6_dst_idev(dst)->dev : NULL,
- &daddr->v6.sin6_addr,
+ ipv6_get_saddr(dst, &daddr->v6.sin6_addr,
inet6_sk(&sk->inet.sk)->srcprefs,
&saddr->v6.sin6_addr);
SCTP_DEBUG_PRINTK("saddr from ipv6_get_saddr: " NIP6_FMT "\n",David Miller wrote:
From: Eugene Teo <redacted>
Date: Mon, 11 Aug 2008 16:40:24 +0800
quoted
Brian Haley wrote:
quoted
Alexey Dobriyan wrote:
quoted
On Thu, Aug 07, 2008 at 07:00:56PM +0200, John Gumb wrote:
quoted
Scenario: no ipv6 default route set.
# ip -f inet6 route get fec0::1
BUG: unable to handle kernel NULL pointer dereference at 00000000
IP: [<c0369b85>] rt6_fill_node+0x175/0x3b0
EIP is at rt6_fill_node+0x175/0x3b0
0xffffffff80424dd3 is in rt6_fill_node (net/ipv6/route.c:2191).
2186 } else
2187 #endif
2188 NLA_PUT_U32(skb, RTA_IIF, iif);
2189 } else if (dst) {
2190 struct in6_addr saddr_buf;
2191 ====> if
(ipv6_dev_get_saddr(ip6_dst_idev(&rt->u.dst)->dev,
^^^^^^^^^^^^^^^^^^^^^^^^
NULL
2192 dst, 0, &saddr_buf) == 0)
2193 NLA_PUT(skb, RTA_PREFSRC, 16,
&saddr_buf);
2194 }
The commit that changed this can't be reverted easily, but the patch
below works for me.
Fix NULL de-reference in rt6_fill_node() when there's no IPv6 input
device present in the dst entry.
Signed-off-by: Brian Haley <redacted>
Cc: Stable <stable@kernel.org>
We've already determined from one tester that the behavior is
changing with Brian's patch. Furthermore I haven't applied it
to mainline so it isn't anywhere near being submittable for
-stable.
With the patch I posted, this is the behaviour I get:
$ ip -f inet6 route get fec0::1
unreachable fec0::1 dev lo table unspec proto none src
fe80::214:4fff:fe0f:7332 metric -1 error -101 hoplimit 255
John emailed me that he will be testing this patch. I have not tested
Brian's patch.
I submit all relevant networking bug fixes to -stable when they are
ready and in a proper state to be submitted, you don't have to do it
for me. Sending me a gentle reminder or nudge, on the other hand, is
fine.
Thanks for letting me know. I wasn't familiar, but I welcome the hint.
Thanks,
Eugene
Folks
I've enclosed patch from Eugene just so we all know which patch we're
talking about. It 'works' according to the following definition:
a) Fixed OOPS
b) runs overnight in our test network. This run doesn't do much specific
ipv6 testing - but clearly what's there is catching stuff :-;
cheers
John
-----Original Message-----
From: Eugene Teo [mailto:eugeneteo@kernel.sg]
Sent: 11 August 2008 12:04
To: Brian Haley
Cc: Alexey Dobriyan; John Gumb; linux-kernel@vger.kernel.org;
netdev@vger.kernel.org; YOSHIFUJI Hideaki
Subject: Re: OOPS, ip -f inet6 route get fec0::1,
linux-2.6.26,ip6_route_output, rt6_fill_node+0x175
Hi,
<quote sender="Brian Haley">
Alexey Dobriyan wrote:
quoted
On Thu, Aug 07, 2008 at 07:00:56PM +0200, John Gumb wrote:
[...]
The commit that changed this can't be reverted easily, but the patch
below works for me.
Fix NULL de-reference in rt6_fill_node() when there's no IPv6 input
device present in the dst entry.
Signed-off-by: Brian Haley <redacted>
I think it's better to use a helper routine like ipv6_get_saddr to make
sure that both dst and rt6i_idev arguments are checked for NULL.
I have compiled, and tested the patch.
Thanks,
Eugene
---
Fix NULL pointer dereference in rt6_fill_node().
# ip -f inet6 route get fec0::1
BUG: unable to handle kernel NULL pointer dereference at 00000000
IP: [<c0369b85>] rt6_fill_node+0x175/0x3b0
EIP is at rt6_fill_node+0x175/0x3b0
Cc: Stable <stable@kernel.org>
Signed-off-by: Eugene Teo <redacted>
---
include/net/addrconf.h | 4 ++++
include/net/ip6_fib.h | 2 +-
net/ipv6/addrconf.c | 8 ++++++++
net/ipv6/fib6_rules.c | 3 +--
net/ipv6/ip6_output.c | 3 +--
net/ipv6/route.c | 3 +--
net/ipv6/xfrm6_policy.c | 3 +--
net/sctp/ipv6.c | 3 +--
8 files changed, 18 insertions(+), 11 deletions(-)
diff --git a/include/net/addrconf.h b/include/net/addrconf.h
index 06b2814..c43ad8e 100644
--- a/include/net/addrconf.h
+++ b/include/net/addrconf.h
@@ -80,6 +80,10 @@ extern struct inet6_ifaddr
*ipv6_get_ifaddr(struct net *net,
struct net_device *dev,
int strict);
+extern int ipv6_get_saddr(struct dst_entry *dst,
+ const struct in6_addr
*daddr,
+ unsigned int srcprefs,
+ struct in6_addr *saddr);
extern int ipv6_dev_get_saddr(struct net_device
*dev,
const struct in6_addr
*daddr,
unsigned int srcprefs,
diff --git a/include/net/ip6_fib.h b/include/net/ip6_fib.h
index 7c5c0f7..73f7f6b 100644
--- a/include/net/ip6_fib.h
+++ b/include/net/ip6_fib.h
@@ -118,7 +118,7 @@ struct rt6_info
static inline struct inet6_dev *ip6_dst_idev(struct dst_entry *dst)
{
- return ((struct rt6_info *)dst)->rt6i_idev;
+ return dst ? ((struct rt6_info *)dst)->rt6i_idev : NULL;
}
struct fib6_walker_tdiff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index a7842c5..55647bd 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -1239,6 +1239,14 @@ try_nextdev:
return 0;
}
+int ipv6_get_saddr(struct dst_entry *dst, const struct in6_addr *daddr,
+ unsigned int srcprefs, struct in6_addr *saddr)
+{
+ struct inet6_dev *idev = ip6_dst_idev(dst);
+ return ipv6_dev_get_saddr(idev ? idev->dev : NULL, daddr,srcprefs, saddr);
+
+}
+EXPORT_SYMBOL(ipv6_get_saddr);
EXPORT_SYMBOL(ipv6_dev_get_saddr);
int ipv6_get_lladdr(struct net_device *dev, struct in6_addr *addr,
diff --git a/net/ipv6/fib6_rules.c b/net/ipv6/fib6_rules.c
index 8d05527..27c859c 100644
--- a/net/ipv6/fib6_rules.c
+++ b/net/ipv6/fib6_rules.c
@@ -93,8 +93,7 @@ static int fib6_rule_action(struct fib_rule *rule,
struct flowi *flp,
if (flags & RT6_LOOKUP_F_SRCPREF_COA)
srcprefs |= IPV6_PREFER_SRC_COA;
- if
(ipv6_dev_get_saddr(ip6_dst_idev(&rt->u.dst)->dev,
- &flp->fl6_dst, srcprefs,
+ if (ipv6_get_saddr(&rt->u.dst, &flp->fl6_dst,
srcprefs,
&saddr))
goto again;
if (!ipv6_prefix_equal(&saddr, &r->src.addr,
diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index a4402de..fc5f4f4 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -934,8 +934,7 @@ static int ip6_dst_lookup_tail(struct sock *sk,
goto out_err_release;
if (ipv6_addr_any(&fl->fl6_src)) {
- err = ipv6_dev_get_saddr(ip6_dst_idev(*dst)->dev,
- &fl->fl6_dst,
+ err = ipv6_get_saddr(*dst, &fl->fl6_dst,
sk ? inet6_sk(sk)->srcprefs :0,
&fl->fl6_src);
if (err)
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 5a3e87e..950d709 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -2188,8 +2188,7 @@ static int rt6_fill_node(struct sk_buff *skb,
struct rt6_info *rt,
NLA_PUT_U32(skb, RTA_IIF, iif);
} else if (dst) {
struct in6_addr saddr_buf;
- if (ipv6_dev_get_saddr(ip6_dst_idev(&rt->u.dst)->dev,
- dst, 0, &saddr_buf) == 0)
+ if (ipv6_get_saddr(&rt->u.dst, dst, 0, &saddr_buf) == 0)
NLA_PUT(skb, RTA_PREFSRC, 16, &saddr_buf);
}
diff --git a/net/ipv6/xfrm6_policy.c b/net/ipv6/xfrm6_policy.c
index 8f1e054..ad582bd 100644
--- a/net/ipv6/xfrm6_policy.c
+++ b/net/ipv6/xfrm6_policy.c
@@ -57,8 +57,7 @@ static int xfrm6_get_saddr(xfrm_address_t *saddr,
xfrm_address_t *daddr)
if (IS_ERR(dst))
return -EHOSTUNREACH;
- ipv6_dev_get_saddr(ip6_dst_idev(dst)->dev,
- (struct in6_addr *)&daddr->a6, 0,
+ ipv6_get_saddr(dst, (struct in6_addr *)&daddr->a6, 0,
(struct in6_addr *)&saddr->a6);
dst_release(dst);
return 0;
diff --git a/net/sctp/ipv6.c b/net/sctp/ipv6.c
index 483a01d..0fa9411 100644
--- a/net/sctp/ipv6.c
+++ b/net/sctp/ipv6.c
@@ -319,8 +319,7 @@ static void sctp_v6_get_saddr(struct sctp_sock *sk,
__func__, asoc, dst,
NIP6(daddr->v6.sin6_addr));
if (!asoc) {
- ipv6_dev_get_saddr(dst ? ip6_dst_idev(dst)->dev : NULL,
- &daddr->v6.sin6_addr,
+ ipv6_get_saddr(dst, &daddr->v6.sin6_addr,
inet6_sk(&sk->inet.sk)->srcprefs,
&saddr->v6.sin6_addr);
SCTP_DEBUG_PRINTK("saddr from ipv6_get_saddr: " NIP6_FMT
"\n",
From: "John Gumb" <redacted>
Date: Tue, 12 Aug 2008 11:11:47 +0200
I've enclosed patch from Eugene just so we all know which patch we're
talking about. It 'works' according to the following definition:
a) Fixed OOPS
b) runs overnight in our test network. This run doesn't do much specific
ipv6 testing - but clearly what's there is catching stuff :-;
While Eugene's patch seems mostly fine, it's a bit over the top
to cure this OOPS and get backported to -stable I think.
So I've applied Brian's patch, as below, because we have many
other reports that it fixes the crash too.
I'd appreciate it if you'd test Brian's patch as well as you
tested Eugene's as this is what will go into the tree for the
time being.
We can reinvestigate Eugene's patch, but one thing I don't like
about it is that it adds this silly NULL check when that is
totally unnecessary in the vast majority of these call sites.
Yes yes, I'll submit this to stable too before someone bugs me
about that again. I'll first let this sit and get tested for
a few days before I do that submission so don't panic if you
don't see it for a few days.
commit 5e0115e500fe9dd2ca11e6f92db9123204f1327a
Author: Brian Haley [off-list ref]
Date: Wed Aug 13 01:58:57 2008 -0700
ipv6: Fix OOPS, ip -f inet6 route get fec0::1, linux-2.6.26, ip6_route_output, rt6_fill_node+0x175
Alexey Dobriyan wrote:
> On Thu, Aug 07, 2008 at 07:00:56PM +0200, John Gumb wrote:
>> Scenario: no ipv6 default route set.
>
>> # ip -f inet6 route get fec0::1
>>
>> BUG: unable to handle kernel NULL pointer dereference at 00000000
>> IP: [<c0369b85>] rt6_fill_node+0x175/0x3b0
>> EIP is at rt6_fill_node+0x175/0x3b0
>
> 0xffffffff80424dd3 is in rt6_fill_node (net/ipv6/route.c:2191).
> 2186 } else
> 2187 #endif
> 2188 NLA_PUT_U32(skb, RTA_IIF, iif);
> 2189 } else if (dst) {
> 2190 struct in6_addr saddr_buf;
> 2191 ====> if (ipv6_dev_get_saddr(ip6_dst_idev(&rt->u.dst)->dev,
> ^^^^^^^^^^^^^^^^^^^^^^^^
> NULL
>
> 2192 dst, 0, &saddr_buf) == 0)
> 2193 NLA_PUT(skb, RTA_PREFSRC, 16, &saddr_buf);
> 2194 }
The commit that changed this can't be reverted easily, but the patch
below works for me.
Fix NULL de-reference in rt6_fill_node() when there's no IPv6 input
device present in the dst entry.
Signed-off-by: Brian Haley [off-list ref]
Signed-off-by: David S. Miller [off-list ref]
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 5a3e87e..41b165f 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -2187,8 +2187,9 @@ static int rt6_fill_node(struct sk_buff *skb, struct rt6_info *rt,
#endif
NLA_PUT_U32(skb, RTA_IIF, iif);
} else if (dst) {
+ struct inet6_dev *idev = ip6_dst_idev(&rt->u.dst);
struct in6_addr saddr_buf;
- if (ipv6_dev_get_saddr(ip6_dst_idev(&rt->u.dst)->dev,
+ if (ipv6_dev_get_saddr(idev ? idev->dev : NULL,
dst, 0, &saddr_buf) == 0)
NLA_PUT(skb, RTA_PREFSRC, 16, &saddr_buf);
}