IPv6 badness continues

IPv6 badness continues

[Jeff Garzik]

This problem seems to be appearing much sooner, in 2.6.10 and 2.6.10-bk 
kernels.  Herbert, your patch did not apply to recent kernels, so I did 
not boot with it.

The first 'Badness in dst_release' message (of many) from 2.6.10-bk1, on 
x86 SMP, is found below.

Standard FC2 OS with IPv6 6to4 setup as described on 
http://linux.yyz.us/ipv6-fc2-howto.html

Sorry, this is my main fileserver and router, so I need to reboot it...

	Jeff


Badness in dst_release at include/net/dst.h:149
  [<f8c14a57>] ip6_dst_check+0x67/0x80 [ipv6]
  [<f8c0b2d4>] ip6_dst_lookup+0x1b4/0x1d0 [ipv6]
  [<f8c1e4b4>] udpv6_sendmsg+0x2b4/0x950 [ipv6]
  [<c0337954>] udp_recvmsg+0x64/0x300
  [<c033f12d>] inet_sendmsg+0x4d/0x60
  [<c02ed4bd>] sock_sendmsg+0xdd/0x100
  [<c011d694>] find_busiest_group+0xd4/0x300
  [<c01fa662>] copy_from_user+0x42/0x70
  [<c01371a0>] autoremove_wake_function+0x0/0x60
  [<c02eef5f>] sys_sendmsg+0x18f/0x1f0
  [<c011e431>] __wake_up_common+0x41/0x70
  [<c011e49e>] __wake_up+0x3e/0x60
  [<c0137877>] wake_futex+0x37/0x70
  [<c013792b>] futex_wake+0x7b/0xd0
  [<c01fa662>] copy_from_user+0x42/0x70
  [<c02ef402>] sys_socketcall+0x242/0x260

Re: IPv6 badness continues

[Lennert Buytenhek]

On Tue, Jan 04, 2005 at 01:40:32AM -0500, Jeff Garzik wrote:

> Standard FC2 OS with IPv6 6to4 setup as described on 
> http://linux.yyz.us/ipv6-fc2-howto.html

I upgraded my problematic machine to FC3, which somehow broke 6to4, and I
didn't bother to reenable it again because the local DNS server (dnscache)
just drops AAAA queries on the floor anyway, leading to lots of timeouts
when trying to resolve host names, etc.  Ever since 6to4 was turned off
on that box I stopped seeing these messages.


--L

Re: IPv6 badness continues

[Pekka Savola]

On Tue, 4 Jan 2005, Lennert Buytenhek wrote:
> On Tue, Jan 04, 2005 at 01:40:32AM -0500, Jeff Garzik wrote:
>
>> Standard FC2 OS with IPv6 6to4 setup as described on
>> http://linux.yyz.us/ipv6-fc2-howto.html
>
> I upgraded my problematic machine to FC3, which somehow broke 6to4, and I
> didn't bother to reenable it again because the local DNS server (dnscache)
> just drops AAAA queries on the floor anyway, leading to lots of timeouts
> when trying to resolve host names, etc.  Ever since 6to4 was turned off
> on that box I stopped seeing these messages.

Surely it doesn't just drop AAAA queries on the floor, but sends an 
error message?

If so, maybe then the problem is that glibc resolver cannot properly 
handle such errors without undue timeouts.

Actually I've just sent a message on libc-alpha@ describing a similar 
timeout issue:

http://sources.redhat.com/ml/libc-alpha/2005-01/msg00007.html

Maybe you could run a little bit of debug on whether the server sends 
errors (which kind) and how the resolver reacts.

-- 
Pekka Savola                 "You each name yourselves king, yet the
Netcore Oy                    kingdom bleeds."
Systems. Networks. Security. -- George R.R. Martin: A Clash of Kings

Re: IPv6 badness continues

[Jeff Garzik]

Lennert Buytenhek wrote:
> On Tue, Jan 04, 2005 at 01:40:32AM -0500, Jeff Garzik wrote:
> 
> 
>>Standard FC2 OS with IPv6 6to4 setup as described on 
>>http://linux.yyz.us/ipv6-fc2-howto.html
> 
> 
> I upgraded my problematic machine to FC3, which somehow broke 6to4, and I
> didn't bother to reenable it again because the local DNS server (dnscache)
> just drops AAAA queries on the floor anyway, leading to lots of timeouts
> when trying to resolve host names, etc.  Ever since 6to4 was turned off
> on that box I stopped seeing these messages.

hmmm, that's annoying.  My router is still FC2, but I'll check that out 
when I upgrade it to FC3.

	Jeff

Re: IPv6 badness continues

[Lennert Buytenhek]

On Tue, Jan 04, 2005 at 10:57:43PM +0200, Pekka Savola wrote:

> >I upgraded my problematic machine to FC3, which somehow broke 6to4, and I
> >didn't bother to reenable it again because the local DNS server (dnscache)
> >just drops AAAA queries on the floor anyway, leading to lots of timeouts
> >when trying to resolve host names, etc.  Ever since 6to4 was turned off
> >on that box I stopped seeing these messages.
> 
> Surely it doesn't just drop AAAA queries on the floor, but sends an 
> error message?

Sorry, I just tried again and can't reproduce this behavior -- when doing
an AAAA query on a random domain name I get a clean 0/0/0 answer indicating
no RRs of the desired type having been found, and doing an AAAA query on a
domain name that does have RRs of that type, I do get the desired records.

I'm trying to recall what specific problem it was that I was seeing..


--L

[RFC PATCH] Fix double dereference (Re: IPv6 badness continues)

[YOSHIFUJI Hideaki / 吉藤英明]

Hello.

In article <41DA3A60.8050102@pobox.com> (at Tue, 04 Jan 2005 01:40:32 -0500), Jeff Garzik <jgarzik@pobox.com> says:

> The first 'Badness in dst_release' message (of many) from 2.6.10-bk1, on 
> x86 SMP, is found below.
:
> Badness in dst_release at include/net/dst.h:149
>   [<f8c14a57>] ip6_dst_check+0x67/0x80 [ipv6]
>   [<f8c0b2d4>] ip6_dst_lookup+0x1b4/0x1d0 [ipv6]
>   [<f8c1e4b4>] udpv6_sendmsg+0x2b4/0x950 [ipv6]

Ok, now I doubt sk_dst_check() code path.

When ip6_dst_check() (or its variants) returns NULL, it will release 
refcnt while it leaves sk->dst_cache non-NULL.
Later, caller sets it to NULL by sk_dst_reset() (or its variants), and it 
decrements refcnt again.

I think we should NOT release refcnt in sk_dst_check() (or its variants)
even we return NULL because that reference was held for sk->dst_cache
and it is available yet. Instead, we should release refcnt when we 
really reset the dst in sk_dst_reset() (or its variants).
(Alternatively, we may always set sk->dst_cache to NULL and release 
refcnt when we're about to return NULL in sk_dst_check()
(or its variants).)

Am I miss something?

===== net/decnet/dn_route.c 1.29 vs edited =====
--- 1.29/net/decnet/dn_route.c	2004-11-10 09:44:25 +09:00
+++ edited/net/decnet/dn_route.c	2005-01-04 17:14:01 +09:00
@@ -253,7 +253,6 @@
  */
 static struct dst_entry *dn_dst_check(struct dst_entry *dst, __u32 cookie)
 {
-	dst_release(dst);
 	return NULL;
 }
 
===== net/ipv4/route.c 1.99 vs edited =====
--- 1.99/net/ipv4/route.c	2004-12-28 12:49:57 +09:00
+++ edited/net/ipv4/route.c	2005-01-04 17:13:49 +09:00
@@ -1321,7 +1321,6 @@
 
 static struct dst_entry *ipv4_dst_check(struct dst_entry *dst, u32 cookie)
 {
-	dst_release(dst);
 	return NULL;
 }
 
===== net/ipv6/route.c 1.102 vs edited =====
--- 1.102/net/ipv6/route.c	2004-11-30 12:24:46 +09:00
+++ edited/net/ipv6/route.c	2005-01-04 17:13:34 +09:00
@@ -578,8 +578,6 @@
 
 	if (rt && rt->rt6i_node && (rt->rt6i_node->fn_sernum == cookie))
 		return dst;
-
-	dst_release(dst);
 	return NULL;
 }
 
===== net/xfrm/xfrm_policy.c 1.61 vs edited =====
--- 1.61/net/xfrm/xfrm_policy.c	2004-12-28 11:33:32 +09:00
+++ edited/net/xfrm/xfrm_policy.c	2005-01-04 17:13:12 +09:00
@@ -1000,8 +1000,6 @@
 {
 	if (!stale_bundle(dst))
 		return dst;
-
-	dst_release(dst);
 	return NULL;
 }
 

-- 
Hideaki YOSHIFUJI @ USAGI Project <yoshfuji@linux-ipv6.org>
GPG FP: 9022 65EB 1ECF 3AD1 0BDF  80D8 4807 F894 E062 0EEA

Re: [RFC PATCH] Fix double dereference (Re: IPv6 badness continues)

[David S. Miller]

On Fri, 07 Jan 2005 11:18:06 +0900 (JST)
YOSHIFUJI Hideaki / ^[$B5HF#1QL@^[(B <yoshfuji@linux-ipv6.org> wrote:

> Am I miss something?

Does every path which destroys an ipv6 socket do the
sk_dst_reset()?  If so, your patch should be valid.

Re: [RFC PATCH] Fix double dereference

[YOSHIFUJI Hideaki / 吉藤英明]

In article <20050106195333.2a90f6cb.davem@davemloft.net> (at Thu, 6 Jan 2005 19:53:33 -0800), "David S. Miller" <davem@davemloft.net> says:

> On Fri, 07 Jan 2005 11:18:06 +0900 (JST)
> YOSHIFUJI Hideaki / ^[$B5HF#1QL@^[(B <yoshfuji@linux-ipv6.org> wrote:
> 
> > Am I miss something?
> 
> Does every path which destroys an ipv6 socket do the
> sk_dst_reset()?  If so, your patch should be valid.

Currently, inet6_destroy_sock() in net/ipv6/af_inet6.c, which is
called from udpv6_destory_sock() etc., takes care of this.
In that means, yes.

However, one for ipv4 doesn't do this.

I'll come up with new changesets.

Thanks.

--yoshfuji

Re: [RFC PATCH] Fix double dereference (Re: IPv6 badness continues)

[Herbert Xu]

On Fri, Jan 07, 2005 at 11:18:06AM +0900, YOSHIFUJI Hideaki / ?$B5HF#1QL@ wrote:
> 
> I think we should NOT release refcnt in sk_dst_check() (or its variants)
> even we return NULL because that reference was held for sk->dst_cache
> and it is available yet. Instead, we should release refcnt when we 
> really reset the dst in sk_dst_reset() (or its variants).
> (Alternatively, we may always set sk->dst_cache to NULL and release 
> refcnt when we're about to return NULL in sk_dst_check()
> (or its variants).)

I think the current approach is safe.  The reason is that sk_dst_cache
has its own reference which gets dropped in __sk_dst_reset.  So even
though ip6_dst_check has dropped a refcnt, it should not hit zero unless
there is a bug somewhere else.

> ===== net/decnet/dn_route.c 1.29 vs edited =====
> --- 1.29/net/decnet/dn_route.c	2004-11-10 09:44:25 +09:00
> +++ edited/net/decnet/dn_route.c	2005-01-04 17:14:01 +09:00
> @@ -253,7 +253,6 @@
>   */
>  static struct dst_entry *dn_dst_check(struct dst_entry *dst, __u32 cookie)
>  {
> -	dst_release(dst);
>  	return NULL;
>  }

This patch is not going to work unless you drop the refcnt somewhere
else.  This refcnt is the one given by the caller of sk_dst_check.
So someone needs to drop it.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: IPv6 badness continues

[Herbert Xu]

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

On Tue, Jan 04, 2005 at 01:40:32AM -0500, Jeff Garzik wrote:
> 
> This problem seems to be appearing much sooner, in 2.6.10 and 2.6.10-bk 
> kernels.  Herbert, your patch did not apply to recent kernels, so I did 
> not boot with it.

Here is that patch rediffed against the current bk.  BTW, I'd like
to access the machine with the problem.  However, in order for it
to be useful I also need access to the build directory of your kernel
so that I can match things up.  Please mail me privately to set it up.

Thanks,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[-- Attachment #2: p --]
[-- Type: text/plain, Size: 335 bytes --]

===== net/ipv6/route.c 1.102 vs edited =====
--- 1.102/net/ipv6/route.c	2004-11-30 14:24:46 +11:00
+++ edited/net/ipv6/route.c	2005-01-08 15:23:24 +11:00
@@ -379,6 +379,7 @@
 	int err;
 
 	write_lock_bh(&rt6_lock);
+	WARN_ON(rt->u.dst->obsolete);
 	err = fib6_add(&ip6_routing_table, rt, nlh, _rtattr);
 	write_unlock_bh(&rt6_lock);
 

Re: IPv6 badness continues

[Jeff Garzik]

Herbert Xu wrote:
> ===== net/ipv6/route.c 1.102 vs edited =====
> --- 1.102/net/ipv6/route.c	2004-11-30 14:24:46 +11:00
> +++ edited/net/ipv6/route.c	2005-01-08 15:23:24 +11:00
> @@ -379,6 +379,7 @@
>  	int err;
>  
>  	write_lock_bh(&rt6_lock);
> +	WARN_ON(rt->u.dst->obsolete);
>  	err = fib6_add(&ip6_routing_table, rt, nlh, _rtattr);
>  	write_unlock_bh(&rt6_lock);

still broken:

net/ipv6/route.c: In function `ip6_ins_rt':
net/ipv6/route.c:382: error: invalid type argument of `->'

Re: IPv6 badness continues

[Herbert Xu]

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

On Sat, Jan 08, 2005 at 12:24:22PM -0500, Jeff Garzik wrote:
> Herbert Xu wrote:
> >===== net/ipv6/route.c 1.102 vs edited =====
> >--- 1.102/net/ipv6/route.c	2004-11-30 14:24:46 +11:00
> >+++ edited/net/ipv6/route.c	2005-01-08 15:23:24 +11:00
> >@@ -379,6 +379,7 @@
> > 	int err;
> > 
> > 	write_lock_bh(&rt6_lock);
> >+	WARN_ON(rt->u.dst->obsolete);
> > 	err = fib6_add(&ip6_routing_table, rt, nlh, _rtattr);
> > 	write_unlock_bh(&rt6_lock);
> 
> still broken:
> 
> net/ipv6/route.c: In function `ip6_ins_rt':
> net/ipv6/route.c:382: error: invalid type argument of `->'

Sorry, I thought you meant that it didn't apply :)

Here is the corrected version.
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[-- Attachment #2: p --]
[-- Type: text/plain, Size: 334 bytes --]

===== net/ipv6/route.c 1.102 vs edited =====
--- 1.102/net/ipv6/route.c	2004-11-30 14:24:46 +11:00
+++ edited/net/ipv6/route.c	2005-01-09 05:34:03 +11:00
@@ -379,6 +379,7 @@
 	int err;
 
 	write_lock_bh(&rt6_lock);
+	WARN_ON(rt->u.dst.obsolete);
 	err = fib6_add(&ip6_routing_table, rt, nlh, _rtattr);
 	write_unlock_bh(&rt6_lock);
 

Re: IPv6 badness continues

[Jeff Garzik]

Herbert Xu wrote:
> Here is the corrected version.

I applied the patch and tried it out.  The kernel was practically 
unusable, as the warning hit as soon as the kernel booted:

> Jan  8 20:06:44 pretzel kernel: Badness in ip6_ins_rt at net/ipv6/route.c:382
> Jan  8 20:06:44 pretzel kernel:  [<f8bee2be>] ip6_ins_rt+0x7e/0x80 [ipv6]
> Jan  8 20:06:44 pretzel kernel:  [<f8bec43e>] __ipv6_ifa_notify+0x12e/0x1a0 [ipv
> 6]
> Jan  8 20:06:44 pretzel kernel:  [<f8bec4e3>] ipv6_ifa_notify+0x33/0x50 [ipv6]
> Jan  8 20:06:44 pretzel kernel:  [<f8be9a10>] init_loopback+0x90/0x110 [ipv6]
> Jan  8 20:06:44 pretzel kernel:  [<f8be9ebf>] addrconf_notify+0xff/0x1a0 [ipv6]
> Jan  8 20:06:44 pretzel kernel:  [<c012fc0d>] notifier_call_chain+0x2d/0x50
> Jan  8 20:06:44 pretzel kernel:  [<c02fc7e7>] dev_open+0x77/0xa0
> Jan  8 20:06:44 pretzel kernel:  [<c02fde43>] dev_change_flags+0x53/0x130
> Jan  8 20:06:44 pretzel kernel:  [<c0342137>] devinet_ioctl+0x257/0x5d0
> Jan  8 20:06:44 pretzel kernel:  [<c03444c6>] inet_ioctl+0x66/0xb0
> Jan  8 20:06:44 pretzel kernel:  [<c02f34c9>] sock_ioctl+0xc9/0x250
> Jan  8 20:06:44 pretzel kernel:  [<c017404a>] sys_ioctl+0xca/0x230
> Jan  8 20:06:44 pretzel kernel:  [<c010309d>] sysenter_past_esp+0x52/0x75

Re: IPv6 badness continues

[Herbert Xu]

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

On Sat, Jan 08, 2005 at 08:12:00PM -0500, Jeff Garzik wrote:
> 
> I applied the patch and tried it out.  The kernel was practically 
> unusable, as the warning hit as soon as the kernel booted:

Sorry.  Should've tested it first.  Please try this one instead.
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[-- Attachment #2: p --]
[-- Type: text/plain, Size: 338 bytes --]

===== net/ipv6/route.c 1.102 vs edited =====
--- 1.102/net/ipv6/route.c	2004-11-30 14:24:46 +11:00
+++ edited/net/ipv6/route.c	2005-01-09 21:28:05 +11:00
@@ -379,6 +379,7 @@
 	int err;
 
 	write_lock_bh(&rt6_lock);
+	WARN_ON(rt->u.dst.obsolete > 0);
 	err = fib6_add(&ip6_routing_table, rt, nlh, _rtattr);
 	write_unlock_bh(&rt6_lock);