From patchwork Tue Jan 10 23:54:02 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Joe Stringer X-Patchwork-Id: 713494 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from mail.linuxfoundation.org (mail.linuxfoundation.org [140.211.169.12]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3typlq2fYtz9sCX for ; Wed, 11 Jan 2017 10:54:31 +1100 (AEDT) Received: from mail.linux-foundation.org (localhost [127.0.0.1]) by mail.linuxfoundation.org (Postfix) with ESMTP id 6D583B57; Tue, 10 Jan 2017 23:54:28 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@mail.linuxfoundation.org Received: from smtp1.linuxfoundation.org (smtp1.linux-foundation.org [172.17.192.35]) by mail.linuxfoundation.org (Postfix) with ESMTPS id 92F6EA5E for ; Tue, 10 Jan 2017 23:54:26 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from relay5-d.mail.gandi.net (relay5-d.mail.gandi.net [217.70.183.197]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id EF9DC159 for ; Tue, 10 Jan 2017 23:54:25 +0000 (UTC) Received: from mfilter28-d.gandi.net (mfilter28-d.gandi.net [217.70.178.159]) by relay5-d.mail.gandi.net (Postfix) with ESMTP id BA7E841C04C; Wed, 11 Jan 2017 00:54:24 +0100 (CET) X-Virus-Scanned: Debian amavisd-new at mfilter28-d.gandi.net Received: from relay5-d.mail.gandi.net ([IPv6:::ffff:217.70.183.197]) by mfilter28-d.gandi.net (mfilter28-d.gandi.net [::ffff:10.0.15.180]) (amavisd-new, port 10024) with ESMTP id 2G4rlaoWbbkC; Wed, 11 Jan 2017 00:54:23 +0100 (CET) X-Originating-IP: 208.91.1.34 Received: from carno.eng.vmware.com (unknown [208.91.1.34]) (Authenticated sender: joe@ovn.org) by relay5-d.mail.gandi.net (Postfix) with ESMTPSA id 2535A41C07D; Wed, 11 Jan 2017 00:54:21 +0100 (CET) From: Joe Stringer To: dev@openvswitch.org Date: Tue, 10 Jan 2017 15:54:02 -0800 Message-Id: <20170110235403.15331-1-joe@ovn.org> X-Mailer: git-send-email 2.10.2 X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_LOW, RCVD_IN_SORBS_SPAM autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on smtp1.linux-foundation.org Subject: [ovs-dev] [PATCH 1/2] revalidator: Prevent double-delete of ukey. X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: ovs-dev-bounces@openvswitch.org Errors-To: ovs-dev-bounces@openvswitch.org revalidator_sweep__() splits checking for whether to delete a ukey from the actual deletion to prevent taking the umap lock for too long. However it uses information gathered from the first critical section to decide to call ukey_delete() - ie, the second critical section. Since 67f08985d769 ("upcall: Replace ukeys for deleted flows."), it is possible for a handler thread to receive an upcall for the same flow and to replace the ukey which is being deleted with a new one, in between these critical sections. This will remove the ukey from the cmap, rcu-defer its deletion, and update the existing ukey. If this occurs in between the critical sections of revalidator cleanup of the flow, then the revalidator will subsequently call ukey_delete() to delete the original ukey, which was already deleted by the handler thread. Guard against this by checking the ukey state in ukey_delete(). Backtrace: Program terminated with signal 11, Segmentation fault. #0 0x00007fe969b13da3 in cmap_replace__ () #1 0x00007fe969b14491 in cmap_replace () #2 0x00007fe969aee9ff in ukey_delete () #3 0x00007fe969aefd42 in revalidator_sweep__ () #4 0x00007fe969af1bad in udpif_revalidator () #5 0x00007fe969b8b2a6 in ovsthread_wrapper () #6 0x00007fe968e07dc5 in start_thread () from /lib64/libpthread.so.0 #7 0x00007fe96862c73d in clone () from /lib64/libc.so.6 Fixes: 54ebeff4c03d ("upcall: Track ukey states.") Fixes: 67f08985d769 ("upcall: Replace ukeys for deleted flows.") Reported-by: Numan Siddique Signed-off-by: Joe Stringer Acked-by: Jarno Rajahalme --- ofproto/ofproto-dpif-upcall.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c index cd2445fc4ab9..1ffeaabf7d8e 100644 --- a/ofproto/ofproto-dpif-upcall.c +++ b/ofproto/ofproto-dpif-upcall.c @@ -1794,9 +1794,11 @@ ukey_delete(struct umap *umap, struct udpif_key *ukey) OVS_REQUIRES(umap->mutex) { ovs_mutex_lock(&ukey->mutex); - cmap_remove(&umap->cmap, &ukey->cmap_node, ukey->hash); - ovsrcu_postpone(ukey_delete__, ukey); - transition_ukey(ukey, UKEY_DELETED); + if (ukey->state < UKEY_DELETED) { + cmap_remove(&umap->cmap, &ukey->cmap_node, ukey->hash); + ovsrcu_postpone(ukey_delete__, ukey); + transition_ukey(ukey, UKEY_DELETED); + } ovs_mutex_unlock(&ukey->mutex); }