From d05e7e9349bd1a0b575f7c92588804510da612c7 Mon Sep 17 00:00:00 2001 From: Jozsef Kadlecsik Date: Tue, 25 Aug 2015 11:11:57 +0200 Subject: Out of bound access in hash:net* types fixed Dave Jones reported that KASan detected out of bounds access in hash:net* types: [ 23.139532] ================================================================== [ 23.146130] BUG: KASan: out of bounds access in hash_net4_add_cidr+0x1db/0x220 at addr ffff8800d4844b58 [ 23.152937] Write of size 4 by task ipset/457 [ 23.159742] ============================================================================= [ 23.166672] BUG kmalloc-512 (Not tainted): kasan: bad access detected [ 23.173641] ----------------------------------------------------------------------------- [ 23.194668] INFO: Allocated in hash_net_create+0x16a/0x470 age=7 cpu=1 pid=456 [ 23.201836] __slab_alloc.constprop.66+0x554/0x620 [ 23.208994] __kmalloc+0x2f2/0x360 [ 23.216105] hash_net_create+0x16a/0x470 [ 23.223238] ip_set_create+0x3e6/0x740 [ 23.230343] nfnetlink_rcv_msg+0x599/0x640 [ 23.237454] netlink_rcv_skb+0x14f/0x190 [ 23.244533] nfnetlink_rcv+0x3f6/0x790 [ 23.251579] netlink_unicast+0x272/0x390 [ 23.258573] netlink_sendmsg+0x5a1/0xa50 [ 23.265485] SYSC_sendto+0x1da/0x2c0 [ 23.272364] SyS_sendto+0xe/0x10 [ 23.279168] entry_SYSCALL_64_fastpath+0x12/0x6f The bug is fixed in the patch and the testsuite is extended in ipset to check cidr handling more thoroughly. --- kernel/net/netfilter/ipset/ip_set_hash_gen.h | 12 ++- tests/cidr.sh | 112 +++++++++++++++++++++++++++ tests/hash:net,iface.t | 2 + tests/hash:net,port.t | 2 + tests/hash:net.t | 2 + tests/iptables.sh | 2 +- 6 files changed, 127 insertions(+), 5 deletions(-) create mode 100755 tests/cidr.sh diff --git a/kernel/net/netfilter/ipset/ip_set_hash_gen.h b/kernel/net/netfilter/ipset/ip_set_hash_gen.h index 8244d17..2949645 100644 --- a/kernel/net/netfilter/ipset/ip_set_hash_gen.h +++ b/kernel/net/netfilter/ipset/ip_set_hash_gen.h @@ -152,9 +152,13 @@ htable_bits(u32 hashsize) DCIDR_PUT(((cidr) ? NCIDR_GET(cidr) : host_mask)) #ifdef IP_SET_HASH_WITH_NET0 +/* cidr from 0 to HOST_MASK value and c = cidr + 1 */ #define NLEN (HOST_MASK + 1) +#define CIDR_POS(c) ((c) - 1) #else +/* cidr from 1 to HOST_MASK value and c = cidr + 1 */ #define NLEN HOST_MASK +#define CIDR_POS(c) ((c) - 2) #endif #else @@ -308,7 +312,7 @@ mtype_add_cidr(struct htype *h, u8 cidr, u8 n) } else if (h->nets[i].cidr[n] < cidr) { j = i; } else if (h->nets[i].cidr[n] == cidr) { - h->nets[cidr - 1].nets[n]++; + h->nets[CIDR_POS(cidr)].nets[n]++; return; } } @@ -317,7 +321,7 @@ mtype_add_cidr(struct htype *h, u8 cidr, u8 n) h->nets[i].cidr[n] = h->nets[i - 1].cidr[n]; } h->nets[i].cidr[n] = cidr; - h->nets[cidr - 1].nets[n] = 1; + h->nets[CIDR_POS(cidr)].nets[n] = 1; } static void @@ -328,8 +332,8 @@ mtype_del_cidr(struct htype *h, u8 cidr, u8 n) for (i = 0; i < NLEN; i++) { if (h->nets[i].cidr[n] != cidr) continue; - h->nets[cidr - 1].nets[n]--; - if (h->nets[cidr - 1].nets[n] > 0) + h->nets[CIDR_POS(cidr)].nets[n]--; + if (h->nets[CIDR_POS(cidr)].nets[n] > 0) return; for (j = i; j < net_end && h->nets[j].cidr[n]; j++) h->nets[j].cidr[n] = h->nets[j + 1].cidr[n]; diff --git a/tests/cidr.sh b/tests/cidr.sh new file mode 100755 index 0000000..b7d695a --- /dev/null +++ b/tests/cidr.sh @@ -0,0 +1,112 @@ +#!/bin/bash + +set -e + +NETS="0.0.0.0/1 +128.0.0.0/2 +192.0.0.0/3 +224.0.0.0/4 +240.0.0.0/5 +248.0.0.0/6 +252.0.0.0/7 +254.0.0.0/8 +255.0.0.0/9 +255.128.0.0/10 +255.192.0.0/11 +255.224.0.0/12 +255.240.0.0/13 +255.248.0.0/14 +255.252.0.0/15 +255.254.0.0/16 +255.255.0.0/17 +255.255.128.0/18 +255.255.192.0/19 +255.255.224.0/20 +255.255.240.0/21 +255.255.248.0/22 +255.255.252.0/23 +255.255.254.0/24 +255.255.255.0/25 +255.255.255.128/26 +255.255.255.192/27 +255.255.255.224/28 +255.255.255.240/29 +255.255.255.248/30 +255.255.255.252/31 +255.255.255.254/32" + +ipset="../src/ipset" + +case "$1" in +net) + $ipset n test hash:net + + while IFS= read x; do + $ipset add test $x + done <<<"$NETS" + + while IFS= read x; do + first=`netmask -r $x | cut -d - -f 1` + $ipset test test $first >/dev/null 2>&1 + last=`netmask -r $x | cut -d - -f 2 | cut -d ' ' -f 1` + $ipset test test $last >/dev/null 2>&1 + done <<<"$NETS" + + while IFS= read x; do + $ipset del test $x + done <<<"$NETS" + ;; +net,port) + $ipset n test hash:net,port + + n=1 + while IFS= read x; do + $ipset add test $x,$n + n=$((n+1)) + done <<<"$NETS" + + n=1 + while IFS= read x; do + first=`netmask -r $x | cut -d - -f 1` + $ipset test test $first,$n >/dev/null 2>&1 + last=`netmask -r $x | cut -d - -f 2 | cut -d ' ' -f 1` + $ipset test test $last,$n >/dev/null 2>&1 + n=$((n+1)) + done <<<"$NETS" + + n=1 + while IFS= read x; do + $ipset del test $x,$n + n=$((n+1)) + done <<<"$NETS" + ;; +net,iface) + $ipset n test hash:net,iface + + $ipset add test 0.0.0.0/0,eth0 + n=1 + while IFS= read x; do + $ipset add test $x,eth$n + n=$((n+1)) + done <<<"$NETS" + + $ipset test test 0.0.0.0/0,eth0 + n=1 + while IFS= read x; do + $ipset test test $x,eth$n >/dev/null 2>&1 + n=$((n+1)) + done <<<"$NETS" + + $ipset del test 0.0.0.0/0,eth0 + n=1 + while IFS= read x; do + $ipset del test $x,eth$n + n=$((n+1)) + done <<<"$NETS" + ;; +*) + echo "Usage: $0 net|net,port|net,iface" + exit 1 + ;; +esac +$ipset x test diff --git a/tests/hash:net,iface.t b/tests/hash:net,iface.t index c19de2b..a847357 100644 --- a/tests/hash:net,iface.t +++ b/tests/hash:net,iface.t @@ -134,6 +134,8 @@ 0 n=`ipset list test | grep -v Revision: | wc -l` && test $n -eq 71 # Delete test set 0 ipset destroy test +# Check all possible CIDR values +0 ./cidr.sh net,iface # Create test set with timeout support 0 ipset create test hash:net,iface timeout 30 # Add a non-matching IP address entry diff --git a/tests/hash:net,port.t b/tests/hash:net,port.t index abe565f..d51d27f 100644 --- a/tests/hash:net,port.t +++ b/tests/hash:net,port.t @@ -114,6 +114,8 @@ 0 ipset -T test 1.1.1.3,80 # Delete test set 0 ipset destroy test +# Check all possible CIDR values +0 ./cidr.sh net,port # Timeout: Check that resizing keeps timeout values 0 ./resizet.sh -4 netport # Nomatch: Check that resizing keeps the nomatch flag diff --git a/tests/hash:net.t b/tests/hash:net.t index f43e7a4..6f54c25 100644 --- a/tests/hash:net.t +++ b/tests/hash:net.t @@ -114,6 +114,8 @@ 0 ipset destroy test # Check CIDR book-keeping 0 ./check_cidrs.sh +# Check all possible CIDR values +0 ./cidr.sh net # Timeout: Check that resizing keeps timeout values 0 ./resizet.sh -4 net # Nomatch: Check that resizing keeps the nomatch flag diff --git a/tests/iptables.sh b/tests/iptables.sh index 7273066..7ea90e0 100755 --- a/tests/iptables.sh +++ b/tests/iptables.sh @@ -1,6 +1,6 @@ #!/bin/sh -set -x +# set -x set -e ipset=${IPSET_BIN:-../src/ipset} -- cgit v1.2.3