From patchwork Mon Mar 16 13:40:15 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sergey Popovich X-Patchwork-Id: 450573 X-Patchwork-Delegate: kadlec@blackhole.kfki.hu Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id C962E1400B7 for ; Tue, 17 Mar 2015 00:51:42 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=fail reason="verification failed; unprotected key" header.d=mail.ua header.i=@mail.ua header.b=ObpVAeyo; dkim-adsp=fail (unprotected policy); dkim-atps=neutral Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753062AbbCPNvj (ORCPT ); Mon, 16 Mar 2015 09:51:39 -0400 Received: from fallback3.mail.ru ([94.100.181.189]:33322 "EHLO fallback3.mail.ru" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753442AbbCPNvi (ORCPT ); Mon, 16 Mar 2015 09:51:38 -0400 Received: from smtp32.i.mail.ru (smtp32.i.mail.ru [94.100.177.92]) by fallback3.mail.ru (mPOP.Fallback_MX) with ESMTP id 7C4A4146EF142 for ; Mon, 16 Mar 2015 16:40:22 +0300 (MSK) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=mail.ua; s=mail; h=References:In-Reply-To:Message-Id:Date:Subject:To:From; bh=7aUxTO9he0zJ2Iygd2wNAcK8YZj9O7KhePGV1NYxAW4=; b=ObpVAeyodkt+Qv9jkLVLPpIy8a06IU0hthPoZf5Y1dATQwMsISMGkzePqgZY756OWtsxwMgfczXuY2jwml43cPwqlXAB2Ge+ctKM+zH6NF/KopTQRtjR+2y5kWfcvofxbNRRIVE2353RLvraHbeZW8ycFSaAo3+patRQOlYRrEo=; Received: from [2a01:6d80::195:20:96:53] (port=57337 helo=tuxracer.localdomain) by smtp32.i.mail.ru with esmtpa (envelope-from ) id 1YXVFk-0006MQ-RX; Mon, 16 Mar 2015 16:40:21 +0300 From: Sergey Popovich To: netfilter-devel@vger.kernel.org, popovich_sergei@mail.ru Subject: netfilter: ipset: Fix double free of comment string when adding element Date: Mon, 16 Mar 2015 15:40:15 +0200 Message-Id: X-Mailer: git-send-email 1.7.10.4 In-Reply-To: References: X-Spam: Not detected X-Mras: Ok Sender: netfilter-devel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netfilter-devel@vger.kernel.org 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 string. 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: #!/bin/bash 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=( [cmd_add_index]="add" [$cmd_del_index]="del" ) 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" || comment= printf "$fmt" "${cmd_a[$cmd_index]}" $o4 "$comment" done 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 --- net/netfilter/ipset/ip_set_hash_gen.h | 1 + 1 file changed, 1 insertion(+) 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); #ifdef IP_SET_HASH_WITH_NETS for (i = 0; i < IPSET_NET_COUNT; i++) mtype_add_cidr(h, SCIDR(d->cidr, i), NLEN(set->family),