From patchwork Mon May 6 10:37:20 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ales Musil X-Patchwork-Id: 1931889 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=G3GHiuda; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=2605:bc80:3010::133; helo=smtp2.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=patchwork.ozlabs.org) Received: from smtp2.osuosl.org (smtp2.osuosl.org [IPv6:2605:bc80:3010::133]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (secp384r1) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4VXyXW3LHfz1xnT for ; Mon, 6 May 2024 20:37:31 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by smtp2.osuosl.org (Postfix) with ESMTP id 6A55740578; Mon, 6 May 2024 10:37:29 +0000 (UTC) X-Virus-Scanned: amavis at osuosl.org Received: from smtp2.osuosl.org ([127.0.0.1]) by localhost (smtp2.osuosl.org [127.0.0.1]) (amavis, port 10024) with ESMTP id iqCNRdmRw-PK; Mon, 6 May 2024 10:37:28 +0000 (UTC) X-Comment: SPF check N/A for local connections - client-ip=140.211.9.56; helo=lists.linuxfoundation.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver= DKIM-Filter: OpenDKIM Filter v2.11.0 smtp2.osuosl.org 2BA38400BA Authentication-Results: smtp2.osuosl.org; dkim=fail reason="signature verification failed" (1024-bit key) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=G3GHiuda Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp2.osuosl.org (Postfix) with ESMTPS id 2BA38400BA; Mon, 6 May 2024 10:37:28 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 083A4C007C; Mon, 6 May 2024 10:37:28 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp2.osuosl.org (smtp2.osuosl.org [IPv6:2605:bc80:3010::133]) by lists.linuxfoundation.org (Postfix) with ESMTP id C6F19C0037 for ; Mon, 6 May 2024 10:37:26 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp2.osuosl.org (Postfix) with ESMTP id AE8C24011F for ; Mon, 6 May 2024 10:37:26 +0000 (UTC) X-Virus-Scanned: amavis at osuosl.org Received: from smtp2.osuosl.org ([127.0.0.1]) by localhost (smtp2.osuosl.org [127.0.0.1]) (amavis, port 10024) with ESMTP id XRlXyg916DK3 for ; Mon, 6 May 2024 10:37:25 +0000 (UTC) Received-SPF: Pass (mailfrom) identity=mailfrom; client-ip=170.10.129.124; helo=us-smtp-delivery-124.mimecast.com; envelope-from=amusil@redhat.com; receiver= DMARC-Filter: OpenDMARC Filter v1.4.2 smtp2.osuosl.org 7DA1D400BA Authentication-Results: smtp2.osuosl.org; dmarc=pass (p=none dis=none) header.from=redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 smtp2.osuosl.org 7DA1D400BA Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by smtp2.osuosl.org (Postfix) with ESMTPS id 7DA1D400BA for ; Mon, 6 May 2024 10:37:25 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1714991844; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=kbZYyzEIQw2iKgrhypCcBOSYWg5U2wdmmR9NARyai+8=; b=G3GHiudasQkq8ddWCZnOvG+HPZBE32ssJyb5E/2JTxakUUCmshfLQ4VlyjNQM+DWBiTfI/ /Rs+ISJSh0UMCuJrbqngOewNF0KGXbdohAxP5PqP9iZ0W2+lwgcOAu0G2vna6115V+6fX4 8brql3sxsU6C/6VaV/cwpH235bi0wFw= Received: from mimecast-mx02.redhat.com (mx-ext.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-329-Hj38HKhlOgifcXe48sGqBA-1; Mon, 06 May 2024 06:37:22 -0400 X-MC-Unique: Hj38HKhlOgifcXe48sGqBA-1 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.rdu2.redhat.com [10.11.54.6]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 36FAA29AA3B2; Mon, 6 May 2024 10:37:22 +0000 (UTC) Received: from amusil.redhat.com (unknown [10.45.224.47]) by smtp.corp.redhat.com (Postfix) with ESMTP id 37E32200B09C; Mon, 6 May 2024 10:37:20 +0000 (UTC) From: Ales Musil To: dev@openvswitch.org Date: Mon, 6 May 2024 12:37:20 +0200 Message-ID: <20240506103720.333179-1-amusil@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.6 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Subject: [ovs-dev] [PATCH ovn] controller: Avoid use after free in LB I-P. 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" Avoid use after free in scenario when controller received LB deletion after the DB was reconnected. The reconnect led to idl clearing up the "old" structs, one of them being the LB. However, during recompute the struct was referenced when it was already gone. Clear the whole objdep_mgr instead of going one-by-one during recompute. ==143949==ERROR: AddressSanitizer: heap-use-after-free READ of size 4 at 0x5130000280d0 thread T0 0 0x61c3c9 in lb_data_local_lb_remove controller/ovn-controller.c:2978:5 1 0x5fd4df in en_lb_data_run controller/ovn-controller.c:3063:9 2 0x6fe0d9 in engine_recompute lib/inc-proc-eng.c:415:5 3 0x6fbdc2 in engine_run_node lib/inc-proc-eng.c:477:9 4 0x6fbdc2 in engine_run lib/inc-proc-eng.c:528:9 5 0x5f39a0 in main controller/ovn-controller.c Fixes: 8382127186bf ("controller: Store load balancer data in separate node") Reported-at: https://issues.redhat.com/browse/FDP-610 Signed-off-by: Ales Musil --- controller/ovn-controller.c | 20 +++++++++---------- tests/ovn-controller.at | 38 +++++++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 10 deletions(-) diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index 23269af83..65b9ba8e5 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -2972,7 +2972,7 @@ lb_data_local_lb_add(struct ed_type_lb_data *lb_data, static void lb_data_local_lb_remove(struct ed_type_lb_data *lb_data, - struct ovn_controller_lb *lb, bool tracked) + struct ovn_controller_lb *lb) { const struct uuid *uuid = &lb->slb->header_.uuid; @@ -2981,12 +2981,8 @@ lb_data_local_lb_remove(struct ed_type_lb_data *lb_data, lb_data_removed_five_tuples_add(lb_data, lb); - if (tracked) { - hmap_insert(&lb_data->old_lbs, &lb->hmap_node, uuid_hash(uuid)); - uuidset_insert(&lb_data->deleted, uuid); - } else { - ovn_controller_lb_destroy(lb); - } + hmap_insert(&lb_data->old_lbs, &lb->hmap_node, uuid_hash(uuid)); + uuidset_insert(&lb_data->deleted, uuid); } static bool @@ -3011,7 +3007,7 @@ lb_data_handle_changed_ref(enum objdep_type type, const char *res_name, continue; } - lb_data_local_lb_remove(lb_data, lb, true); + lb_data_local_lb_remove(lb_data, lb); const struct sbrec_load_balancer *sbrec_lb = sbrec_load_balancer_table_get_for_uuid(ctx_in->lb_table, uuid); @@ -3057,9 +3053,13 @@ en_lb_data_run(struct engine_node *node, void *data) const struct sbrec_load_balancer_table *lb_table = EN_OVSDB_GET(engine_get_input("SB_load_balancer", node)); + objdep_mgr_clear(&lb_data->deps_mgr); + struct ovn_controller_lb *lb; HMAP_FOR_EACH_SAFE (lb, hmap_node, &lb_data->local_lbs) { - lb_data_local_lb_remove(lb_data, lb, false); + hmap_remove(&lb_data->local_lbs, &lb->hmap_node); + lb_data_removed_five_tuples_add(lb_data, lb); + ovn_controller_lb_destroy(lb); } const struct sbrec_load_balancer *sbrec_lb; @@ -3097,7 +3097,7 @@ lb_data_sb_load_balancer_handler(struct engine_node *node, void *data) continue; } - lb_data_local_lb_remove(lb_data, lb, true); + lb_data_local_lb_remove(lb_data, lb); } if (sbrec_load_balancer_is_deleted(sbrec_lb) || diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at index 27cec2aec..cecbc190b 100644 --- a/tests/ovn-controller.at +++ b/tests/ovn-controller.at @@ -2973,3 +2973,41 @@ priority=1100,ip,reg15=0x1,metadata=0x1,nw_src=10.0.0.4 actions=load:0x1->OXM_OF OVN_CLEANUP([hv1]) AT_CLEANUP + +AT_SETUP([ovn-controller - LB remove after disconnect]) +ovn_start + +net_add n1 +sim_add hv1 +as hv1 +check ovs-vsctl add-br br-phys +ovn_attach n1 br-phys 192.168.0.1 +check ovs-vsctl -- add-port br-int vif1 -- \ + set interface vif1 external-ids:iface-id=lsp + +check ovs-vsctl set Open_vSwitch . external-ids:ovn-remote-probe-interval="5000" + +check ovn-nbctl ls-add ls +check ovn-nbctl lsp-add ls lsp \ +-- lsp-set-addresses lsp "f0:00:00:00:00:01 172.16.0.10" + +check ovn-nbctl lb-add lb 192.168.100.100 172.16.0.10 +check ovn-nbctl ls-lb-add ls lb + +wait_for_ports_up +check ovn-nbctl --wait=hv sync + +sleep_sb +OVS_WAIT_UNTIL([grep -q 'OVNSB commit failed' hv1/ovn-controller.log]) + +sleep_controller hv1 +wake_up_sb + +ovn-nbctl lb-del lb + +wake_up_controller hv1 +check ovn-nbctl --wait=hv sync + +OVN_CLEANUP([hv1 +/no response to inactivity probe after .* seconds, disconnecting/d]) +AT_CLEANUP