diff mbox

netfilter: ipset: Fix double free of comment string when adding element

Message ID c711808889c4462a07d4caa715c9fa62569e5510.1426509747.git.popovich_sergei@mail.ua
State Not Applicable
Delegated to: Jozsef Kadlecsik
Headers show

Commit Message

Sergey Popovich March 16, 2015, 1:40 p.m. UTC
When new element added hbucket_elem_add() is called to allocate
new ahash slot when number of elements (n->pos) in bucket reaches
current bucket size (n->size). New ahash slot area initialized
with zeros. In case there is room for element in the current
slot hbucket_elem_add() returns without initializing area
for the new element.

This is ok for the most cases, as we use ip_set_ext_destroy()
to free resources used by the extensions. Currently only comment
extension should be destroyed. Destroying comment also initializes
pointer to string to NULL, so that on slot reuse in mtype_add()
ip_set_init_comment() does not free already freed comment string.

However in case of call sequence mtype_del() -> mtype_add()
for set with comment extension enabled it might happen that
ahash_data() points to the dirty area, where comment extension
data area has it's pointer set to already freed area, causing
ip_set_init_comment() in mtype_add() to double free comment

This happens if mtype_del()/mtype_expire() deletes non-last
element from the slot in bucket (and ip_set_ext_destroy()
properly initializes comment string to NULL), then we use
memcpy() to copy last element in place of the deleted element
and finally decrement first free element position n->pos.

Later if new element added to the bucket it's data area is not
clear, and ip_set_init_comment() in mtype_add() will attempt
to free comment string used by an valid element previously
seen at last position or double free area if that element
already deleted.

In most cases this leads to oops as slab cache structures
gets corrupted.

Test case for this condition is following:

  1. Use current upstream ipset code.
  2. Compile kernel with CONFIG_DEBUG_SLAB=y
  3. Use following script to reproduce condition:


    ipset -exist create test-1 hash:net family inet comment \
           hashsize 65536 maxelem 65536

    declare -i cmd_add_index=0 cmd_del_index=1
    declare -a cmd_a=(
    declare -i cmd_index cmd_a_size=${#cmd_a[@]} o4

    fmt="%s test-1 10.0.10.%u%s\n"

    while :; do
      for ((o4 = 0; o4 < 256; o4++)); do
          cmd_index=$((RANDOM % cmd_a_size))
          [ $cmd_index -eq $cmd_add_index ] &&
              comment=" comment $RANDOM" ||
          printf "$fmt" "${cmd_a[$cmd_index]}" $o4 "$comment"
    done |ipset -exist restore

    ipset destroy test-1

An slab debug reports would be generated reporting memory
double free conditions and memory corruptions.

Fix by initializing memory area for new element in mtype_add().

Signed-off-by: Sergey Popovich <popovich_sergei@mail.ua>
 net/netfilter/ipset/ip_set_hash_gen.h |    1 +
 1 file changed, 1 insertion(+)
diff mbox


diff --git a/net/netfilter/ipset/ip_set_hash_gen.h b/net/netfilter/ipset/ip_set_hash_gen.h
index fcf75be..e886d18 100644
--- a/net/netfilter/ipset/ip_set_hash_gen.h
+++ b/net/netfilter/ipset/ip_set_hash_gen.h
@@ -695,6 +695,7 @@  reuse_slot:
 			goto out;
 		data = ahash_data(n, n->pos++, set->dsize);
+		memset(data, 0, set->dsize);
 		for (i = 0; i < IPSET_NET_COUNT; i++)
 			mtype_add_cidr(h, SCIDR(d->cidr, i), NLEN(set->family),