* [edk2-devel] [PATCH v1] SctPkg: Fixed a pinter error in DevicePathFromTextBBTestCoverage.c @ 2024-02-26 10:17 Chao Li 0 siblings, 0 replies; 4+ messages in thread From: Chao Li @ 2024-02-26 10:17 UTC (permalink / raw) To: devel; +Cc: G Edhaya Chandran, Barton Gao, Carolyn Gjertsen DevicePathFromTextBBTextCoverage.c function CreateDNSDeviceNode has a bug, code: SctStrToIPv4Addr (&IpStr1, (EFI_IPv4_ADDRESS *)(DNS + sizeof (DNS_DEVICE_PATH))); DNS is a pointer, which is increased by a structure size and converted to EFI_IPv4_ADDRESS*, which will point to an unknown address. So fix it. BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4712 Cc: G Edhaya Chandran <Edhaya.Chandran@arm.com> Cc: Barton Gao <gaojie@byosoft.com.cn> Cc: Carolyn Gjertsen <Carolyn.Gjertsen@amd.com> Signed-off-by: Chao Li <lichao@loongson.cn> --- .../BlackBoxTest/DevicePathFromTextBBTestCoverage.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/DevicePathFromText/BlackBoxTest/DevicePathFromTextBBTestCoverage.c b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/DevicePathFromText/BlackBoxTest/DevicePathFromTextBBTestCoverage.c index c96ee246..bd11c25a 100644 --- a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/DevicePathFromText/BlackBoxTest/DevicePathFromTextBBTestCoverage.c +++ b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/DevicePathFromText/BlackBoxTest/DevicePathFromTextBBTestCoverage.c @@ -1734,13 +1734,13 @@ CreateDNSDeviceNode ( } if (DNS->IsIPv6 == 0) { - SctStrToIPv4Addr (&IpStr1, (EFI_IPv4_ADDRESS *)(DNS + sizeof (DNS_DEVICE_PATH))); - SctStrToIPv4Addr (&IpStr2, (EFI_IPv4_ADDRESS *)(DNS + sizeof (DNS_DEVICE_PATH) + sizeof(EFI_IP_ADDRESS))); + SctStrToIPv4Addr (&IpStr1, (EFI_IPv4_ADDRESS *)((UINT8 *)DNS + sizeof (DNS_DEVICE_PATH))); + SctStrToIPv4Addr (&IpStr2, (EFI_IPv4_ADDRESS *)((UINT8 *)DNS + sizeof (DNS_DEVICE_PATH) + sizeof(EFI_IP_ADDRESS))); } if (DNS->IsIPv6 == 1) { - SctStrToIPv6Addr (&IpStr1, (EFI_IPv6_ADDRESS *)(DNS + sizeof (DNS_DEVICE_PATH))); - SctStrToIPv6Addr (&IpStr2, (EFI_IPv6_ADDRESS *)(DNS + sizeof (DNS_DEVICE_PATH) + sizeof(EFI_IP_ADDRESS))); + SctStrToIPv6Addr (&IpStr1, (EFI_IPv6_ADDRESS *)((UINT8 *)DNS + sizeof (DNS_DEVICE_PATH))); + SctStrToIPv6Addr (&IpStr2, (EFI_IPv6_ADDRESS *)((UINT8 *)DNS + sizeof (DNS_DEVICE_PATH) + sizeof(EFI_IP_ADDRESS))); } return (EFI_DEVICE_PATH_PROTOCOL *) DNS; -- 2.27.0 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#115947): https://edk2.groups.io/g/devel/message/115947 Mute This Topic: https://groups.io/mt/104579419/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply related [flat|nested] 4+ messages in thread
[parent not found: <20240226101752.247067-1-lichao@...>]
* Re: [edk2-devel] [PATCH v1] SctPkg: Fixed a pinter error in DevicePathFromTextBBTestCoverage.c [not found] <20240226101752.247067-1-lichao@...> @ 2024-02-26 11:02 ` Heinrich Schuchardt 2024-02-27 1:15 ` Chao Li [not found] ` <17B7934ED7B1BC99.684@groups.io> 0 siblings, 2 replies; 4+ messages in thread From: Heinrich Schuchardt @ 2024-02-26 11:02 UTC (permalink / raw) To: Chao Li; +Cc: G Edhaya Chandran, Barton Gao, Carolyn Gjertsen, edk2-devel On 26.02.24 11:17, Chao Li wrote: > DevicePathFromTextBBTextCoverage.c function CreateDNSDeviceNode has a > bug, code: > > SctStrToIPv4Addr (&IpStr1, (EFI_IPv4_ADDRESS *)(DNS + sizeof (DNS_DEVICE_PATH))); > > DNS is a pointer, which is increased by a structure size and converted > to EFI_IPv4_ADDRESS*, which will point to an unknown address. So fix it. Hello Chao, thanks for diving into this. Please, add Fixes: 847e0363e846 ("SctPkg: Fix the UefiSct -Wincompatible-pointer-types warnings") > > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4712 > > Cc: G Edhaya Chandran <Edhaya.Chandran@...> > Cc: Barton Gao <gaojie@...> > Cc: Carolyn Gjertsen <Carolyn.Gjertsen@...> > Signed-off-by: Chao Li <lichao@...> > --- > .../BlackBoxTest/DevicePathFromTextBBTestCoverage.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/DevicePathFromText/BlackBoxTest/DevicePathFromTextBBTestCoverage.c b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/DevicePathFromText/BlackBoxTest/DevicePathFromTextBBTestCoverage.c > index c96ee246..bd11c25a 100644 > --- a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/DevicePathFromText/BlackBoxTest/DevicePathFromTextBBTestCoverage.c > +++ b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/DevicePathFromText/BlackBoxTest/DevicePathFromTextBBTestCoverage.c > @@ -1734,13 +1734,13 @@ CreateDNSDeviceNode ( > } > > if (DNS->IsIPv6 == 0) { > - SctStrToIPv4Addr (&IpStr1, (EFI_IPv4_ADDRESS *)(DNS + sizeof (DNS_DEVICE_PATH))); > - SctStrToIPv4Addr (&IpStr2, (EFI_IPv4_ADDRESS *)(DNS + sizeof (DNS_DEVICE_PATH) + sizeof(EFI_IP_ADDRESS))); > + SctStrToIPv4Addr (&IpStr1, (EFI_IPv4_ADDRESS *)((UINT8 *)DNS + sizeof (DNS_DEVICE_PATH))); > + SctStrToIPv4Addr (&IpStr2, (EFI_IPv4_ADDRESS *)((UINT8 *)DNS + sizeof (DNS_DEVICE_PATH) + sizeof(EFI_IP_ADDRESS))); > } > > if (DNS->IsIPv6 == 1) { > - SctStrToIPv6Addr (&IpStr1, (EFI_IPv6_ADDRESS *)(DNS + sizeof (DNS_DEVICE_PATH))); > - SctStrToIPv6Addr (&IpStr2, (EFI_IPv6_ADDRESS *)(DNS + sizeof (DNS_DEVICE_PATH) + sizeof(EFI_IP_ADDRESS))); > + SctStrToIPv6Addr (&IpStr1, (EFI_IPv6_ADDRESS *)((UINT8 *)DNS + sizeof (DNS_DEVICE_PATH))); > + SctStrToIPv6Addr (&IpStr2, (EFI_IPv6_ADDRESS *)((UINT8 *)DNS + sizeof (DNS_DEVICE_PATH) + sizeof(EFI_IP_ADDRESS))); The change looks correct. I would prefer using the fields of the DNS_DEVICE_PATH typedef from MdePkg/Include/Protocol/DevicePath.h to avoid the conversions: > + SctStrToIPv4Addr (&IpStr1, &DNS->DnsServerIp[0].v4) > + SctStrToIPv4Addr (&IpStr2, &DNS->DnsServerIp[1].v4) > + SctStrToIPv6Addr (&IpStr1, &DNS->DnsServerIp[0].v6) > + SctStrToIPv6Addr (&IpStr2, &DNS->DnsServerIp[1].v6) Best regards Heinrich > } > > return (EFI_DEVICE_PATH_PROTOCOL *) DNS; > -- > 2.27.0 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#115949): https://edk2.groups.io/g/devel/message/115949 Mute This Topic: https://groups.io/mt/104579419/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [edk2-devel] [PATCH v1] SctPkg: Fixed a pinter error in DevicePathFromTextBBTestCoverage.c 2024-02-26 11:02 ` Heinrich Schuchardt @ 2024-02-27 1:15 ` Chao Li [not found] ` <17B7934ED7B1BC99.684@groups.io> 1 sibling, 0 replies; 4+ messages in thread From: Chao Li @ 2024-02-27 1:15 UTC (permalink / raw) To: devel, heinrich.schuchardt Cc: G Edhaya Chandran, Barton Gao, Carolyn Gjertsen [-- Attachment #1: Type: text/plain, Size: 3625 bytes --] Hi Heinrich, Thanks, Chao On 2024/2/26 19:02, Heinrich Schuchardt wrote: > On 26.02.24 11:17, Chao Li wrote: >> DevicePathFromTextBBTextCoverage.c function CreateDNSDeviceNode has a >> bug, code: >> >> SctStrToIPv4Addr (&IpStr1, (EFI_IPv4_ADDRESS *)(DNS + sizeof >> (DNS_DEVICE_PATH))); >> >> DNS is a pointer, which is increased by a structure size and converted >> to EFI_IPv4_ADDRESS*, which will point to an unknown address. So fix it. > > Hello Chao, > > thanks for diving into this. > > Please, add > > Fixes: 847e0363e846 ("SctPkg: Fix the UefiSct > -Wincompatible-pointer-types warnings") > >> >> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4712 >> >> Cc: G Edhaya Chandran <Edhaya.Chandran@...> >> Cc: Barton Gao <gaojie@...> >> Cc: Carolyn Gjertsen <Carolyn.Gjertsen@...> >> Signed-off-by: Chao Li <lichao@...> >> --- >> .../BlackBoxTest/DevicePathFromTextBBTestCoverage.c | 8 ++++---- >> 1 file changed, 4 insertions(+), 4 deletions(-) >> >> diff --git >> a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/DevicePathFromText/BlackBoxTest/DevicePathFromTextBBTestCoverage.c >> b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/DevicePathFromText/BlackBoxTest/DevicePathFromTextBBTestCoverage.c >> >> index c96ee246..bd11c25a 100644 >> --- >> a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/DevicePathFromText/BlackBoxTest/DevicePathFromTextBBTestCoverage.c >> +++ >> b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/DevicePathFromText/BlackBoxTest/DevicePathFromTextBBTestCoverage.c >> @@ -1734,13 +1734,13 @@ CreateDNSDeviceNode ( >> } >> >> if (DNS->IsIPv6 == 0) { >> - SctStrToIPv4Addr (&IpStr1, (EFI_IPv4_ADDRESS *)(DNS + sizeof >> (DNS_DEVICE_PATH))); >> - SctStrToIPv4Addr (&IpStr2, (EFI_IPv4_ADDRESS *)(DNS + sizeof >> (DNS_DEVICE_PATH) + sizeof(EFI_IP_ADDRESS))); >> + SctStrToIPv4Addr (&IpStr1, (EFI_IPv4_ADDRESS *)((UINT8 *)DNS + >> sizeof (DNS_DEVICE_PATH))); >> + SctStrToIPv4Addr (&IpStr2, (EFI_IPv4_ADDRESS *)((UINT8 *)DNS + >> sizeof (DNS_DEVICE_PATH) + sizeof(EFI_IP_ADDRESS))); >> } >> >> if (DNS->IsIPv6 == 1) { >> - SctStrToIPv6Addr (&IpStr1, (EFI_IPv6_ADDRESS *)(DNS + sizeof >> (DNS_DEVICE_PATH))); >> - SctStrToIPv6Addr (&IpStr2, (EFI_IPv6_ADDRESS *)(DNS + sizeof >> (DNS_DEVICE_PATH) + sizeof(EFI_IP_ADDRESS))); >> + SctStrToIPv6Addr (&IpStr1, (EFI_IPv6_ADDRESS *)((UINT8 *)DNS + >> sizeof (DNS_DEVICE_PATH))); >> + SctStrToIPv6Addr (&IpStr2, (EFI_IPv6_ADDRESS *)((UINT8 *)DNS + >> sizeof (DNS_DEVICE_PATH) + sizeof(EFI_IP_ADDRESS))); > > The change looks correct. > > I would prefer using the fields of the DNS_DEVICE_PATH typedef from > MdePkg/Include/Protocol/DevicePath.h to avoid the conversions: > > > + SctStrToIPv4Addr (&IpStr1, &DNS->DnsServerIp[0].v4) > > + SctStrToIPv4Addr (&IpStr2, &DNS->DnsServerIp[1].v4) > > > + SctStrToIPv6Addr (&IpStr1, &DNS->DnsServerIp[0].v6) > > + SctStrToIPv6Addr (&IpStr2, &DNS->DnsServerIp[1].v6) I also prefer this way, but the definition of DNS in UEFI/Protocol/DevicePath.h is not same as the MdePkg version, should we make them the same? > > Best regards > > Heinrich > >> } >> >> return (EFI_DEVICE_PATH_PROTOCOL *) DNS; >> -- >> 2.27.0 > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#115995): https://edk2.groups.io/g/devel/message/115995 Mute This Topic: https://groups.io/mt/104579419/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- [-- Attachment #2: Type: text/html, Size: 6471 bytes --] ^ permalink raw reply [flat|nested] 4+ messages in thread
[parent not found: <17B7934ED7B1BC99.684@groups.io>]
* Re: [edk2-devel] [PATCH v1] SctPkg: Fixed a pinter error in DevicePathFromTextBBTestCoverage.c [not found] ` <17B7934ED7B1BC99.684@groups.io> @ 2024-02-27 6:34 ` Chao Li 0 siblings, 0 replies; 4+ messages in thread From: Chao Li @ 2024-02-27 6:34 UTC (permalink / raw) To: devel, heinrich.schuchardt Cc: G Edhaya Chandran, Barton Gao, Carolyn Gjertsen [-- Attachment #1: Type: text/plain, Size: 3950 bytes --] Hi Heinrich, I guess it is not allowed to make them the same now, so I submitted V2, added your recomended fix message to the commit message, and also added more reviewers. Thanks, Chao On 2024/2/27 09:15, Chao Li wrote: > > Hi Heinrich, > > On 2024/2/26 19:02, Heinrich Schuchardt wrote: >> On 26.02.24 11:17, Chao Li wrote: >>> DevicePathFromTextBBTextCoverage.c function CreateDNSDeviceNode has a >>> bug, code: >>> >>> SctStrToIPv4Addr (&IpStr1, (EFI_IPv4_ADDRESS *)(DNS + sizeof >>> (DNS_DEVICE_PATH))); >>> >>> DNS is a pointer, which is increased by a structure size and converted >>> to EFI_IPv4_ADDRESS*, which will point to an unknown address. So fix >>> it. >> >> Hello Chao, >> >> thanks for diving into this. >> >> Please, add >> >> Fixes: 847e0363e846 ("SctPkg: Fix the UefiSct >> -Wincompatible-pointer-types warnings") >> >>> >>> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4712 >>> >>> Cc: G Edhaya Chandran <Edhaya.Chandran@...> >>> Cc: Barton Gao <gaojie@...> >>> Cc: Carolyn Gjertsen <Carolyn.Gjertsen@...> >>> Signed-off-by: Chao Li <lichao@...> >>> --- >>> .../BlackBoxTest/DevicePathFromTextBBTestCoverage.c | 8 >>> ++++---- >>> 1 file changed, 4 insertions(+), 4 deletions(-) >>> >>> diff --git >>> a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/DevicePathFromText/BlackBoxTest/DevicePathFromTextBBTestCoverage.c >>> b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/DevicePathFromText/BlackBoxTest/DevicePathFromTextBBTestCoverage.c >>> >>> index c96ee246..bd11c25a 100644 >>> --- >>> a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/DevicePathFromText/BlackBoxTest/DevicePathFromTextBBTestCoverage.c >>> +++ >>> b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/DevicePathFromText/BlackBoxTest/DevicePathFromTextBBTestCoverage.c >>> @@ -1734,13 +1734,13 @@ CreateDNSDeviceNode ( >>> } >>> >>> if (DNS->IsIPv6 == 0) { >>> - SctStrToIPv4Addr (&IpStr1, (EFI_IPv4_ADDRESS *)(DNS + sizeof >>> (DNS_DEVICE_PATH))); >>> - SctStrToIPv4Addr (&IpStr2, (EFI_IPv4_ADDRESS *)(DNS + sizeof >>> (DNS_DEVICE_PATH) + sizeof(EFI_IP_ADDRESS))); >>> + SctStrToIPv4Addr (&IpStr1, (EFI_IPv4_ADDRESS *)((UINT8 *)DNS + >>> sizeof (DNS_DEVICE_PATH))); >>> + SctStrToIPv4Addr (&IpStr2, (EFI_IPv4_ADDRESS *)((UINT8 *)DNS + >>> sizeof (DNS_DEVICE_PATH) + sizeof(EFI_IP_ADDRESS))); >>> } >>> >>> if (DNS->IsIPv6 == 1) { >>> - SctStrToIPv6Addr (&IpStr1, (EFI_IPv6_ADDRESS *)(DNS + sizeof >>> (DNS_DEVICE_PATH))); >>> - SctStrToIPv6Addr (&IpStr2, (EFI_IPv6_ADDRESS *)(DNS + sizeof >>> (DNS_DEVICE_PATH) + sizeof(EFI_IP_ADDRESS))); >>> + SctStrToIPv6Addr (&IpStr1, (EFI_IPv6_ADDRESS *)((UINT8 *)DNS + >>> sizeof (DNS_DEVICE_PATH))); >>> + SctStrToIPv6Addr (&IpStr2, (EFI_IPv6_ADDRESS *)((UINT8 *)DNS + >>> sizeof (DNS_DEVICE_PATH) + sizeof(EFI_IP_ADDRESS))); >> >> The change looks correct. >> >> I would prefer using the fields of the DNS_DEVICE_PATH typedef from >> MdePkg/Include/Protocol/DevicePath.h to avoid the conversions: >> >> > + SctStrToIPv4Addr (&IpStr1, &DNS->DnsServerIp[0].v4) >> > + SctStrToIPv4Addr (&IpStr2, &DNS->DnsServerIp[1].v4) >> >> > + SctStrToIPv6Addr (&IpStr1, &DNS->DnsServerIp[0].v6) >> > + SctStrToIPv6Addr (&IpStr2, &DNS->DnsServerIp[1].v6) > I also prefer this way, but the definition of DNS in > UEFI/Protocol/DevicePath.h is not same as the MdePkg version, should > we make them the same? >> >> Best regards >> >> Heinrich >> >>> } >>> >>> return (EFI_DEVICE_PATH_PROTOCOL *) DNS; >>> -- >>> 2.27.0 >> >> >> >> >> > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#116005): https://edk2.groups.io/g/devel/message/116005 Mute This Topic: https://groups.io/mt/104579419/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- [-- Attachment #2: Type: text/html, Size: 7118 bytes --] ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-02-27 6:39 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-02-26 10:17 [edk2-devel] [PATCH v1] SctPkg: Fixed a pinter error in DevicePathFromTextBBTestCoverage.c Chao Li
[not found] <20240226101752.247067-1-lichao@...>
2024-02-26 11:02 ` Heinrich Schuchardt
2024-02-27 1:15 ` Chao Li
[not found] ` <17B7934ED7B1BC99.684@groups.io>
2024-02-27 6:34 ` Chao Li
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox