From patchwork Tue May 14 19:25:24 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Vladislav Odintsov X-Patchwork-Id: 1935127 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" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256 header.s=20230601 header.b=DftmOvKC; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=140.211.166.133; helo=smtp2.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=patchwork.ozlabs.org) Received: from smtp2.osuosl.org (smtp2.osuosl.org [140.211.166.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 4Vf5tH398Pz2140 for ; Wed, 15 May 2024 05:25:41 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by smtp2.osuosl.org (Postfix) with ESMTP id 7AC874181E; Tue, 14 May 2024 19:25:39 +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 FjwvDiXCmeyc; Tue, 14 May 2024 19:25:37 +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 EA0DD4180A Authentication-Results: smtp2.osuosl.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256 header.s=20230601 header.b=DftmOvKC Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp2.osuosl.org (Postfix) with ESMTPS id EA0DD4180A; Tue, 14 May 2024 19:25:36 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id BE7EEC0072; Tue, 14 May 2024 19:25:36 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp2.osuosl.org (smtp2.osuosl.org [140.211.166.133]) by lists.linuxfoundation.org (Postfix) with ESMTP id 0757DC0037 for ; Tue, 14 May 2024 19:25:35 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp2.osuosl.org (Postfix) with ESMTP id DD39940114 for ; Tue, 14 May 2024 19:25:34 +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 RUEkhwdFVf2S for ; Tue, 14 May 2024 19:25:34 +0000 (UTC) Received-SPF: Pass (mailfrom) identity=mailfrom; client-ip=2a00:1450:4864:20::12e; helo=mail-lf1-x12e.google.com; envelope-from=odivlad@gmail.com; receiver= DMARC-Filter: OpenDMARC Filter v1.4.2 smtp2.osuosl.org E0A93400C1 Authentication-Results: smtp2.osuosl.org; dmarc=pass (p=none dis=none) header.from=gmail.com DKIM-Filter: OpenDKIM Filter v2.11.0 smtp2.osuosl.org E0A93400C1 Received: from mail-lf1-x12e.google.com (mail-lf1-x12e.google.com [IPv6:2a00:1450:4864:20::12e]) by smtp2.osuosl.org (Postfix) with ESMTPS id E0A93400C1 for ; Tue, 14 May 2024 19:25:33 +0000 (UTC) Received: by mail-lf1-x12e.google.com with SMTP id 2adb3069b0e04-5238fe0cfc9so40960e87.0 for ; Tue, 14 May 2024 12:25:33 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1715714731; x=1716319531; darn=openvswitch.org; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=oauaaD/0hFQsksshFRYv7GGVpj228E6Ml72ZH8j/VQY=; b=DftmOvKCsmkzjjwoDfxIEJMgZfNUWtRguT+fOvMV5nujtdLHruoZLMXBpFhL/eSoXK Ba/OfraVR8m/GLRL4kgELw9M/dPcUbSWdlxjpn7ajSZDHLLsuvq5h+MaINoMxxerFIpw 99fjx27Pj1LWImXfLJDe938rGmfmp6w9DI9mxDpzPwO6g3niNHU+x+EibtO1sU1LyZhb Q1Gn49SHBvErGyhkNEzWTOGdO3h1A+SUtFC+Ynrc1NzRAQpIx9FxFdR9XH4R3euqrs4K u9CZ1Q4QgsVUbJB4paOzM48Zs8/9VEeg7smbBuXTGomZsSJWe+jlhZEjOk/4n8JNTorT BD5A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1715714731; x=1716319531; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=oauaaD/0hFQsksshFRYv7GGVpj228E6Ml72ZH8j/VQY=; b=lef+KRP1tRHGZrzJ5/+K3OmlQBAGwketyKseVFenGpkKWuQtwtFpan6lZv2VSeGKO0 6NTYQ30yeTztObi671FO3dS0sNh5LFOW4pGyTV9JiOkciEYillNsMQVdzBlUTe3Qy428 AW0Wbp7mXVjXqVhyRJWB6svvGYt0vCCO8bUhOmwc+94E3j2WNUgHMXUgRAS4h9R795tj jMAHb9HT/wSOcvpmNf2LAptpxwa0P+Z3s4P8Vl0JodVxFWRz1P9U7VxM2Nt3LAOEJwvO 6aJZG6sZLiPjIRP/shRMBnEoQD3gVbdHkJybaR1NFhcV+MLPdLX7L82O5nF5wUtYKPQu Cp2g== X-Gm-Message-State: AOJu0YyT3sjkyPq3c6uoLQjk/kjHnjd5p0xdIN3U/aNKrc4pkh0o5f3X 1XyJrNb9Hmi3P8GAIP+92atnKQyguVAebEwEn4nMK0+VK+/iFPrX3+7IZw== X-Google-Smtp-Source: AGHT+IFUL5cAWMt+jAUERyIQXDxOrCefLrvaqxgADy7hG6zNCoqG9z4kgruxHlmYFkOuXRY7VW26yg== X-Received: by 2002:a19:5e5d:0:b0:51f:b3b:6373 with SMTP id 2adb3069b0e04-521e0c63c91mr5288086e87.15.1715714730703; Tue, 14 May 2024 12:25:30 -0700 (PDT) Received: from ip-10-70-112-12.vpc-1e810be1.internal (c2-185-102-122-48.elastic.cloud.croc.ru. [185.102.122.48]) by smtp.gmail.com with ESMTPSA id 2adb3069b0e04-521f35ad58fsm2253268e87.47.2024.05.14.12.25.28 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 14 May 2024 12:25:29 -0700 (PDT) From: Vladislav Odintsov To: dev@openvswitch.org Date: Tue, 14 May 2024 22:25:24 +0300 Message-ID: <20240514192525.524142-1-odivlad@gmail.com> X-Mailer: git-send-email 2.44.0 MIME-Version: 1.0 Cc: Vladislav Odintsov Subject: [ovs-dev] [PATCH ovn] controller: Store src_mac, src_ip in svc_monitor struct. 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" These structure members are read in pinctrl_handler() in a separare thread. This is unsafe: when IDL is re-connected or some IDL objects are freed after svc_monitors list with svc_monitor structures, which point to sbrec_service_monitor is initialized, sb_svc_mon structure property can point to wrong address, which then leads to segmentation fault in svc_monitor_send_tcp_health_check__() and svc_monitor_send_udp_health_check() on accessing svc_mon->sb_svc_mon. Imagine situation: Main ovn-controller thread: 1. Connects to SB and fills IDL with DB contents. 2. run pinctrl_run() with pinctrl mutex and sync_svc_monitors() as a part of it. 3. Release mutex. ... 4. Loss of OVNSB connection for any reason (network outage/inactivity probe timeout/etc), start new main-loop iteration, re-initialize IDL in ovsdb_idl_run() (which probably will destroy previous IDL objects). ... pinctrl thread: 5. Awake from poll_block(). ... run new iteration in its main loop ... 6. Aqcuire mutex lock. 7. Run svc_monitors_run(), run svc_monitor_send_tcp_health_check__() or svc_monitor_send_udp_health_check(), which try to access IDL objects with outdated pointers. 8. Segmentation fault: Stack trace of thread 212406: >> __GI_strlen (libc.so.6) >> inet_pton (libc.so.6) >> ip_parse (ovn-controller) >> svc_monitor_send_tcp_health_check__.part.37 (ovn-controller) >> svc_monitor_send_health_check (ovn-controller) >> pinctrl_handler (ovn-controller) >> ovsthread_wrapper (ovn-controller) >> start_thread (libpthread.so.0) >> __clone (libc.so.6) This patch removes unsafe access to IDL objects from pinctrl thread. New svc_monitor structure members are used to store needed data. CC: Numan Siddique Fixes: 8be01f4a5329 ("Send service monitor health checks") Signed-off-by: Vladislav Odintsov --- controller/pinctrl.c | 43 ++++++++++++++++++++++--------------------- 1 file changed, 22 insertions(+), 21 deletions(-) diff --git a/controller/pinctrl.c b/controller/pinctrl.c index 6a2c3dc68..b843edb35 100644 --- a/controller/pinctrl.c +++ b/controller/pinctrl.c @@ -7307,6 +7307,9 @@ struct svc_monitor { long long int timestamp; bool is_ip6; + struct eth_addr src_mac; + struct in6_addr src_ip; + long long int wait_time; long long int next_send_time; @@ -7501,6 +7504,15 @@ sync_svc_monitors(struct ovsdb_idl_txn *ovnsb_idl_txn, svc_mon->n_success = 0; svc_mon->n_failures = 0; + eth_addr_from_string(sb_svc_mon->src_mac, &svc_mon->src_mac); + if (is_ipv4) { + ovs_be32 ip4_src; + ip_parse(sb_svc_mon->src_ip, &ip4_src); + svc_mon->src_ip = in6_addr_mapped_ipv4(ip4_src); + } else { + ipv6_parse(sb_svc_mon->src_ip, &svc_mon->src_ip); + } + hmap_insert(&svc_monitors_map, &svc_mon->hmap_node, hash); ovs_list_push_back(&svc_monitors, &svc_mon->list_node); changed = true; @@ -8259,19 +8271,14 @@ svc_monitor_send_tcp_health_check__(struct rconn *swconn, struct dp_packet packet; dp_packet_use_stub(&packet, packet_stub, sizeof packet_stub); - struct eth_addr eth_src; - eth_addr_from_string(svc_mon->sb_svc_mon->src_mac, ð_src); if (svc_mon->is_ip6) { - struct in6_addr ip6_src; - ipv6_parse(svc_mon->sb_svc_mon->src_ip, &ip6_src); - pinctrl_compose_ipv6(&packet, eth_src, svc_mon->ea, - &ip6_src, &svc_mon->ip, IPPROTO_TCP, + pinctrl_compose_ipv6(&packet, svc_mon->src_mac, svc_mon->ea, + &svc_mon->src_ip, &svc_mon->ip, IPPROTO_TCP, 63, TCP_HEADER_LEN); } else { - ovs_be32 ip4_src; - ip_parse(svc_mon->sb_svc_mon->src_ip, &ip4_src); - pinctrl_compose_ipv4(&packet, eth_src, svc_mon->ea, - ip4_src, in6_addr_get_mapped_ipv4(&svc_mon->ip), + pinctrl_compose_ipv4(&packet, svc_mon->src_mac, svc_mon->ea, + in6_addr_get_mapped_ipv4(&svc_mon->src_ip), + in6_addr_get_mapped_ipv4(&svc_mon->ip), IPPROTO_TCP, 63, TCP_HEADER_LEN); } @@ -8327,24 +8334,18 @@ svc_monitor_send_udp_health_check(struct rconn *swconn, struct svc_monitor *svc_mon, ovs_be16 udp_src) { - struct eth_addr eth_src; - eth_addr_from_string(svc_mon->sb_svc_mon->src_mac, ð_src); - uint64_t packet_stub[128 / 8]; struct dp_packet packet; dp_packet_use_stub(&packet, packet_stub, sizeof packet_stub); if (svc_mon->is_ip6) { - struct in6_addr ip6_src; - ipv6_parse(svc_mon->sb_svc_mon->src_ip, &ip6_src); - pinctrl_compose_ipv6(&packet, eth_src, svc_mon->ea, - &ip6_src, &svc_mon->ip, IPPROTO_UDP, + pinctrl_compose_ipv6(&packet, svc_mon->src_mac, svc_mon->ea, + &svc_mon->src_ip, &svc_mon->ip, IPPROTO_UDP, 63, UDP_HEADER_LEN + 8); } else { - ovs_be32 ip4_src; - ip_parse(svc_mon->sb_svc_mon->src_ip, &ip4_src); - pinctrl_compose_ipv4(&packet, eth_src, svc_mon->ea, - ip4_src, in6_addr_get_mapped_ipv4(&svc_mon->ip), + pinctrl_compose_ipv4(&packet, svc_mon->src_mac, svc_mon->ea, + in6_addr_get_mapped_ipv4(&svc_mon->src_ip), + in6_addr_get_mapped_ipv4(&svc_mon->ip), IPPROTO_UDP, 63, UDP_HEADER_LEN + 8); }