From patchwork Tue Feb 23 13:55:08 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ilya Maximets X-Patchwork-Id: 1443503 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=140.211.166.138; helo=smtp1.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from smtp1.osuosl.org (smtp1.osuosl.org [140.211.166.138]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4DlLCv3FXnz9sVr for ; Wed, 24 Feb 2021 00:55:23 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by smtp1.osuosl.org (Postfix) with ESMTP id 7830B83AB9; Tue, 23 Feb 2021 13:55:21 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp1.osuosl.org ([127.0.0.1]) by localhost (smtp1.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id UKTDUb3I-Kec; Tue, 23 Feb 2021 13:55:20 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp1.osuosl.org (Postfix) with ESMTP id 70C5F83410; Tue, 23 Feb 2021 13:55:19 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 489BBC000A; Tue, 23 Feb 2021 13:55:19 +0000 (UTC) X-Original-To: ovs-dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp3.osuosl.org (smtp3.osuosl.org [140.211.166.136]) by lists.linuxfoundation.org (Postfix) with ESMTP id 77DFBC0001 for ; Tue, 23 Feb 2021 13:55:17 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id 59BA06060B for ; Tue, 23 Feb 2021 13:55:17 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp3.osuosl.org ([127.0.0.1]) by localhost (smtp3.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 7woywXpCzF5o for ; Tue, 23 Feb 2021 13:55:16 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 Received: from relay4-d.mail.gandi.net (relay4-d.mail.gandi.net [217.70.183.196]) by smtp3.osuosl.org (Postfix) with ESMTPS id F393D605F2 for ; Tue, 23 Feb 2021 13:55:15 +0000 (UTC) X-Originating-IP: 78.45.89.65 Received: from im-t490s.redhat.com (ip-78-45-89-65.net.upcbroadband.cz [78.45.89.65]) (Authenticated sender: i.maximets@ovn.org) by relay4-d.mail.gandi.net (Postfix) with ESMTPSA id 28D38E0004; Tue, 23 Feb 2021 13:55:10 +0000 (UTC) From: Ilya Maximets To: ovs-dev@openvswitch.org, Ben Pfaff Date: Tue, 23 Feb 2021 14:55:08 +0100 Message-Id: <20210223135508.322585-1-i.maximets@ovn.org> X-Mailer: git-send-email 2.26.2 MIME-Version: 1.0 Cc: Ilya Maximets , Dumitru Ceara Subject: [ovs-dev] [PATCH] ovsdb-cs: Fix use-after-free for the request id. X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" ovsdb_cs_send_transaction() returns the pointer to the same 'request_id' object that is used internally. This leads to situation where transaction in idl and CS module has the same 'request_id' object. However, CS module is able to destroy this transaction id at any time, e.g. if connection state chnaged, but idl transaction might be still around at this moment and application might still use it. Found by running 'make check-ovsdb-cluster' with AddressSanitizer: ==79922==ERROR: AddressSanitizer: heap-use-after-free on address 0x604000167a98 at pc 0x000000626acf bp 0x7ffcdb38a4c0 sp 0x7ffcdb38a4b8 READ of size 8 at 0x604000167a98 thread T0 #0 0x626ace in json_destroy lib/json.c:354:18 #1 0x56d1ab in ovsdb_idl_txn_destroy lib/ovsdb-idl.c:2528:5 #2 0x53a908 in do_vsctl utilities/ovs-vsctl.c:3008:5 #3 0x539251 in main utilities/ovs-vsctl.c:203:17 #4 0x7f7f7e376081 in __libc_start_main (/lib64/libc.so.6+0x27081) #5 0x461fed in _start (utilities/ovs-vsctl+0x461fed) 0x604000167a98 is located 8 bytes inside of 40-byte region [0x604000167a90,0x604000167ab8) freed by thread T0 here: #0 0x503ac7 in free (utilities/ovs-vsctl+0x503ac7) #1 0x626aae in json_destroy lib/json.c:378:9 #2 0x6adfa2 in ovsdb_cs_run lib/ovsdb-cs.c:625:13 #3 0x567731 in ovsdb_idl_run lib/ovsdb-idl.c:394:5 #4 0x56fed1 in ovsdb_idl_txn_commit_block lib/ovsdb-idl.c:3187:9 #5 0x53a4df in do_vsctl utilities/ovs-vsctl.c:2898:14 #6 0x539251 in main utilities/ovs-vsctl.c:203:17 #7 0x7f7f7e376081 in __libc_start_main previously allocated by thread T0 here: #0 0x503dcf in malloc (utilities/ovs-vsctl+0x503dcf) #1 0x594656 in xmalloc lib/util.c:138:15 #2 0x626431 in json_create lib/json.c:1451:25 #3 0x626972 in json_integer_create lib/json.c:263:25 #4 0x62da0f in jsonrpc_create_id lib/jsonrpc.c:563:12 #5 0x62d9a8 in jsonrpc_create_request lib/jsonrpc.c:570:23 #6 0x6af3a6 in ovsdb_cs_send_transaction lib/ovsdb-cs.c:1357:35 #7 0x56e3d5 in ovsdb_idl_txn_commit lib/ovsdb-idl.c:3147:27 #8 0x56fea9 in ovsdb_idl_txn_commit_block lib/ovsdb-idl.c:3186:22 #9 0x53a4df in do_vsctl utilities/ovs-vsctl.c:2898:14 #10 0x539251 in main utilities/ovs-vsctl.c:203:17 #11 0x7f7f7e376081 in __libc_start_main Fixes: 1c337c43ac1c ("ovsdb-idl: Break into two layers.") Signed-off-by: Ilya Maximets --- lib/ovsdb-cs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ovsdb-cs.c b/lib/ovsdb-cs.c index e0934772e..ded7cc455 100644 --- a/lib/ovsdb-cs.c +++ b/lib/ovsdb-cs.c @@ -1367,7 +1367,7 @@ ovsdb_cs_send_transaction(struct ovsdb_cs *cs, struct json *operations) sizeof *cs->txns); } cs->txns[cs->n_txns++] = request_id; - return request_id; + return json_clone(request_id); } /* Makes 'cs' drop its record of transaction 'request_id'. If a reply arrives