Folks
Looks like we have an issue with linux-2.6.26 & ipv6
Scenario: no ipv6 default route set.
Repro: Enter command
# ip -f inet6 route get fec0::1
And we get BUG: unable to handle kernel NULL pointer deref....
This has been an issue since linux-2.6.26-rc4. It's taken a while to
nail it. We are currently testing linux-2.6.26.2.
This appears to have been an issue in the past. This is where I got the
magic ip route command from.
http://www.ussg.iu.edu/hypermail/linux/kernel/0510.2/0522.html
http://www.ussg.iu.edu/hypermail/linux/kernel/0510.2/0535.html
http://www.ussg.iu.edu/hypermail/linux/kernel/0510.2/1522.html
~ # ip -f inet6 route get fec0::1
Produces, with linux-2.6.26.2,
BUG: unable to handle kernel NULL pointer dereference at 00000000
IP: [<c0369b85>] rt6_fill_node+0x175/0x3b0
*pdpt = 0000000036466001 *pde = 0000000000000000
Oops: 0000 [#1] SMP
Modules linked in: pcnet32 smsc47m192 i2c_i801 i2c_dev i2c_core r8169
coretemp i
t87 hwmon_vid lcm e1000e
Pid: 3033, comm: ip Not tainted (2.6.26.2 #1)
EIP: 0060:[<c0369b85>] EFLAGS: 00010246 CPU: 1
EIP is at rt6_fill_node+0x175/0x3b0
EAX: 00000000 EBX: f7115bbc ECX: 00000000 EDX: f7115c60
ESI: f7c1f100 EDI: f7548f00 EBP: f7115bdc ESP: f7115ba4
DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068
Process ip (pid: 3033, ti=f7114000 task=f64cbc50 task.ti=f7114000)
Stack: f7115bbc 00000000 f7115c54 f7115bc0 f7115c60 f6d75078 00000000
f7115bdc
c036a5f0 c036b360 00000000 f75487a0 00000000 f7548f00 f7115c9c
c036c30e
f7115c70 00000000 00000018 00000bd9 489b2024 00000000 00000000
00000000
Call Trace:
[<c036a5f0>] ? ip6_route_output+0x50/0xa0
[<c036b360>] ? ip6_pol_route_output+0x0/0x20
[<c036c30e>] ? inet6_rtm_getroute+0x16e/0x200
[<c036c1a0>] ? inet6_rtm_getroute+0x0/0x200
[<c030ef19>] ? rtnetlink_rcv_msg+0x1b9/0x1f0
[<c030ed60>] ? rtnetlink_rcv_msg+0x0/0x1f0
[<c031426d>] ? netlink_rcv_skb+0x8d/0xb0
[<c030ed57>] ? rtnetlink_rcv+0x17/0x20
[<c031402d>] ? netlink_unicast+0x23d/0x270
[<c030162a>] ? memcpy_fromiovec+0x4a/0x70
[<c0314811>] ? netlink_sendmsg+0x1c1/0x290
[<c02fa165>] ? sock_sendmsg+0xc5/0xf0
[<c01363a0>] ? autoremove_wake_function+0x0/0x50
[<c01363a0>] ? autoremove_wake_function+0x0/0x50
[<c02fa165>] ? sock_sendmsg+0xc5/0xf0
[<c0217f37>] ? copy_from_user+0x37/0x70
[<c03018ec>] ? verify_iovec+0x2c/0x90
[<c02fa29a>] ? sys_sendmsg+0x10a/0x220
[<c015ab08>] ? __inc_zone_page_state+0x18/0x20
[<c01642ed>] ? __page_set_anon_rmap+0x2d/0x40
[<c0164325>] ? page_add_new_anon_rmap+0x25/0x30
[<c015eda6>] ? handle_mm_fault+0x606/0x750
[<c0160f5e>] ? vma_adjust+0xfe/0x410
[<c0113156>] ? do_page_fault+0x126/0x830
[<c02fb343>] ? sys_socketcall+0x233/0x260
[<c0102f39>] ? sysenter_past_esp+0x6a/0x91
=======================
Code: 62 01 00 00 c6 43 01 80 8b 45 0c 85 c0 0f 85 13 02 00 00 8b 45 d8
85 c0 74
3c 8b 86 88 00 00 00 8d 5d e0 31 c9 89 1c 24 8b 55 d8 <8b> 00 e8 d4 e3
ff ff 85
c0 75 20 b9 10 00 00 00 ba 07 00 00 00
EIP: [<c0369b85>] rt6_fill_node+0x175/0x3b0 SS:ESP 0068:f7115ba4
---[ end trace e9f2563374550ae8 ]---
I will look into producing a patch.
Best regards
John Gumb
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 }
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 <[email protected]>
looks good to me.
thanks people.
John
On Fri, 2008-08-08 at 00:57 -0400, Brian Haley wrote:
> 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 <[email protected]>
>
>
Brian Haley wrote:
> 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 <[email protected]>
Cc: Stable <[email protected]>
Eugene
Hi,
<quote sender="Brian Haley">
> Alexey Dobriyan wrote:
>> 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 <[email protected]>
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 <[email protected]>
Signed-off-by: Eugene Teo <[email protected]>
---
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_t
diff --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: Eugene Teo <[email protected]>
Date: Mon, 11 Aug 2008 16:40:24 +0800
> Brian Haley wrote:
> > 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 <[email protected]>
>
> Cc: Stable <[email protected]>
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.
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.
David Miller wrote:
> From: Eugene Teo <[email protected]>
> Date: Mon, 11 Aug 2008 16:40:24 +0800
>
>> Brian Haley wrote:
>>> 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 <[email protected]>
>> Cc: Stable <[email protected]>
>
> 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
Eugene Teo wrote:
> 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 <[email protected]>
> Signed-off-by: Eugene Teo <[email protected]>
Acked-by: Brian Haley <[email protected]>
But Yoshfuji might have another opinion since he did the work to remove
ipv6_get_saddr() in the first place.
-Brian
Eugene Teo wrote:
> David Miller wrote:
>> From: Eugene Teo <[email protected]>
>> Date: Mon, 11 Aug 2008 16:40:24 +0800
[...]
> 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
Ok, so there's a mistake in my patch. It should return the loopback MAC
address instead. I'm wondering if the fix should be related to
initialising rt6i_idev in addrconf_init routine like in the upstream
commit: c62dba9011b93fd88fde929848582b2a98309878. The code changed quite
a lot.
Eugene
Eugene Teo wrote:
> Eugene Teo wrote:
>> David Miller wrote:
>>> From: Eugene Teo <[email protected]>
>>> Date: Mon, 11 Aug 2008 16:40:24 +0800
> [...]
>> 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
>
> Ok, so there's a mistake in my patch. It should return the loopback MAC
s/MAC/ipv6/
> address instead. I'm wondering if the fix should be related to
> initialising rt6i_idev in addrconf_init routine like in the upstream
> commit: c62dba9011b93fd88fde929848582b2a98309878. The code changed quite
> a lot.
So I checked, it's initialised in ip6_route_init().
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:[email protected]]
Sent: 11 August 2008 12:04
To: Brian Haley
Cc: Alexey Dobriyan; John Gumb; [email protected];
[email protected]; 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:
>> 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 <[email protected]>
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 <[email protected]>
Signed-off-by: Eugene Teo <[email protected]>
---
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_t
diff --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" <[email protected]>
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 <[email protected]>
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 <[email protected]>
Signed-off-by: David S. Miller <[email protected]>
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);
}