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.

diff --git a/kernel/net/netfilter/ipset/ip_set_hash_gen.h b/kernel/net/netfilter/ipset/ip_set_hash_gen.h
index 8244d172633c9652dba99b23ab8083578f06100f..2949645006e4a928c04ba84f9905fe9e4e449461 100644 (file)
--- 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 (executable)
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 c19de2b5d43e6cd4a22f66427b4da2bfb0940a7d..a847357a89386427bc23026c6678cd015baf8021 100644 (file)
--- 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 abe565f8752b03899dc22ac286adb9b60bb55859..d51d27f8c505b1ca78349db483a732ddce8c9808 100644 (file)
--- 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 f43e7a4c2ce16b5eff984540920f8fc843ce5758..6f54c256441b5c0fe4e3ddc355ffc0a694156b56 100644 (file)
--- 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 7273066103f6e225655a27fa01b33c417bf7d64c..7ea90e025b6f539963348a7d32e78a2343e0fb97 100755 (executable)
--- 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}