From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by spool.mail.gandi.net (Postfix) with ESMTPS id F1C82D8002E for ; Sun, 25 Feb 2024 15:02:03 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=dfGWLCX+3YMnGgRn7msEUCIpfatB8rPbb4gstzziNTs=; c=relaxed/simple; d=groups.io; h=Message-ID:Date:MIME-Version:Subject:To:References:From:Cc:In-Reply-To:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Language:Content-Type:Content-Transfer-Encoding; s=20140610; t=1708873322; v=1; b=d+s4CTO0/m3n6T9iPi+8SXM8l7paWUJD26qfp0xJVSQ1lJguk898Uq8ILiXKNXtlTYkPZz4H yEVAyJJDDQaUkWILXggXQXpx9D1efd2jKieP0EKqkJHYx5XYpQpmpBMjjYxVBbMVfEaxHHd+SMz s9B/IiCa1HlpJGbYQebjoj0k= X-Received: by 127.0.0.2 with SMTP id yiJtYY7687511xmnq7U4jA06; Sun, 25 Feb 2024 07:02:02 -0800 X-Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by mx.groups.io with SMTP id smtpd.web11.40714.1708873321819247151 for ; Sun, 25 Feb 2024 07:02:01 -0800 X-Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-145-ZscqKl2xMaqiYvG3aNgG2w-1; Sun, 25 Feb 2024 10:01:56 -0500 X-MC-Unique: ZscqKl2xMaqiYvG3aNgG2w-1 X-Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (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 8E3C0830DCC; Sun, 25 Feb 2024 15:01:56 +0000 (UTC) X-Received: from [10.39.192.57] (unknown [10.39.192.57]) by smtp.corp.redhat.com (Postfix) with ESMTPS id B34102026D06; Sun, 25 Feb 2024 15:01:55 +0000 (UTC) Message-ID: Date: Sun, 25 Feb 2024 16:01:54 +0100 MIME-Version: 1.0 Subject: Re: [edk2-devel] edk2-test: bug in DevicePathFromTextBBTestCoverage.c To: devel@edk2.groups.io, chip.programmer@att.net References: <73061DCB4B4D46209875F9B9B0F87C84.ref@DESKTOPQUG2G9K> <73061DCB4B4D46209875F9B9B0F87C84@DESKTOPQUG2G9K> From: "Laszlo Ersek" Cc: Abdul Lateef Attar , G Edhaya Chandran In-Reply-To: <73061DCB4B4D46209875F9B9B0F87C84@DESKTOPQUG2G9K> X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.4 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Precedence: Bulk List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Reply-To: devel@edk2.groups.io,lersek@redhat.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: X-Gm-Message-State: xESZ411MMF59YMDhlxHiBJrYx7686176AA= Content-Language: en-US Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20140610 header.b=d+s4CTO0; spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce@groups.io; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=redhat.com (policy=none) Hello Charles, On 2/24/24 17:01, Charles Hyde wrote: > There is a bug in the following source file: > > uefi-sct\ > SctPkg\ > TestCase\ > UEFI\ > EFI\ > Protocol\ > DevicePathFromText\ > BlackBoxTest\ > DevicePathFromTextBBTestCoverage.c > > > Lines 1737, 1738, 1742, and 1743 reference DNS without any cast > override, which causes the pointer reference logic to be wrong, > leading the functions SctStrToIPv4Addr and SctStrToIPv6Addr to cause > memory corruption. The DNS reference needs to have (UINT8 *) cast > override, like is done in DevicePathToTextBBTestMain.c. You are correct, as far as I can tell. This is a regression from: commit 847e0363e846296881c238dc43766fd40f6c2aec Author: Abdul Lateef Attar Date: Thu Jun 9 16:24:02 2022 +0530 SctPkg: Fix the UefiSct -Wincompatible-pointer-types warnings REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D2126 Fixes the incompatible pointer types warning for UefiSct package. Cc: G Edhaya Chandran Cc: Barton Gao Cc: Carolyn Gjertsen Cc: Samer El-Haj-Mahmoud Cc: Eric Jin Cc: Arvin Chen Cc: Supreeth Venkatesh Signed-off-by: Abdul Lateef Attar 500 files changed, 5537 insertions(+), 1162 deletions(-) Unfortunately, the change for "DevicePathFromTextBBTestCoverage.c" in this -- *way* oversized -- patch was incorrect: > @@ -1734,13 +1734,13 @@ CreateDNSDeviceNode ( > } > > if (DNS->IsIPv6 =3D=3D 0) { > - SctStrToIPv4Addr (&IpStr1, (UINT8 *)DNS + sizeof (DNS_DEVICE_PATH)); > - SctStrToIPv4Addr (&IpStr2, (UINT8 *)DNS + sizeof (DNS_DEVICE_PATH) += sizeof(EFI_IP_ADDRESS)); > + SctStrToIPv4Addr (&IpStr1, (EFI_IPv4_ADDRESS *)(DNS + sizeof (DNS_DE= VICE_PATH))); > + SctStrToIPv4Addr (&IpStr2, (EFI_IPv4_ADDRESS *)(DNS + sizeof (DNS_DE= VICE_PATH) + sizeof(EFI_IP_ADDRESS))); > } > > if (DNS->IsIPv6 =3D=3D 1) { > - SctStrToIPv6Addr (&IpStr1, (UINT8 *)DNS + sizeof (DNS_DEVICE_PATH)); > - SctStrToIPv6Addr (&IpStr2, (UINT8 *)DNS + sizeof (DNS_DEVICE_PATH) += sizeof(EFI_IP_ADDRESS)); > + SctStrToIPv6Addr (&IpStr1, (EFI_IPv6_ADDRESS *)(DNS + sizeof (DNS_DE= VICE_PATH))); > + SctStrToIPv6Addr (&IpStr2, (EFI_IPv6_ADDRESS *)(DNS + sizeof (DNS_DE= VICE_PATH) + sizeof(EFI_IP_ADDRESS))); > } > > return (EFI_DEVICE_PATH_PROTOCOL *) DNS; The original (UINT8*) cast was correct for determining the "IPv4Addr" and "IPv6Addr" argument *values* for SctStrToIPv4Addr() and SctStrToIPv6Addr(), respectively; the problem was with the argument *types*. The patch added the necessary outer cast, but incorrectly removed the still needed inner cast. ... The original code was in poor style to begin with. The following should work, without any casts at all: SctStrToIPv4Addr (&IpStr1, &DNS->DnsServerIp[0].v4); SctStrToIPv4Addr (&IpStr2, &DNS->DnsServerIp[1].v4); SctStrToIPv6Addr (&IpStr1, &DNS->DnsServerIp[0].v6); SctStrToIPv6Addr (&IpStr2, &DNS->DnsServerIp[1].v6); That's because DNS is of type pointer-to-DNS_DEVICE_PATH, and DNS_DEVICE_PATH is defined as follows, in "MdePkg/Include/Protocol/DevicePath.h": > #pragma pack(1) > ... > typedef struct { > EFI_DEVICE_PATH_PROTOCOL Header; > /// > /// Indicates the DNS server address is IPv4 or IPv6 address. > /// > UINT8 IsIPv6; > /// > /// Instance of the DNS server address. > /// > EFI_IP_ADDRESS DnsServerIp[]; > } DNS_DEVICE_PATH; > ... > #pragma pack() and "EFI_IP_ADDRESS" is defined in "MdePkg/Include/Uefi/UefiBaseType.h" as follows: > typedef union { > UINT32 Addr[4]; > EFI_IPv4_ADDRESS v4; > EFI_IPv6_ADDRESS v6; > } EFI_IP_ADDRESS; The "DnsServerIp" flexible array member should simply be put to use. AbduL, can you please post a fix? Thanks Laszlo -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#115930): https://edk2.groups.io/g/devel/message/115930 Mute This Topic: https://groups.io/mt/104548292/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-