From patchwork Tue Oct 9 19:29:06 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Fu, Qiaobin" X-Patchwork-Id: 46412 X-Patchwork-Delegate: thomas@monjalon.net Return-Path: X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 8A5E01B3DA; Tue, 9 Oct 2018 21:30:22 +0200 (CEST) Received: from relay68.bu.edu (relay68.bu.edu [128.197.228.73]) by dpdk.org (Postfix) with ESMTP id 420181B3C0 for ; Tue, 9 Oct 2018 21:30:21 +0200 (CEST) X-Envelope-From: qiaobinf@bu.edu X-BU-AUTH: xia1.bu.edu [128.197.41.97] Received: from BU-AUTH (localhost.localdomain [127.0.0.1]) (authenticated bits=0) by relay68.bu.edu (8.14.3/8.14.3) with ESMTP id w99JTCFe013605 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-GCM-SHA256 bits=128 verify=NO); Tue, 9 Oct 2018 15:29:24 -0400 From: Qiaobin Fu To: bruce.richardson@intel.com, pablo.de.lara.guarch@intel.com Cc: dev@dpdk.org, doucette@bu.edu, keith.wiles@intel.com, sameh.gobriel@intel.com, charlie.tai@intel.com, stephen@networkplumber.org, nd@arm.com, honnappa.nagarahalli@arm.com, yipeng1.wang@intel.com, michel@digirati.com.br, qiaobinf@bu.edu Date: Tue, 9 Oct 2018 19:29:06 +0000 Message-Id: <20181009192907.85650-1-qiaobinf@bu.edu> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20180831165101.20026-1-qiaobinf@bu.edu> References: <20180831165101.20026-1-qiaobinf@bu.edu> Subject: [dpdk-dev] [PATCH v4 1/2] hash table: fix a bug in rte_hash_iterate() X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" In current implementation of rte_hash_iterate(), it tries to obtain the lock after the while loop. However, this may lead to a bug. Notice the following racing condition: 1. The while loop above finishes because it finds a not empty slot. But it does so without a lock. 2. Then we get the lock. 3. The position that was once not empty is now empty. BUG because next_key is invalid. This patch fixes this small bug. Signed-off-by: Qiaobin Fu Reviewed-by: Michel Machado --- lib/librte_hash/rte_cuckoo_hash.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/librte_hash/rte_cuckoo_hash.c b/lib/librte_hash/rte_cuckoo_hash.c index f7b86c8c9..a3e76684d 100644 --- a/lib/librte_hash/rte_cuckoo_hash.c +++ b/lib/librte_hash/rte_cuckoo_hash.c @@ -1317,16 +1317,18 @@ rte_hash_iterate(const struct rte_hash *h, const void **key, void **data, uint32 bucket_idx = *next / RTE_HASH_BUCKET_ENTRIES; idx = *next % RTE_HASH_BUCKET_ENTRIES; + __hash_rw_reader_lock(h); /* If current position is empty, go to the next one */ while (h->buckets[bucket_idx].key_idx[idx] == EMPTY_SLOT) { (*next)++; /* End of table */ - if (*next == total_entries) + if (*next == total_entries) { + __hash_rw_reader_unlock(h); return -ENOENT; + } bucket_idx = *next / RTE_HASH_BUCKET_ENTRIES; idx = *next % RTE_HASH_BUCKET_ENTRIES; } - __hash_rw_reader_lock(h); /* Get position of entry in key table */ position = h->buckets[bucket_idx].key_idx[idx]; next_key = (struct rte_hash_key *) ((char *)h->key_store +