From patchwork Sun Dec 31 17:34:31 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Gert Doering X-Patchwork-Id: 3540 Return-Path: Delivered-To: patchwork@openvpn.net Received: by 2002:a05:7301:2791:b0:100:d2e5:60d with SMTP id hm17csp1118621dyb; Sun, 31 Dec 2023 09:35:16 -0800 (PST) X-Google-Smtp-Source: AGHT+IFEuBVrgFOep1F74nYB3wEj52WLvbk+/tTVGQO6hHV1WQ0JAZbxRIKfs3MEKBGANvTApq8F X-Received: by 2002:a17:902:ce92:b0:1d4:f1c:634b with SMTP id f18-20020a170902ce9200b001d40f1c634bmr31367680plg.6.1704044115862; Sun, 31 Dec 2023 09:35:15 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1704044115; cv=none; d=google.com; s=arc-20160816; b=FrtOx8Y5TejvMPD3tsAz335cBnW8OvDrkf+EkpnhKR/8Xaw6HjyizbGUgip3GBH619 aPW6RYdehZ7eG4VrJPQaVTUPW9UXLFNDqnkZMTwpDUTO7crUB9Tv0JqLulHHO4D/NVt3 3RY/xmM9/tngz4mTJbb+FQOJ6uVvz/3RbpWmX92Il3N6XTWDhCeNMvsWQhPl7j8uvY64 PnPgh1qHzCjvpdqWPVz7encAvkDB8OM7jfVamLqJPNKCSLtReJrHfLSFbaJgmOmX47s2 EM6styTI/YOnSGtG7syBotAXcziJj/Q5o7qJFbmPsMuPLBac0I63NtWji65aXOLuLF/q IKLQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=errors-to:content-transfer-encoding:list-subscribe:list-help :list-post:list-archive:list-unsubscribe:list-id:precedence:subject :mime-version:references:in-reply-to:message-id:date:to:from :dkim-signature:dkim-signature; bh=uw6R2RXH7MXVQFdeiwsBRfMmN3tNsieorq2l04LUFjk=; fh=4NbAC/LsuMLI0S0hprUlLSLCiHwg6SCAifhH718Jh0Q=; b=IMG2vWRJufY7jZu7Q7Ga2/TmW3AoazdoxYElZssXI/T7c8psU1h+GVhJOeKe+unj2I kCOuYSAvojNb/MNHoTF8CWvCqhMcJimjTSQN6J9BuXrvpoDoRpomhug88rFjIzKnY9st 2Vb+xHKtoc5oY1zsBhlJ4RKQQq7P6unyBIc080Q064m/pjN39soQu+dABXGhhRrcuaqv 9pwB0QUiFUZDF4W6TbRdN860Lc/aPAMbk+MLwdTMCnVEWi3luny8YvDmhOhxlVNn95Sh i1swYJUA1jtV9L9RVeByhjuYizK1sZiz9hmf9Eriu4vwnwwVf9KUpnyYpEo6JBe+jmDl nIFA== ARC-Authentication-Results: i=1; mx.google.com; dkim=neutral (body hash did not verify) header.i=@sourceforge.net header.s=x header.b=hcTqY9HM; dkim=neutral (body hash did not verify) header.i=@sf.net header.s=x header.b=XGzuhe1M; spf=pass (google.com: domain of openvpn-devel-bounces@lists.sourceforge.net designates 216.105.38.7 as permitted sender) smtp.mailfrom=openvpn-devel-bounces@lists.sourceforge.net; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=muc.de Received: from lists.sourceforge.net (lists.sourceforge.net. [216.105.38.7]) by mx.google.com with ESMTPS id w5-20020a170902e88500b001d408d60138si14014122plg.115.2023.12.31.09.35.15 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Sun, 31 Dec 2023 09:35:15 -0800 (PST) Received-SPF: pass (google.com: domain of openvpn-devel-bounces@lists.sourceforge.net designates 216.105.38.7 as permitted sender) client-ip=216.105.38.7; Authentication-Results: mx.google.com; dkim=neutral (body hash did not verify) header.i=@sourceforge.net header.s=x header.b=hcTqY9HM; dkim=neutral (body hash did not verify) header.i=@sf.net header.s=x header.b=XGzuhe1M; spf=pass (google.com: domain of openvpn-devel-bounces@lists.sourceforge.net designates 216.105.38.7 as permitted sender) smtp.mailfrom=openvpn-devel-bounces@lists.sourceforge.net; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=muc.de Received: from [127.0.0.1] (helo=sfs-ml-1.v29.lw.sourceforge.com) by sfs-ml-1.v29.lw.sourceforge.com with esmtp (Exim 4.95) (envelope-from ) id 1rJzi9-000896-OJ; Sun, 31 Dec 2023 17:34:54 +0000 Received: from [172.30.20.202] (helo=mx.sourceforge.net) by sfs-ml-1.v29.lw.sourceforge.com with esmtps (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.95) (envelope-from ) id 1rJzi8-000890-ML for openvpn-devel@lists.sourceforge.net; Sun, 31 Dec 2023 17:34:53 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sourceforge.net; s=x; h=Content-Transfer-Encoding:MIME-Version:References: In-Reply-To:Message-ID:Date:Subject:To:From:Sender:Reply-To:Cc:Content-Type: Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender: Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=oxHaaA6o6zmNe33UA0xn8GlSHILlF/Htvk0UG2FQ2xk=; b=hcTqY9HM1KH3jjLNFlUvsjj9tH EJrRCqJVRLxvmWHpbEJNNXtqhI3uMqIw7tXF3HKxm8WwpIodUwPDz7slKqKvnAMD3IdojPmyNDZFP vcuLEJE1ghx1qlNU22h086bXK4J3Tb6z+shlNpy9EkkHDTAthwVmXCIvxsRfAXi/jXDs=; DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sf.net; s=x ; h=Content-Transfer-Encoding:MIME-Version:References:In-Reply-To:Message-ID: Date:Subject:To:From:Sender:Reply-To:Cc:Content-Type:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=oxHaaA6o6zmNe33UA0xn8GlSHILlF/Htvk0UG2FQ2xk=; b=XGzuhe1Mt49O4lzOqfQlygcQR/ g7UaHqx3c3Q9VH95SLOdW7XgBpOxb6r+gNtcOA8I0gl8BEyiayGeORvzU/Ru6HLcyxYbcSrJn4tyh /uMOo1c7Nc/2GD0CkurBYXNRMTfdKaNZiK89lFW/r0oqUYRoqgoEcNdGHx2Qcrl8wPDQ=; Received: from dhcp-174.greenie.muc.de ([193.149.48.174] helo=blue.greenie.muc.de) by sfi-mx-2.v28.lw.sourceforge.com with esmtps (TLS1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.95) id 1rJzi4-0000fC-QY for openvpn-devel@lists.sourceforge.net; Sun, 31 Dec 2023 17:34:52 +0000 Received: from blue.greenie.muc.de (localhost [127.0.0.1]) by blue.greenie.muc.de (8.17.1.9/8.17.1.9) with ESMTP id 3BVHYWPb031366 for ; Sun, 31 Dec 2023 18:34:32 +0100 Received: (from gert@localhost) by blue.greenie.muc.de (8.17.1.9/8.17.1.9/Submit) id 3BVHYW7w031365 for openvpn-devel@lists.sourceforge.net; Sun, 31 Dec 2023 18:34:32 +0100 From: Gert Doering To: openvpn-devel@lists.sourceforge.net Date: Sun, 31 Dec 2023 18:34:31 +0100 Message-ID: <20231231173431.31356-1-gert@greenie.muc.de> X-Mailer: git-send-email 2.41.0 In-Reply-To: References: MIME-Version: 1.0 X-Spam-Score: -0.0 (/) X-Spam-Report: Spam detection software, running on the system "util-spamd-1.v13.lw.sourceforge.com", has NOT identified this incoming email as spam. The original message has been attached to this so you can view it or label similar future email. If you have any questions, see the administrator of that system for details. Content preview: From: Arne Schwabe The undefined behaviour USAN clang checker found this. This fix is a bit messy but so are the original structures. Content analysis details: (-0.0 points, 6.0 required) pts rule name description ---- ---------------------- -------------------------------------------------- -0.0 SPF_PASS SPF: sender matches SPF record -0.0 SPF_HELO_PASS SPF: HELO matches SPF record -0.0 T_SCC_BODY_TEXT_LINE No description available. X-Headers-End: 1rJzi4-0000fC-QY Subject: [Openvpn-devel] [PATCH v3] Fix unaligned access in macOS, FreeBSD, Solaris hwaddr X-BeenThere: openvpn-devel@lists.sourceforge.net X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: openvpn-devel-bounces@lists.sourceforge.net X-getmail-retrieved-from-mailbox: Inbox X-GMAIL-THRID: =?utf-8?q?1786819762622797779?= X-GMAIL-MSGID: =?utf-8?q?1786819762622797779?= From: Arne Schwabe The undefined behaviour USAN clang checker found this. This fix is a bit messy but so are the original structures. Since the API on Solaris/Illuminos does not return the AF_LINK sockaddr type we are interested in, there is little value in fixing the code on that platform to iterate through a list that does not contain the element we are looking for. Add includes stddef.h for offsetof and integer.h for max_int. Change-Id: Ia797c8801fa9a9bc10b6674efde5fdbd7132e4a8 Signed-off-by: Arne Schwabe Acked-by: Gert Doering --- This change was reviewed on Gerrit and approved by at least one developer. I request to merge it to master. Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/454 This mail reflects revision 3 of this Change. Acked-by according to Gerrit (reflected above): Gert Doering diff --git a/src/openvpn/route.c b/src/openvpn/route.c index 6cc112c..0e6667f 100644 --- a/src/openvpn/route.c +++ b/src/openvpn/route.c @@ -24,6 +24,7 @@ /* * Support routines for adding/deleting network routes. */ +#include #ifdef HAVE_CONFIG_H #include "config.h" @@ -40,6 +41,7 @@ #include "win32.h" #include "options.h" #include "networking.h" +#include "integer.h" #include "memdbg.h" @@ -3639,11 +3641,15 @@ rgi->flags |= RGI_NETMASK_DEFINED; } +#if !defined(TARGET_SOLARIS) + /* Illumos/Solaris does not provide AF_LINK entries when calling the + * SIOCGIFCONF API, so there is little sense to trying to figure out a + * MAC address from an API that does not provide that information */ + /* try to read MAC addr associated with interface that owns default gateway */ if (rgi->flags & RGI_IFACE_DEFINED) { struct ifconf ifc; - struct ifreq *ifr; const int bufsize = 4096; char *buffer; @@ -3668,22 +3674,50 @@ for (cp = buffer; cp <= buffer + ifc.ifc_len - sizeof(struct ifreq); ) { - ifr = (struct ifreq *)cp; -#if defined(TARGET_SOLARIS) - const size_t len = sizeof(ifr->ifr_name) + sizeof(ifr->ifr_addr); -#else - const size_t len = sizeof(ifr->ifr_name) + max(sizeof(ifr->ifr_addr), ifr->ifr_addr.sa_len); -#endif + struct ifreq ifr = { 0 }; + /* this is not always using an 8 byte alignment that struct ifr + * requires. Need to memcpy() to a strict ifr to force 8-byte + * alignment required for member access */ + memcpy(&ifr, cp, sizeof(struct ifreq)); + const size_t len = sizeof(ifr.ifr_name) + max(sizeof(ifr.ifr_addr), ifr.ifr_addr.sa_len); - if (!ifr->ifr_addr.sa_family) + if (!ifr.ifr_addr.sa_family) { break; } - if (!strncmp(ifr->ifr_name, rgi->iface, IFNAMSIZ)) + if (!strncmp(ifr.ifr_name, rgi->iface, IFNAMSIZ)) { - if (ifr->ifr_addr.sa_family == AF_LINK) + if (ifr.ifr_addr.sa_family == AF_LINK) { - struct sockaddr_dl *sdl = (struct sockaddr_dl *)&ifr->ifr_addr; + /* This is a confusing member access on multiple levels. + * + * struct sockaddr_dl is 20 bytes (on macOS and NetBSD, + * larger on other BSDs) in size and has + * 12 bytes space for the Ethernet interface name + * (max 16 bytes) and hw address (6 bytes) + * + * So if the interface name is more than 6 byte, the + * location of hwaddr extends beyond the struct. + * + * This struct is embedded into ifreq that has + * 16 bytes for a sockaddr and also expects this + * struct to potentially extend beyond the bounds of + * the struct. + * + * We only copied 32 bytes (size of ifr at least on macOS + * might differ on other platforms again) from cp to ifr. + * + * But as hwaddr might extend but sdl might extend beyond + * ifr's. So we need recalculate how large the actual size + * of the embedded dl_sock actually is and then also need + * to copy it since it also most likely does not have the + * proper alignment required to access the struct. + */ + const size_t sock_dl_len = max_int((int) (sizeof(struct sockaddr_dl)), + (int) (ifr.ifr_addr.sa_len)); + + struct sockaddr_dl *sdl = gc_malloc(sock_dl_len, true, &gc); + memcpy(sdl, cp + offsetof(struct ifreq, ifr_addr), sock_dl_len); memcpy(rgi->hwaddr, LLADDR(sdl), 6); rgi->flags |= RGI_HWADDR_DEFINED; } @@ -3691,6 +3725,7 @@ cp += len; } } +#endif /* if !defined(TARGET_SOLARIS) */ done: if (sockfd >= 0)