Skip to contentSkip to conversation

Netlink BUG() on AMD64

PATCH New Jan Kasprzak 4 participants 9 messages Feb 5, 2004 – Feb 12, 2004

Subsystems (3)
Networking Network drivers IPv4/IPv6
lore
Share & tools
lore

Jan Kasprzak

author
#0 lore raw

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

Signature · 5 lines
| 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
author
#1 lore raw

PATCH 1/1 Netlink BUG() on AMD64

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.

net/ipv4/fib_rules.c renamed from net/ipv4/fib_rules.c.orig +1 -1
@@ -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

Signature · 5 lines
| 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
author
#2 lore raw

PATCH 1/1 Netlink BUG() on AMD64

In reply to 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.

net/ipv4/fib_rules.c renamed from net/ipv4/fib_rules.c.orig +1 -1
@@ -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

Signature · 5 lines
| 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
#3 lore raw

In article <[email protected]> (at Wed, 11 Feb 2004 19:11:14 +0100), Jan Kasprzak <[email protected]> says:

Quoted 15 lines

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

net/ipv4/fib_rules.c renamed from net/ipv4/fib_rules.c.orig +1 -1
@@ -438,7 +438,7 @@
 
 nlmsg_failure:
 rtattr_failure:
-	skb_put(skb, b - skb->tail);
+	skb_trim(skb, b - skb->data);
 	return -1;
 }
Quoted 1 line

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

#4 lore raw

In article <[email protected]> (at Thu, 12 Feb 2004 03:45:37 +0900 (JST)), YOSHIFUJI Hideaki / 吉藤英明 <[email protected]> says:

Quoted 17 lines

In article <[email protected]> (at Wed, 11 Feb 2004 19:11:14 +0100), Jan Kasprzak <[email protected]> says:

Quoted 15 lines

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

net/ipv4/fib_rules.c renamed from net/ipv4/fib_rules.c.orig +1 -1
@@ -438,7 +438,7 @@
 
 nlmsg_failure:
 rtattr_failure:
-	skb_put(skb, b - skb->tail);
+	skb_trim(skb, b - skb->data);
 	return -1;
 }
Quoted 4 lines
Quoted 1 line

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 =====

net/decnet/dn_rules.c +1 -1
@@ -381,7 +381,7 @@
 
 nlmsg_failure:
 rtattr_failure:
-	skb_put(skb, b - skb->tail);
+	skb_trim(skb, b - skb->data);
 	return -1;
 }
Signature · 2 lines
Hideaki YOSHIFUJI @ USAGI Project <[email protected]>
GPG FP: 9022 65EB 1ECF 3AD1 0BDF  80D8 4807 F894 E062 0EEA
#5 lore raw
Quoted 7 lines · On Thu, 12 Feb 2004 03:58:25 +0900 (JST) YOSHIFUJI Hideaki / $B5HF#1QL@(B…

In article <[email protected]> (at Thu, 12 Feb 2004 03:45:37 +0900 (JST)), YOSHIFUJI Hideaki / $B5HF#1QL@(B <[email protected]> says:

Quoted 5 lines

In article <[email protected]> (at Wed, 11 Feb 2004 19:11:14 +0100), Jan Kasprzak <[email protected]> says:

Quoted 3 lines
net/ipv4/fib_rules.c renamed from net/ipv4/fib_rules.c.orig +0 -0
@@ -438,7 +438,7 @@

...

Quoted 5 lines
Quoted 2 lines
Quoted 2 lines
net/ipv4/fib_rules.c renamed from net/ipv4/fib_rules.c.orig +1 -1
-	skb_put(skb, b - skb->tail);
+	skb_trim(skb, b - skb->data);
net/decnet/dn_rules.c +0 -0
@@ -381,7 +381,7 @@

...

net/decnet/dn_rules.c line 384

Quoted 2 lines
net/decnet/dn_rules.c +1 -1
-	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 :-)

author
#6 lore raw

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

Signature · 5 lines
| 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
#7 lore raw
Quoted 2 lines · On Thu, 12 Feb 2004 05:10:10 +0100 Jan Kasprzak <[email protected]> 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.

#8 lore raw
In reply to David S. Miller

Hello!

Quoted 2 lines

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