IPv6 oops on ifup in latest BK

Poo.  You guys are no fun.  I _just_ got my home network up on IPv6, and 
the latest 'bk pull' breaks it (IOW, a change in the latest 24 hours). 
Right after I post http://yyz.us/ipv6-fc2-howto.html too ;-)

Attached minicom.cap.txt gives the ksymoops output and dmesg output. 
Appears to die in ipv6_get_hoplimit.

	Jeff

On Tue, 24 Aug 2004 02:07:28 -0400
Jeff Garzik [off-list ref] wrote:

Attached minicom.cap.txt gives the ksymoops output and dmesg output. 
Appears to die in ipv6_get_hoplimit.

Yoshifuji-san, it is rt6i_dev changes.  The problem is that
ipv6_get_hoplimit() gets called with NULL dev.

I believe it is an error in the logic for RTCF_REJECT
processing.  If user does not specify a specific device
index, and this is RTCF_REJECT, then we will end up
with dev being NULL.

It is this piece of code in ip6_route_add():

		if (dev && dev != &loopback_dev) {

It does not handle the case where dev == NULL correctly.
Original code did do the right thing:

		if (dev)
			dev_put(dev);
		dev = &loopback_dev;
		dev_hold(dev);

Maybe new code should be something like:

		if (dev && dev != &loopback_dev) {
			dev_put(dev);
			in6_dev_put(idev);
		}
		dev = &loopback_dev;
		dev_hold(dev);
		idev = in6_dev_get(dev);
		if (!idev) {
			err = -ENODEV;
			goto out;
		}

What do you think?

David S. Miller [off-list ref] wrote:

Maybe new code should be something like:

               if (dev && dev != &loopback_dev) {

You want
		 if (dev) {
here.

                       dev_put(dev);
                       in6_dev_put(idev);
               }
               dev = &loopback_dev;
               dev_hold(dev);
               idev = in6_dev_get(dev);
               if (!idev) {
                       err = -ENODEV;
                       goto out;
               }

What do you think?

Yes this would work.  But I think Yoshifuji-san is trying to avoid the
unnecessary put/get in the case where dev is already loopback_dev.
So something like this might work:

		if (dev != &loopback_dev) {
			if (dev) {
				dev_put(dev);
				in6_dev_put(idev);
			}
			...
		}
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} [off-list ref]
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

In article [off-list ref] (at Mon, 23 Aug 2004 23:51:23 -0700), "David S. Miller" [off-list ref] says:

quoted

Attached minicom.cap.txt gives the ksymoops output and dmesg output. 
Appears to die in ipv6_get_hoplimit.

Yoshifuji-san, it is rt6i_dev changes.  The problem is that
ipv6_get_hoplimit() gets called with NULL dev.

:

It is this piece of code in ip6_route_add():

		if (dev && dev != &loopback_dev) {

It does not handle the case where dev == NULL correctly.
Original code did do the right thing:

		if (dev)
			dev_put(dev);
		dev = &loopback_dev;
		dev_hold(dev);

Good catch and spotting.  Please try this patch.
Thank you.

===== net/ipv6/route.c 1.88 vs edited =====

--- 1.88/net/ipv6/route.c	2004-08-17 11:25:06 +09:00
+++ edited/net/ipv6/route.c	2004-08-24 17:09:10 +09:00

@@ -820,9 +820,12 @@
 	 */
 	if ((rtmsg->rtmsg_flags&RTF_REJECT) ||
 	    (dev && (dev->flags&IFF_LOOPBACK) && !(addr_type&IPV6_ADDR_LOOPBACK))) {
-		if (dev && dev != &loopback_dev) {
-			dev_put(dev);
-			in6_dev_put(idev);
+		/* hold loopback dev/idev if we haven't done so. */
+		if (dev != &loopback_dev) {
+			if (dev) {
+				dev_put(dev);
+				in6_dev_put(idev);
+			}
 			dev = &loopback_dev;
 			dev_hold(dev);
 			idev = in6_dev_get(dev);

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

On Tue, 24 Aug 2004 17:22:07 +0900 (JST)
YOSHIFUJI Hideaki / $B5HF#1QL@(B [off-list ref] wrote:

Good catch and spotting.  Please try this patch.
Thank you.

Applied, thank you.