Netlink BUG() on AMD64

Netlink BUG() on AMD64

[Jan Kasprzak]

	Hello, world!\n

I have got kernel BUG() while running the "ip rule list" command
on my dual AMD64 box with 2.6.2 kernel. I have a blacklist of IP
addresses, and I have one IP rule for each of this addresses:

ip rule add pref 500 from x.y.z.a dev $UPLINK_DEV blackhole

I have about 200 such rules (with different x.y.z.a IPv4 addresses,
but all with the same preference of 500 and same $UPLINK_DEV - currently
eth3). I have measured that when I add less than 60 such rules, I do not
get BUG() during "ip rule list" command. When I add 60 or more,
I get overflow in skb_put(). So the kernel is definitely overflowing
something.

The kernel messages:

skput:over: ffffffff802bb833:3804 put:-36 dev:<NULL>----------- [cut here ] --------- [please bite here ] ---------
Kernel BUG at skbuff:88
invalid operand: 0000 [1]
CPU 0
Pid: 2778, comm: ip Not tainted
RIP: 0010:[<ffffffff80270f72>] <ffffffff80270f72>{skb_over_panic+50}
RSP: 0018:000001003e187a18  EFLAGS: 00010216
RAX: 0000000000000037 RBX: 000001003e92b580 RCX: 0000000000000001
RDX: 0000000000000002 RSI: 000001003ff8abf0 RDI: 0000000000000001
RBP: 00000100417a49c0 R08: 0000000000000001 R09: 0000000000000000
R10: 00000000ffffffff R11: 0000000000000000 R12: 000001003e308edc
R13: 00000000402280ce R14: 0000000000000ada R15: 000000000000003c
FS:  0000002a9555cc60(0000) GS:ffffffff80416d40(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
CR2: 000000349ca96390 CR3: 0000000000101000 CR4: 00000000000006a0
Process ip (pid: 2778, stackpage=1003f01d140)
Stack: 0000000000000ada ffffffff802bb83b 0000000000000246 0000000080271022
       000001003e168bc0 000001003e92b580 ffffffff802bb580 000001003ffaeb40
       000001003e168bc0 000001003e168b40
Call Trace:<ffffffff802bb83b>{inet_dump_rules+699} <ffffffff802bb580>{inet_dump_rules+0}
       <ffffffff80282e37>{netlink_dump+135} <ffffffff802bb580>{inet_dump_rules+0}
       <ffffffff8027dbf0>{rtnetlink_done+0} <ffffffff802830e9>{netlink_dump_start+281}
       <ffffffff8027dd50>{rtnetlink_rcv+336} <ffffffff8016af75>{bh_lru_install+229}
       <ffffffff801af379>{__journal_file_buffer+377} <ffffffff80282c86>{netlink_data_ready+22}
       <ffffffff802823e9>{netlink_unicast+793} <ffffffff8012dc20>{default_wake_function+0}
       <ffffffff8012dc20>{default_wake_function+0} <ffffffff80282a4c>{netlink_sendmsg+684}
       <ffffffff8026d8d5>{sock_sendmsg+133} <ffffffff8014f111>{__alloc_pages+161}
       <ffffffff8014b6a7>{find_get_page+23} <ffffffff8014c5c9>{filemap_nopage+345}
       <ffffffff8015a76d>{do_no_page+925} <ffffffff8026d690>{sockfd_lookup+32}
       <ffffffff8026d337>{move_addr_to_kernel+39} <ffffffff8026ebc3>{sys_sendto+195}
       <ffffffff8026ea07>{sys_getsockname+135} <ffffffff8026d651>{sock_map_fd+353}
       <ffffffff80281be0>{netlink_create+160} <ffffffff8010ec04>{system_call+124}
        
 
Code: 0f 0b ff ed 34 80 ff ff ff ff 58 00 48 83 c4 08 c3 66 66 66
RIP <ffffffff80270f72>{skb_over_panic+50} RSP <000001003e187a18>
  
-Yenya

-- 
| Jan "Yenya" Kasprzak  <kas at {fi.muni.cz - work | yenya.net - private}> |
| GPG: ID 1024/D3498839      Fingerprint 0D99A7FB206605D7 8B35FCDE05B18A5E |
| http://www.fi.muni.cz/~kas/   Czech Linux Homepage: http://www.linux.cz/ |
 Any compiler or language that likes to hide things like memory allocations
 behind your back just isn't a good choice for a kernel.   --Linus Torvalds

[Patch] Netlink BUG() on AMD64

[Jan Kasprzak]

Jan Kasprzak wrote:
: I have got kernel BUG() while running the "ip rule list" command
: on my dual AMD64 box with 2.6.2 kernel. I have a blacklist of IP
: addresses, and I have one IP rule for each of this addresses:
: 
: ip rule add pref 500 from x.y.z.a dev $UPLINK_DEV blackhole
: 
: I have about 200 such rules (with different x.y.z.a IPv4 addresses,
: but all with the same preference of 500 and same $UPLINK_DEV - currently
: eth3). I have measured that when I add less than 60 such rules, I do not
: get BUG() during "ip rule list" command. When I add 60 or more,
: I get overflow in skb_put(). So the kernel is definitely overflowing
: something.

	The problem is that in fib_rules.c there is an attempt to
skb_put() a negative increment. Which is OK on platforms where sizeof(unsigned)
== sizeof(void *). skb_put() has its second argument as unsigned int,
so instead of adding a -36 bytes here, it adds 4294967260 bytes to the
skb->tail, which extends it further than skb->end, which causes the
BUG() I have mentioned. The solution would be to change the argument
of skb_put() and friends to be either long or signed int, or to call
skb_trim() instead of skb_put in fib_rules.c instead.

	I suggest the following patch, but all occurences of
nlmsg_failure: and rtattr_failure: labels should be checked for a similar
problem.

--- linux-2.6.2/net/ipv4/fib_rules.c.orig	2004-02-11 18:55:58.000000000 +0100
+++ linux-2.6.2/net/ipv4/fib_rules.c	2004-02-11 19:03:08.319215408 +0100
@@ -438,7 +438,7 @@
 
 nlmsg_failure:
 rtattr_failure:
-	skb_put(skb, b - skb->tail);
+	skb_trim(skb, b - skb->data);
 	return -1;
 }
 
Please apply or let me know what the proper fix should be.

-Yenya

-- 
| Jan "Yenya" Kasprzak  <kas at {fi.muni.cz - work | yenya.net - private}> |
| GPG: ID 1024/D3498839      Fingerprint 0D99A7FB206605D7 8B35FCDE05B18A5E |
| http://www.fi.muni.cz/~kas/   Czech Linux Homepage: http://www.linux.cz/ |
 Any compiler or language that likes to hide things like memory allocations
 behind your back just isn't a good choice for a kernel.   --Linus Torvalds

Re: [Patch] Netlink BUG() on AMD64

[YOSHIFUJI Hideaki / 吉藤英明]

In article <20040211181113.GA2849@fi.muni.cz> (at Wed, 11 Feb 2004 19:11:14 +0100), Jan Kasprzak <kas@informatics.muni.cz> says:

> 	I suggest the following patch, but all occurences of
> nlmsg_failure: and rtattr_failure: labels should be checked for a similar
> problem.
> 
> --- linux-2.6.2/net/ipv4/fib_rules.c.orig	2004-02-11 18:55:58.000000000 +0100
> +++ linux-2.6.2/net/ipv4/fib_rules.c	2004-02-11 19:03:08.319215408 +0100
> @@ -438,7 +438,7 @@
>  
>  nlmsg_failure:
>  rtattr_failure:
> -	skb_put(skb, b - skb->tail);
> +	skb_trim(skb, b - skb->data);
>  	return -1;
>  }
>  
> Please apply or let me know what the proper fix should be.

looks good to me.
Other places including net/ipv6/{addrconf.c,route.c} seems okay.

--yoshfuji

Re: [Patch] Netlink BUG() on AMD64

[YOSHIFUJI Hideaki / 吉藤英明]

In article <20040212.034537.11291491.yoshfuji@linux-ipv6.org> (at Thu, 12 Feb 2004 03:45:37 +0900 (JST)), YOSHIFUJI Hideaki / 吉藤英明 <yoshfuji@linux-ipv6.org> says:

> In article <20040211181113.GA2849@fi.muni.cz> (at Wed, 11 Feb 2004 19:11:14 +0100), Jan Kasprzak <kas@informatics.muni.cz> says:
> 
> > 	I suggest the following patch, but all occurences of
> > nlmsg_failure: and rtattr_failure: labels should be checked for a similar
> > problem.
> > 
> > --- linux-2.6.2/net/ipv4/fib_rules.c.orig	2004-02-11 18:55:58.000000000 +0100
> > +++ linux-2.6.2/net/ipv4/fib_rules.c	2004-02-11 19:03:08.319215408 +0100
> > @@ -438,7 +438,7 @@
> >  
> >  nlmsg_failure:
> >  rtattr_failure:
> > -	skb_put(skb, b - skb->tail);
> > +	skb_trim(skb, b - skb->data);
> >  	return -1;
> >  }
> >  
> > Please apply or let me know what the proper fix should be.
> 
> looks good to me.
> Other places including net/ipv6/{addrconf.c,route.c} seems okay.

Oops, I'd looked into ipv4 and ipv6 only. 
I've grep'ed and found one in net/decnet.

===== net/decnet/dn_rules.c 1.6 vs edited =====
--- 1.6/net/decnet/dn_rules.c	Fri May  9 01:46:11 2003
+++ edited/net/decnet/dn_rules.c	Thu Feb 12 03:52:42 2004
@@ -381,7 +381,7 @@
 
 nlmsg_failure:
 rtattr_failure:
-	skb_put(skb, b - skb->tail);
+	skb_trim(skb, b - skb->data);
 	return -1;
 }
 

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

Re: [Patch] Netlink BUG() on AMD64

[David S. Miller]

On Thu, 12 Feb 2004 03:58:25 +0900 (JST)
YOSHIFUJI Hideaki / ^[$B5HF#1QL@^[(B <yoshfuji@linux-ipv6.org> wrote:

> In article <20040212.034537.11291491.yoshfuji@linux-ipv6.org> (at Thu, 12 Feb 2004 03:45:37 +0900 (JST)), YOSHIFUJI Hideaki / ^[$B5HF#1QL@^[(B <yoshfuji@linux-ipv6.org> says:
> 
> > In article <20040211181113.GA2849@fi.muni.cz> (at Wed, 11 Feb 2004 19:11:14 +0100), Jan Kasprzak <kas@informatics.muni.cz> says:
> > 
> > > --- linux-2.6.2/net/ipv4/fib_rules.c.orig	2004-02-11 18:55:58.000000000 +0100
> > > +++ linux-2.6.2/net/ipv4/fib_rules.c	2004-02-11 19:03:08.319215408 +0100
> > > @@ -438,7 +438,7 @@
 ...
> > > -	skb_put(skb, b - skb->tail);
> > > +	skb_trim(skb, b - skb->data);
> --- 1.6/net/decnet/dn_rules.c	Fri May  9 01:46:11 2003
> +++ edited/net/decnet/dn_rules.c	Thu Feb 12 03:52:42 2004
> @@ -381,7 +381,7 @@
 ...
> -	skb_put(skb, b - skb->tail);
> +	skb_trim(skb, b - skb->data);

Both fixes applied, thanks guys.

I was tempted to make skb_put()'s second argument signed, but I'm in no mood
to audit the entire tree for that :-)

Re: [Patch] Netlink BUG() on AMD64

[Jan Kasprzak]

David S. Miller wrote:
: Both fixes applied, thanks guys.
: 
: I was tempted to make skb_put()'s second argument signed, but I'm in no mood
: to audit the entire tree for that :-)

	I have grep'd the kernel for all occurences of rtattr_failure
and nlmsg_failure, and there are no more skb_put()s than those two
(hmm, legacy protocols which no-one uses, such as Decnet or IPv4 :-).
However, the same problem is in 2.4, so please push these fixes to
Marcelo as well.

-Yenya

-- 
| Jan "Yenya" Kasprzak  <kas at {fi.muni.cz - work | yenya.net - private}> |
| GPG: ID 1024/D3498839      Fingerprint 0D99A7FB206605D7 8B35FCDE05B18A5E |
| http://www.fi.muni.cz/~kas/   Czech Linux Homepage: http://www.linux.cz/ |
 Any compiler or language that likes to hide things like memory allocations
 behind your back just isn't a good choice for a kernel.   --Linus Torvalds

Re: [Patch] Netlink BUG() on AMD64

[David S. Miller]

On Thu, 12 Feb 2004 05:10:10 +0100
Jan Kasprzak <kas@informatics.muni.cz> wrote:

> However, the same problem is in 2.4, so please push these fixes to
> Marcelo as well.

I know, I made the change in both trees.

But thanks for reminding me.

Re: [Patch] Netlink BUG() on AMD64

[kuznet]

Hello!

> I was tempted to make skb_put()'s second argument signed, but I'm in no mood
> to audit the entire tree for that :-)

No, no, it really was just a silly mistake, misprint most likely,
skb_put() was expected to eat only positive arguments.

Alexey