OOPS, ip -f inet6 route get fec0::1, linux-2.6.26, ip6_route_output, rt6_fill_node+0x175

OOPS, ip -f inet6 route get fec0::1, linux-2.6.26, ip6_route_output, rt6_fill_node+0x175

[John Gumb]

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 

Re: OOPS, ip -f inet6 route get fec0::1, linux-2.6.26, ip6_route_output, rt6_fill_node+0x175

[Alexey Dobriyan]

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            }

Re: OOPS, ip -f inet6 route get fec0::1, linux-2.6.26, ip6_route_output, rt6_fill_node+0x175

[Brian Haley]

[-- Attachment #1: Type: text/plain, Size: 1153 bytes --]

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 <brian.haley@hp.com>



[-- Attachment #2: idev.patch --]
[-- Type: text/x-patch, Size: 551 bytes --]

diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 5a3e87e..303a245 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -2188,7 +2188,8 @@ 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,
+		struct inet6_dev *idev = ip6_dst_idev(&rt->u.dst);
+		if (ipv6_dev_get_saddr(idev ? idev->dev : NULL,
 				       dst, 0, &saddr_buf) == 0)
 			NLA_PUT(skb, RTA_PREFSRC, 16, &saddr_buf);
 	}

Re: OOPS, ip -f inet6 route get fec0::1, linux-2.6.26, ip6_route_output, rt6_fill_node+0x175

[John Gumb]

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 <brian.haley@hp.com>
> 
> 

Re: OOPS, ip -f inet6 route get fec0::1, linux-2.6.26, ip6_route_output, rt6_fill_node+0x175

[Eugene Teo]

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 <brian.haley@hp.com>

Cc: Stable <stable@kernel.org>

Eugene

Re: OOPS, ip -f inet6 route get fec0::1, linux-2.6.26, ip6_route_output, rt6_fill_node+0x175

[David Miller]

From: Eugene Teo <eteo@redhat.com>
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 <brian.haley@hp.com>
> 
> 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.

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.

Re: OOPS, ip -f inet6 route get fec0::1, linux-2.6.26, ip6_route_output, rt6_fill_node+0x175

[Eugene Teo]

David Miller wrote:
> From: Eugene Teo <eteo@redhat.com>
> 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 <brian.haley@hp.com>
>> 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

Re: OOPS, ip -f inet6 route get fec0::1, linux-2.6.26, ip6_route_output, rt6_fill_node+0x175

[Eugene Teo]

Eugene Teo wrote:
> David Miller wrote:
>> From: Eugene Teo <eteo@redhat.com>
>> 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

Re: OOPS, ip -f inet6 route get fec0::1, linux-2.6.26, ip6_route_output, rt6_fill_node+0x175

[Eugene Teo]

Eugene Teo wrote:
> Eugene Teo wrote:
>> David Miller wrote:
>>> From: Eugene Teo <eteo@redhat.com>
>>> 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

Re: OOPS, ip -f inet6 route get fec0::1, linux-2.6.26, ip6_route_output, rt6_fill_node+0x175

[Eugene Teo]

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 <brian.haley@hp.com>

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 <eugeneteo@kernel.sg>
---
 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",

Re: OOPS, ip -f inet6 route get fec0::1, linux-2.6.26, ip6_route_output, rt6_fill_node+0x175

[Brian Haley]

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 <stable@kernel.org>
> Signed-off-by: Eugene Teo <eugeneteo@kernel.sg>

Acked-by: Brian Haley <brian.haley@hp.com>

But Yoshfuji might have another opinion since he did the work to remove 
ipv6_get_saddr() in the first place.

-Brian

RE: OOPS, ip -f inet6 route get fec0::1, linux-2.6.26,ip6_route_output, rt6_fill_node+0x175

[John Gumb]

[-- Attachment #1: Type: text/plain, Size: 6205 bytes --]

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:
>> 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 <brian.haley@hp.com>

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 <eugeneteo@kernel.sg>
---
 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",

[-- Attachment #2: bug#52312-idev.patch --]
[-- Type: application/octet-stream, Size: 4804 bytes --]

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 <eugeneteo@kernel.sg>
---
 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",

Re: OOPS, ip -f inet6 route get fec0::1, linux-2.6.26,ip6_route_output, rt6_fill_node+0x175

[David Miller]

From: "John Gumb" <john.gumb@tandberg.com>
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 <brian.haley@hp.com>
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 <brian.haley@hp.com>
    Signed-off-by: David S. Miller <davem@davemloft.net>

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);
 	}