From patchwork Fri Sep 9 20:48:51 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Joe Stringer X-Patchwork-Id: 668255 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from archives.nicira.com (archives.nicira.com [96.126.127.54]) by ozlabs.org (Postfix) with ESMTP id 3sW8Sm2BFvz9ryT for ; Sat, 10 Sep 2016 06:49:12 +1000 (AEST) Received: from archives.nicira.com (localhost [127.0.0.1]) by archives.nicira.com (Postfix) with ESMTP id A6A6B10703; Fri, 9 Sep 2016 13:49:07 -0700 (PDT) X-Original-To: dev@openvswitch.org Delivered-To: dev@openvswitch.org Received: from mx3v3.cudamail.com (mx3.cudamail.com [64.34.241.5]) by archives.nicira.com (Postfix) with ESMTPS id E33AB106FA for ; Fri, 9 Sep 2016 13:49:05 -0700 (PDT) Received: from bar6.cudamail.com (localhost [127.0.0.1]) by mx3v3.cudamail.com (Postfix) with ESMTPS id 6CBBB16170E for ; Fri, 9 Sep 2016 14:49:05 -0600 (MDT) X-ASG-Debug-ID: 1473454144-0b3237334415d20001-byXFYA Received: from mx1-pf2.cudamail.com ([192.168.24.2]) by bar6.cudamail.com with ESMTP id DQpjtmh8Sfz2xyAv (version=TLSv1 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO) for ; Fri, 09 Sep 2016 14:49:04 -0600 (MDT) X-Barracuda-Envelope-From: joe@ovn.org X-Barracuda-RBL-Trusted-Forwarder: 192.168.24.2 Received: from unknown (HELO relay6-d.mail.gandi.net) (217.70.183.198) by mx1-pf2.cudamail.com with ESMTPS (DHE-RSA-AES256-SHA encrypted); 9 Sep 2016 20:49:04 -0000 Received-SPF: pass (mx1-pf2.cudamail.com: SPF record at ovn.org designates 217.70.183.198 as permitted sender) X-Barracuda-Apparent-Source-IP: 217.70.183.198 X-Barracuda-RBL-IP: 217.70.183.198 Received: from mfilter36-d.gandi.net (mfilter36-d.gandi.net [217.70.178.167]) by relay6-d.mail.gandi.net (Postfix) with ESMTP id 02282FB8B8; Fri, 9 Sep 2016 22:49:03 +0200 (CEST) X-Virus-Scanned: Debian amavisd-new at mfilter36-d.gandi.net Received: from relay6-d.mail.gandi.net ([IPv6:::ffff:217.70.183.198]) by mfilter36-d.gandi.net (mfilter36-d.gandi.net [::ffff:10.0.15.180]) (amavisd-new, port 10024) with ESMTP id rKGkMUdeuJv8; Fri, 9 Sep 2016 22:49:01 +0200 (CEST) X-Originating-IP: 208.91.1.34 Received: from archer.eng.vmware.com (unknown [208.91.1.34]) (Authenticated sender: joe@ovn.org) by relay6-d.mail.gandi.net (Postfix) with ESMTPSA id 8F0AAFB8A0; Fri, 9 Sep 2016 22:49:00 +0200 (CEST) X-CudaMail-Envelope-Sender: joe@ovn.org From: Joe Stringer To: dev@openvswitch.org X-CudaMail-Whitelist-To: dev@openvswitch.org X-CudaMail-MID: CM-E2-908062817 X-CudaMail-DTE: 090916 X-CudaMail-Originating-IP: 217.70.183.198 Date: Fri, 9 Sep 2016 13:48:51 -0700 X-ASG-Orig-Subj: [##CM-E2-908062817##][PATCH 1/3] ovsdb-monitor: Fix segfault during replication. Message-Id: <20160909204853.27839-2-joe@ovn.org> X-Mailer: git-send-email 2.9.3 In-Reply-To: <20160909204853.27839-1-joe@ovn.org> References: <20160909204853.27839-1-joe@ovn.org> X-Barracuda-Connect: UNKNOWN[192.168.24.2] X-Barracuda-Start-Time: 1473454144 X-Barracuda-Encrypted: DHE-RSA-AES256-SHA X-Barracuda-URL: https://web.cudamail.com:443/cgi-mod/mark.cgi X-ASG-Whitelist: Header =?UTF-8?B?eFwtY3VkYW1haWxcLXdoaXRlbGlzdFwtdG8=?= X-Virus-Scanned: by bsmtpd at cudamail.com X-Barracuda-BRTS-Status: 1 Subject: [ovs-dev] [PATCH 1/3] ovsdb-monitor: Fix segfault during replication. X-BeenThere: dev@openvswitch.org X-Mailman-Version: 2.1.16 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: dev-bounces@openvswitch.org Sender: "dev" Prior to commit 60e0cd041958 ("ovsdb: Replication usability improvements"), a changes would never have 'change->new' == NULL. Apparently this is now possible, which leads to the crash below. Valgrind reports: ==18725== Process terminating with default action of signal 11 (SIGSEGV): dumping core ==18725== Access not within mapped region at address 0x0 ==18725== at 0x43937E: ovsdb_datum_compare_3way (ovsdb-data.c:1626) ==18725== by 0x439344: ovsdb_datum_equals (ovsdb-data.c:1616) ==18725== by 0x4166CC: update_monitor_row_data (monitor.c:310) ==18725== by 0x414A90: ovsdb_monitor_changes_update (monitor.c:1255) ==18725== by 0x417009: ovsdb_monitor_change_cb (monitor.c:1339) ==18725== by 0x41DB52: ovsdb_txn_for_each_change (transaction.c:906) ==18725== by 0x416CC9: ovsdb_monitor_commit (monitor.c:1553) ==18725== by 0x41D993: ovsdb_txn_commit_ (transaction.c:868) ==18725== by 0x41D6F5: ovsdb_txn_commit (transaction.c:893) ==18725== by 0x418185: process_notification (replication.c:576) ==18725== by 0x417705: replication_run (replication.c:185) ==18725== by 0x408240: main_loop (ovsdb-server.c:198) ==18725== by 0x406432: main (ovsdb-server.c:429) Fixes: 60e0cd041958 ("ovsdb: Replication usability improvements") Signed-off-by: Joe Stringer --- This addresses the crash, but I don't understand the deeper reasoning why we get into this state in the first place so I welcome any counter proposals. This also affects branch-2.6. --- ovsdb/monitor.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/ovsdb/monitor.c b/ovsdb/monitor.c index 9a6fbf5a5a30..5320c7db0514 100644 --- a/ovsdb/monitor.c +++ b/ovsdb/monitor.c @@ -1252,7 +1252,11 @@ ovsdb_monitor_changes_update(const struct ovsdb_row *old, change->new = clone_monitor_row_data(mt, new); } else { if (new) { - update_monitor_row_data(mt, new, change->new); + if (change->new) { + update_monitor_row_data(mt, new, change->new); + } else { + change->new = clone_monitor_row_data(mt, new); + } } else { free_monitor_row_data(mt, change->new); change->new = NULL;