* [PATCH 1/1] uefi-sct/SctPkg: NULL deref in DevicePathToText test
@ 2020-10-29 20:01 Heinrich Schuchardt
2020-11-04 15:52 ` [edk2-devel] " G Edhaya Chandran
2020-11-24 16:07 ` G Edhaya Chandran
0 siblings, 2 replies; 8+ messages in thread
From: Heinrich Schuchardt @ 2020-10-29 20:01 UTC (permalink / raw)
To: Samer El-Haj-Mahmoud, Eric Jin, G Edhaya Chandran,
EDK II Development
Cc: Grant Likely, Heinrich Schuchardt
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3029
Function DevicePathToTextConvertDeviceNodeToTextCoverageTest() tests if
DeviceNodeToText() correctly converts a Relative Offset Range node. After
calling SctConvertTextToDeviceNode() it tries to set the field Reserved
of the returned device node to 0.
If the tested firmware does not return the expected text
SctConvertTextToDeviceNode() may return NULL or a device node that is
shorter than expected. In both cases it is not possible to access the
field Reserved.
So we must check both that the returned node is not NULL and that it has
the exepected size.
Due to the missing check a NULL dereference was observed when running the
SCT on U-Boot.
Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
.../BlackBoxTest/DevicePathToTextBBTestCoverage.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/DevicePathToText/BlackBoxTest/DevicePathToTextBBTestCoverage.c b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/DevicePathToText/BlackBoxTest/DevicePathToTextBBTestCoverage.c
index ee91bdfb..784d4748 100644
--- a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/DevicePathToText/BlackBoxTest/DevicePathToTextBBTestCoverage.c
+++ b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/DevicePathToText/BlackBoxTest/DevicePathToTextBBTestCoverage.c
@@ -1198,8 +1198,12 @@ DevicePathToTextConvertDeviceNodeToTextCoverageTest (
((MEDIA_OFFSET_DEVICE_PATH *)pDeviceNode1)->EndingOffset = 0x1234;
Text = DevicePathToText->ConvertDeviceNodeToText (pDeviceNode1, FALSE, FALSE);
pDeviceNode2 = SctConvertTextToDeviceNode(Text);
- ((MEDIA_OFFSET_DEVICE_PATH *)pDeviceNode2)->Reserved = 0;
-
+ SctPrint(L"pDeviceNode2 = %p\n", pDeviceNode2);
+ if (pDeviceNode2 &&
+ ((MEDIA_OFFSET_DEVICE_PATH *)pDeviceNode2)->Length ==
+ sizeof(MEDIA_OFFSET_DEVICE_PATH)) {
+ ((MEDIA_OFFSET_DEVICE_PATH *)pDeviceNode2)->Reserved = 0;
+ }
if ((pDeviceNode2 != NULL) && (SctCompareMem (pDeviceNode2, pDeviceNode1, SctDevicePathNodeLength(pDeviceNode1)) == 0)) {
AssertionType = EFI_TEST_ASSERTION_PASSED;
} else {
--
2.28.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [edk2-devel] [PATCH 1/1] uefi-sct/SctPkg: NULL deref in DevicePathToText test
2020-10-29 20:01 [PATCH 1/1] uefi-sct/SctPkg: NULL deref in DevicePathToText test Heinrich Schuchardt
@ 2020-11-04 15:52 ` G Edhaya Chandran
2020-11-23 10:25 ` Grant Likely
2020-11-24 16:07 ` G Edhaya Chandran
1 sibling, 1 reply; 8+ messages in thread
From: G Edhaya Chandran @ 2020-11-04 15:52 UTC (permalink / raw)
To: Heinrich Schuchardt, devel
[-- Attachment #1: Type: text/plain, Size: 57 bytes --]
Reviewed-by: G Edhaya Chandran<edhaya.chandran@arm.com>
[-- Attachment #2: Type: text/html, Size: 492 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [edk2-devel] [PATCH 1/1] uefi-sct/SctPkg: NULL deref in DevicePathToText test
2020-10-29 20:01 [PATCH 1/1] uefi-sct/SctPkg: NULL deref in DevicePathToText test Heinrich Schuchardt
2020-11-04 15:52 ` [edk2-devel] " G Edhaya Chandran
@ 2020-11-24 16:07 ` G Edhaya Chandran
2020-11-24 16:26 ` Heinrich Schuchardt
1 sibling, 1 reply; 8+ messages in thread
From: G Edhaya Chandran @ 2020-11-24 16:07 UTC (permalink / raw)
To: Heinrich Schuchardt, devel
[-- Attachment #1: Type: text/plain, Size: 115 bytes --]
Upstreamed by commit-id : https://github.com/tianocore/edk2-test/commit/75c92b85bf9bb82a7049bf44f40a2741f963230c
[-- Attachment #2: Type: text/html, Size: 119 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [edk2-devel] [PATCH 1/1] uefi-sct/SctPkg: NULL deref in DevicePathToText test
2020-11-24 16:07 ` G Edhaya Chandran
@ 2020-11-24 16:26 ` Heinrich Schuchardt
2020-11-24 16:33 ` G Edhaya Chandran
0 siblings, 1 reply; 8+ messages in thread
From: Heinrich Schuchardt @ 2020-11-24 16:26 UTC (permalink / raw)
To: G Edhaya Chandran, devel
On 11/24/20 5:07 PM, G Edhaya Chandran wrote:
> Upstreamed by commit-id
> : https://github.com/tianocore/edk2-test/commit/75c92b85bf9bb82a7049bf44f40a2741f963230c
>
Hello Edhaya,
it seems a SctPrint() statement slipped into the patch which was only
used for debugging.
Should I open a new issue? Or just send a patch removing the SctPrint
with reference to #3029?
Best regards
Heinrich
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [edk2-devel] [PATCH 1/1] uefi-sct/SctPkg: NULL deref in DevicePathToText test
2020-11-24 16:26 ` Heinrich Schuchardt
@ 2020-11-24 16:33 ` G Edhaya Chandran
2020-11-24 17:08 ` Heinrich Schuchardt
0 siblings, 1 reply; 8+ messages in thread
From: G Edhaya Chandran @ 2020-11-24 16:33 UTC (permalink / raw)
To: Heinrich Schuchardt, devel@edk2.groups.io
Hi Heinrich,
Oh okay. Actually, I have already upstreamed this.
I suggest, I will remove the debug SctPrint myself as part of another upstream.
Is it okay?
With Warm Regards,
Edhay
> -----Original Message-----
> From: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Sent: 24 November 2020 21:57
> To: G Edhaya Chandran <Edhaya.Chandran@arm.com>; devel@edk2.groups.io
> Subject: Re: [edk2-devel] [PATCH 1/1] uefi-sct/SctPkg: NULL deref in
> DevicePathToText test
>
> On 11/24/20 5:07 PM, G Edhaya Chandran wrote:
> > Upstreamed by commit-id
> > :
> > https://github.com/tianocore/edk2-test/commit/75c92b85bf9bb82a7049bf44
> > f40a2741f963230c
> >
>
> Hello Edhaya,
>
> it seems a SctPrint() statement slipped into the patch which was only used for
> debugging.
>
> Should I open a new issue? Or just send a patch removing the SctPrint with
> reference to #3029?
>
> Best regards
>
> Heinrich
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [edk2-devel] [PATCH 1/1] uefi-sct/SctPkg: NULL deref in DevicePathToText test
2020-11-24 16:33 ` G Edhaya Chandran
@ 2020-11-24 17:08 ` Heinrich Schuchardt
2020-11-25 6:40 ` G Edhaya Chandran
0 siblings, 1 reply; 8+ messages in thread
From: Heinrich Schuchardt @ 2020-11-24 17:08 UTC (permalink / raw)
To: G Edhaya Chandran, devel@edk2.groups.io
On 11/24/20 5:33 PM, G Edhaya Chandran wrote:
> Hi Heinrich,
>
> Oh okay. Actually, I have already upstreamed this.
> I suggest, I will remove the debug SctPrint myself as part of another upstream.
>
> Is it okay?
>
> With Warm Regards,
> Edhay
Thanks for taking care.
Best regards
Heinrich
>
>
>> -----Original Message-----
>> From: Heinrich Schuchardt <xypron.glpk@gmx.de>
>> Sent: 24 November 2020 21:57
>> To: G Edhaya Chandran <Edhaya.Chandran@arm.com>; devel@edk2.groups.io
>> Subject: Re: [edk2-devel] [PATCH 1/1] uefi-sct/SctPkg: NULL deref in
>> DevicePathToText test
>>
>> On 11/24/20 5:07 PM, G Edhaya Chandran wrote:
>>> Upstreamed by commit-id
>>> :
>>> https://github.com/tianocore/edk2-test/commit/75c92b85bf9bb82a7049bf44
>>> f40a2741f963230c
>>>
>>
>> Hello Edhaya,
>>
>> it seems a SctPrint() statement slipped into the patch which was only used for
>> debugging.
>>
>> Should I open a new issue? Or just send a patch removing the SctPrint with
>> reference to #3029?
>>
>> Best regards
>>
>> Heinrich
> IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [edk2-devel] [PATCH 1/1] uefi-sct/SctPkg: NULL deref in DevicePathToText test
2020-11-24 17:08 ` Heinrich Schuchardt
@ 2020-11-25 6:40 ` G Edhaya Chandran
0 siblings, 0 replies; 8+ messages in thread
From: G Edhaya Chandran @ 2020-11-25 6:40 UTC (permalink / raw)
To: Heinrich Schuchardt, devel@edk2.groups.io
[-- Attachment #1.1: Type: text/plain, Size: 4257 bytes --]
Hi Heinrich,
This part is giving a build error, can you please check.
https://github.com/tianocore/edk2-test/commit/75c92b85bf9bb82a7049bf44f40a2741f963230c#diff-7f0df1676a2f898b39de5685efb7a296a7f08e5e5ccb4aaec0a5fdebea926727
[cid:image001.jpg@01D6C323.F7844200]
/home/zubmoh01/Work/sct/SctPkg/TestCase/UEFI/EFI/Protocol/DevicePathToText/BlackBoxTest/DevicePathToTextBBTestCoverage.c: In function ‘DevicePathToTextConvertDeviceNodeToTextCoverageTest’:
/home/zubmoh01/Work/sct/SctPkg/TestCase/UEFI/EFI/Protocol/DevicePathToText/BlackBoxTest/DevicePathToTextBBTestCoverage.c:1203:49: error: ‘MEDIA_OFFSET_DEVICE_PATH {aka struct <anonymous>}’ has no member named ‘Length’
((MEDIA_OFFSET_DEVICE_PATH *)pDeviceNode2)->Length ==
^~
"aarch64-linux-gnu-gcc" -MMD -MF /home/zubmoh01/Work/sct/Build/UefiSct/RELEASE_GCC5/AARCH64/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxTest/Dependency/CombinationImage2/CombinationImage2/OUTPUT/ProtocolDefinition.obj.deps -g -Os -fshort-wchar -fno-builtin -fno-strict-aliasing -Wall -Werror -Wno-array-bounds -include AutoGen.h -fno-common -ffunction-sections -fdata-sections -DSTRING_ARRAY_NAME=ImageServices_CombinationImage2Strings -g -Os -fshort-wchar -fno-builtin -fno-strict-aliasing -Wall -Werror -Wno-array-bounds -include AutoGen.h -fno-common -mlittle-endian -fno-short-enums -fverbose-asm -funsigned-char -ffunction-sections -fdata-sections -Wno-address -fno-asynchronous-unwind-tables -fno-unwind-tables -fno-pic -fno-pie -ffixed-x18 -mcmodel=small -flto -Wno-unused-but-set-variable -Wno-unused-const-variable -D EFIAARCH64 -D EFIAARCH64 -ffreestanding -nostdinc -nostdlib -Wno-error=unused-function -Wno-error=unused-but-set-variable -Wno-error -DMDEPKG_NDEBUG -c -o
With Warm Regards,
Edhay
> -----Original Message-----
> From: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Sent: 24 November 2020 22:38
> To: G Edhaya Chandran <Edhaya.Chandran@arm.com>; devel@edk2.groups.io
> Subject: Re: [edk2-devel] [PATCH 1/1] uefi-sct/SctPkg: NULL deref in
> DevicePathToText test
>
> On 11/24/20 5:33 PM, G Edhaya Chandran wrote:
> > Hi Heinrich,
> >
> > Oh okay. Actually, I have already upstreamed this.
> > I suggest, I will remove the debug SctPrint myself as part of another upstream.
> >
> > Is it okay?
> >
> > With Warm Regards,
> > Edhay
>
> Thanks for taking care.
>
> Best regards
>
> Heinrich
>
> >
> >
> >> -----Original Message-----
> >> From: Heinrich Schuchardt <xypron.glpk@gmx.de<mailto:xypron.glpk@gmx.de>>
> >> Sent: 24 November 2020 21:57
> >> To: G Edhaya Chandran <Edhaya.Chandran@arm.com<mailto:Edhaya.Chandran@arm.com>>;
> devel@edk2.groups.io<mailto:devel@edk2.groups.io>
> >> Subject: Re: [edk2-devel] [PATCH 1/1] uefi-sct/SctPkg: NULL deref in
> >> DevicePathToText test
> >>
> >> On 11/24/20 5:07 PM, G Edhaya Chandran wrote:
> >>> Upstreamed by commit-id
> >>> :
> >>> https://github.com/tianocore/edk2-test/commit/75c92b85bf9bb82a7049bf
> >>> 44
> >>> f40a2741f963230c
> >>>
> >>
> >> Hello Edhaya,
> >>
> >> it seems a SctPrint() statement slipped into the patch which was only
> >> used for debugging.
> >>
> >> Should I open a new issue? Or just send a patch removing the SctPrint
> >> with reference to #3029?
> >>
> >> Best regards
> >>
> >> Heinrich
> > IMPORTANT NOTICE: The contents of this email and any attachments are
> confidential and may also be privileged. If you are not the intended recipient,
> please notify the sender immediately and do not disclose the contents to any
> other person, use it for any purpose, or store or copy the information in any
> medium. Thank you.
> >
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
[-- Attachment #1.2: Type: text/html, Size: 10874 bytes --]
[-- Attachment #2: image001.jpg --]
[-- Type: image/jpeg, Size: 29665 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-11-25 6:40 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-10-29 20:01 [PATCH 1/1] uefi-sct/SctPkg: NULL deref in DevicePathToText test Heinrich Schuchardt
2020-11-04 15:52 ` [edk2-devel] " G Edhaya Chandran
2020-11-23 10:25 ` Grant Likely
2020-11-24 16:07 ` G Edhaya Chandran
2020-11-24 16:26 ` Heinrich Schuchardt
2020-11-24 16:33 ` G Edhaya Chandran
2020-11-24 17:08 ` Heinrich Schuchardt
2020-11-25 6:40 ` G Edhaya Chandran
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox