BaseTools/VolInfo: Correct buffer for GenCrc32 tool

[edk2-devel] [PATCH] BaseTools/VolInfo: Correct buffer for GenCrc32 tool

Posted by Konstantin Aladyshev 1 year, 9 months ago

If the guided section was encoded with GenCrc32 tool the resulting
'EFI_GUID_DEFINED_SECTION.DataOffset' field points to the start of
the meaningfull data that follows the CRC32 value.
But if we want to decode the section with GenCrc32 tool we need to
provide a buffer that includes the CRC32 value itself.

Signed-off-by: Konstantin Aladyshev <aladyshev22@gmail.com>
---
 BaseTools/Source/C/VolInfo/VolInfo.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/BaseTools/Source/C/VolInfo/VolInfo.c b/BaseTools/Source/C/VolInfo/VolInfo.c
index f450796f9c..47a8f80f46 100644
--- a/BaseTools/Source/C/VolInfo/VolInfo.c
+++ b/BaseTools/Source/C/VolInfo/VolInfo.c
@@ -1999,6 +1999,13 @@ Returns:
           );
         free (ExtractionTool);
 
+        if (!CompareGuid (
+               EfiGuid,
+               &gEfiCrc32GuidedSectionExtractionProtocolGuid
+               )
+           ) {
+          DataOffset -= 4;
+        }
         Status =
           PutFileImage (
             ToolInputFile,
-- 
2.25.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#91530): https://edk2.groups.io/g/devel/message/91530
Mute This Topic: https://groups.io/mt/92482555/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] BaseTools/VolInfo: Correct buffer for GenCrc32 tool

Posted by Yuwei Chen 1 year, 9 months ago

Hi Konstantin, this is really an issue need to be solved.

When GenSec tool uses the guidtools to encode the section, it differentiates how GenCrc32 is used from other tools..

GenSec defines a CRC32_SECTION_HEADER structure for GenCrc32, and calculate the DataOffset of the Section with this header.

However other guidtools just use the EFI_GUID_DEFINED_SECTION header and calculate the DataOffset with it.



"
typedef struct {
  EFI_GUID_DEFINED_SECTION  GuidSectionHeader;
  UINT32                    CRC32Checksum;
} CRC32_SECTION_HEADER;


Crc32GuidSect->GuidSectionHeader.DataOffset  = sizeof (CRC32_SECTION_HEADER);



"

Personally think that is the root cause of the 4 bytes issue.



I do not know why the GenCrc32 usage is different from other tools in origin design.

Propose to solve this issue from GenSec side to keep all the guidtool with same function.



Thanks,

Christine



> -----Original Message-----

> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of

> Konstantin Aladyshev

> Sent: Tuesday, July 19, 2022 9:45 PM

> To: devel@edk2.groups.io

> Cc: Feng, Bob C <bob.c.feng@intel.com>; Gao, Liming

> <gaoliming@byosoft.com.cn>; Chen, Christine <yuwei.chen@intel.com>;

> Konstantin Aladyshev <aladyshev22@gmail.com>

> Subject: [edk2-devel] [PATCH] BaseTools/VolInfo: Correct buffer for

> GenCrc32 tool

>

> If the guided section was encoded with GenCrc32 tool the resulting

> 'EFI_GUID_DEFINED_SECTION.DataOffset' field points to the start of the

> meaningfull data that follows the CRC32 value.

> But if we want to decode the section with GenCrc32 tool we need to provide

> a buffer that includes the CRC32 value itself.

>

> Signed-off-by: Konstantin Aladyshev <aladyshev22@gmail.com<mailto:aladyshev22@gmail.com>>

> ---

>  BaseTools/Source/C/VolInfo/VolInfo.c | 7 +++++++

>  1 file changed, 7 insertions(+)

>

> diff --git a/BaseTools/Source/C/VolInfo/VolInfo.c

> b/BaseTools/Source/C/VolInfo/VolInfo.c

> index f450796f9c..47a8f80f46 100644

> --- a/BaseTools/Source/C/VolInfo/VolInfo.c

> +++ b/BaseTools/Source/C/VolInfo/VolInfo.c

> @@ -1999,6 +1999,13 @@ Returns:

>            );         free (ExtractionTool); +        if (!CompareGuid (+               EfiGuid,+

> &gEfiCrc32GuidedSectionExtractionProtocolGuid+               )+           ) {+

> DataOffset -= 4;+        }         Status =           PutFileImage (             ToolInputFile,--

> 2.25.1

>

>

>

> -=-=-=-=-=-=

> Groups.io Links: You receive all messages sent to this group.

> View/Reply Online (#91530): https://edk2.groups.io/g/devel/message/91530

> Mute This Topic: https://groups.io/mt/92482555/4546272

> Group Owner: devel+owner@edk2.groups.io<mailto:devel+owner@edk2.groups.io>

> Unsubscribe: https://edk2.groups.io/g/devel/unsub [yuwei.chen@intel.com]

> -=-=-=-=-=-=

>




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#91960): https://edk2.groups.io/g/devel/message/91960
Mute This Topic: https://groups.io/mt/92482555/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] BaseTools/VolInfo: Correct buffer for GenCrc32 tool

Posted by Konstantin Aladyshev 1 year, 9 months ago

Hi Christine!

I think the GenCrc32 usage is different because it just prepends the
data with the CRC32 value leaving the main data unmodified, opposed to
all of the encryption utilities that change the data itself.

Therefore in the case of the GenCrc32 produced section it is possible
to get the main data just by the 'DataOffset' field.

Here is the definition of the EFI_GUIDED_SECTION_PROCESSING_REQUIRED
flag from the PI specification:
```
EFI_GUIDED_SECTION_ PROCESSING_REQUIRED:
Set to 1 if the section requires processing to obtain meaningful data
from the section contents. Processing would be required, for example,
if the section contents were encrypted or compressed. If the
EFI_GUIDED_SECTION_PROCESSING_REQUIRED bit is cleared to zero, it is
possible to retrieve the section’s contents without processing in the
absence of an associated instance of the
EFI_GUIDED_SECTION_EXTRACTION_PROTOCOL (DXE) or
EFI_PEI_GUIDED_SECTION_EXTRACTION_PPI (PEI).. In this case, the
beginning of the encapsulated section stream is indicated by the value
of DataOffset
```

So in the case of encryption utilities like
LzmaCompress/TianoCompress/BrotliCompress/... we set
EFI_GUIDED_SECTION_PROCESSING_REQUIRED=1 and DataOffset points to the
very start of data right after the EFI_GUID_DEFINED_SECTION header.
But in the case of GenCrc32 we can just set
EFI_GUIDED_SECTION_PROCESSING_REQUIRED=0 and point DataOffset to the
(EFI_GUID_DEFINED_SECTION + 4 bytes).

Best regards,
Konstantin Aladyshev

On Fri, Jul 29, 2022 at 7:02 AM Chen, Christine <yuwei.chen@intel.com> wrote:
>
> Hi Konstantin, this is really an issue need to be solved.
>
> When GenSec tool uses the guidtools to encode the section, it differentiates how GenCrc32 is used from other tools..
>
> GenSec defines a CRC32_SECTION_HEADER structure for GenCrc32, and calculate the DataOffset of the Section with this header.
>
> However other guidtools just use the EFI_GUID_DEFINED_SECTION header and calculate the DataOffset with it.
>
>
>
> “
>
> typedef struct {
>
>   EFI_GUID_DEFINED_SECTION  GuidSectionHeader;
>
>   UINT32                    CRC32Checksum;
>
> } CRC32_SECTION_HEADER;
>
>
>
> Crc32GuidSect->GuidSectionHeader.DataOffset  = sizeof (CRC32_SECTION_HEADER);
>
>
>
> ”
>
> Personally think that is the root cause of the 4 bytes issue.
>
>
>
> I do not know why the GenCrc32 usage is different from other tools in origin design.
>
> Propose to solve this issue from GenSec side to keep all the guidtool with same function.
>
>
>
> Thanks,
>
> Christine
>
>
>
> > -----Original Message-----
>
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
>
> > Konstantin Aladyshev
>
> > Sent: Tuesday, July 19, 2022 9:45 PM
>
> > To: devel@edk2.groups.io
>
> > Cc: Feng, Bob C <bob.c.feng@intel.com>; Gao, Liming
>
> > <gaoliming@byosoft.com.cn>; Chen, Christine <yuwei.chen@intel.com>;
>
> > Konstantin Aladyshev <aladyshev22@gmail.com>
>
> > Subject: [edk2-devel] [PATCH] BaseTools/VolInfo: Correct buffer for
>
> > GenCrc32 tool
>
> >
>
> > If the guided section was encoded with GenCrc32 tool the resulting
>
> > 'EFI_GUID_DEFINED_SECTION.DataOffset' field points to the start of the
>
> > meaningfull data that follows the CRC32 value.
>
> > But if we want to decode the section with GenCrc32 tool we need to provide
>
> > a buffer that includes the CRC32 value itself.
>
> >
>
> > Signed-off-by: Konstantin Aladyshev <aladyshev22@gmail.com>
>
> > ---
>
> >  BaseTools/Source/C/VolInfo/VolInfo.c | 7 +++++++
>
> >  1 file changed, 7 insertions(+)
>
> >
>
> > diff --git a/BaseTools/Source/C/VolInfo/VolInfo.c
>
> > b/BaseTools/Source/C/VolInfo/VolInfo.c
>
> > index f450796f9c..47a8f80f46 100644
>
> > --- a/BaseTools/Source/C/VolInfo/VolInfo.c
>
> > +++ b/BaseTools/Source/C/VolInfo/VolInfo.c
>
> > @@ -1999,6 +1999,13 @@ Returns:
>
> >            );         free (ExtractionTool); +        if (!CompareGuid (+               EfiGuid,+
>
> > &gEfiCrc32GuidedSectionExtractionProtocolGuid+               )+           ) {+
>
> > DataOffset -= 4;+        }         Status =           PutFileImage (             ToolInputFile,--
>
> > 2.25.1
>
> >
>
> >
>
> >
>
> > -=-=-=-=-=-=
>
> > Groups.io Links: You receive all messages sent to this group.
>
> > View/Reply Online (#91530): https://edk2.groups.io/g/devel/message/91530
>
> > Mute This Topic: https://groups.io/mt/92482555/4546272
>
> > Group Owner: devel+owner@edk2.groups.io
>
> > Unsubscribe: https://edk2.groups.io/g/devel/unsub [yuwei.chen@intel.com]
>
> > -=-=-=-=-=-=
>
> >
>
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#91970): https://edk2.groups.io/g/devel/message/91970
Mute This Topic: https://groups.io/mt/92482555/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] BaseTools/VolInfo: Correct buffer for GenCrc32 tool

Posted by Bob Feng 1 year, 7 months ago

Yes, I agree that in the case of GenCrc32 we can just set
EFI_GUIDED_SECTION_PROCESSING_REQUIRED=0 and point DataOffset to the (EFI_GUID_DEFINED_SECTION + 4 bytes).

For this patch, 
Reviewed-by: Bob Feng <bob.c.feng@intel.com>

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Konstantin Aladyshev
Sent: Friday, July 29, 2022 5:03 PM
To: Chen, Christine <yuwei.chen@intel.com>
Cc: devel@edk2.groups.io; Feng, Bob C <bob.c.feng@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>
Subject: Re: [edk2-devel] [PATCH] BaseTools/VolInfo: Correct buffer for GenCrc32 tool

Hi Christine!

I think the GenCrc32 usage is different because it just prepends the data with the CRC32 value leaving the main data unmodified, opposed to all of the encryption utilities that change the data itself.

Therefore in the case of the GenCrc32 produced section it is possible to get the main data just by the 'DataOffset' field.

Here is the definition of the EFI_GUIDED_SECTION_PROCESSING_REQUIRED
flag from the PI specification:
```
EFI_GUIDED_SECTION_ PROCESSING_REQUIRED:
Set to 1 if the section requires processing to obtain meaningful data from the section contents. Processing would be required, for example, if the section contents were encrypted or compressed. If the EFI_GUIDED_SECTION_PROCESSING_REQUIRED bit is cleared to zero, it is possible to retrieve the section’s contents without processing in the absence of an associated instance of the EFI_GUIDED_SECTION_EXTRACTION_PROTOCOL (DXE) or EFI_PEI_GUIDED_SECTION_EXTRACTION_PPI (PEI).. In this case, the beginning of the encapsulated section stream is indicated by the value of DataOffset ```

So in the case of encryption utilities like LzmaCompress/TianoCompress/BrotliCompress/... we set
EFI_GUIDED_SECTION_PROCESSING_REQUIRED=1 and DataOffset points to the very start of data right after the EFI_GUID_DEFINED_SECTION header.
But in the case of GenCrc32 we can just set
EFI_GUIDED_SECTION_PROCESSING_REQUIRED=0 and point DataOffset to the (EFI_GUID_DEFINED_SECTION + 4 bytes).

Best regards,
Konstantin Aladyshev

On Fri, Jul 29, 2022 at 7:02 AM Chen, Christine <yuwei.chen@intel.com> wrote:
>
> Hi Konstantin, this is really an issue need to be solved.
>
> When GenSec tool uses the guidtools to encode the section, it differentiates how GenCrc32 is used from other tools..
>
> GenSec defines a CRC32_SECTION_HEADER structure for GenCrc32, and calculate the DataOffset of the Section with this header.
>
> However other guidtools just use the EFI_GUID_DEFINED_SECTION header and calculate the DataOffset with it.
>
>
>
> “
>
> typedef struct {
>
>   EFI_GUID_DEFINED_SECTION  GuidSectionHeader;
>
>   UINT32                    CRC32Checksum;
>
> } CRC32_SECTION_HEADER;
>
>
>
> Crc32GuidSect->GuidSectionHeader.DataOffset  = sizeof 
> Crc32GuidSect->(CRC32_SECTION_HEADER);
>
>
>
> ”
>
> Personally think that is the root cause of the 4 bytes issue.
>
>
>
> I do not know why the GenCrc32 usage is different from other tools in origin design.
>
> Propose to solve this issue from GenSec side to keep all the guidtool with same function.
>
>
>
> Thanks,
>
> Christine
>
>
>
> > -----Original Message-----
>
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
>
> > Konstantin Aladyshev
>
> > Sent: Tuesday, July 19, 2022 9:45 PM
>
> > To: devel@edk2.groups.io
>
> > Cc: Feng, Bob C <bob.c.feng@intel.com>; Gao, Liming
>
> > <gaoliming@byosoft.com.cn>; Chen, Christine <yuwei.chen@intel.com>;
>
> > Konstantin Aladyshev <aladyshev22@gmail.com>
>
> > Subject: [edk2-devel] [PATCH] BaseTools/VolInfo: Correct buffer for
>
> > GenCrc32 tool
>
> >
>
> > If the guided section was encoded with GenCrc32 tool the resulting
>
> > 'EFI_GUID_DEFINED_SECTION.DataOffset' field points to the start of 
> > the
>
> > meaningfull data that follows the CRC32 value.
>
> > But if we want to decode the section with GenCrc32 tool we need to 
> > provide
>
> > a buffer that includes the CRC32 value itself.
>
> >
>
> > Signed-off-by: Konstantin Aladyshev <aladyshev22@gmail.com>
>
> > ---
>
> >  BaseTools/Source/C/VolInfo/VolInfo.c | 7 +++++++
>
> >  1 file changed, 7 insertions(+)
>
> >
>
> > diff --git a/BaseTools/Source/C/VolInfo/VolInfo.c
>
> > b/BaseTools/Source/C/VolInfo/VolInfo.c
>
> > index f450796f9c..47a8f80f46 100644
>
> > --- a/BaseTools/Source/C/VolInfo/VolInfo.c
>
> > +++ b/BaseTools/Source/C/VolInfo/VolInfo.c
>
> > @@ -1999,6 +1999,13 @@ Returns:
>
> >            );         free (ExtractionTool); +        if (!CompareGuid (+               EfiGuid,+
>
> > &gEfiCrc32GuidedSectionExtractionProtocolGuid+               )+           ) {+
>
> > DataOffset -= 4;+        }         Status =           PutFileImage (             ToolInputFile,--
>
> > 2.25.1
>
> >
>
> >
>
> >
>
> > -=-=-=-=-=-=
>
> > Groups.io Links: You receive all messages sent to this group.
>
> > View/Reply Online (#91530): 
> > https://edk2.groups.io/g/devel/message/91530
>
> > Mute This Topic: https://groups.io/mt/92482555/4546272
>
> > Group Owner: devel+owner@edk2.groups.io
>
> > Unsubscribe: https://edk2.groups.io/g/devel/unsub 
> > [yuwei.chen@intel.com]
>
> > -=-=-=-=-=-=
>
> >
>
>







-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#94611): https://edk2.groups.io/g/devel/message/94611
Mute This Topic: https://groups.io/mt/92482555/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-