Re: [edk2] Problems in Image Decoding in HII Database code
Tim, Agree with your analysis, I will follow up to fix it. Thanks, Eric From: Tim Lewis [mailto:tim.le...@insyde.com] Sent: Tuesday, November 05, 2013 12:57 AM To: edk2-devel@lists.sourceforge.net Subject: Re: [edk2] Problems in Image Decoding in HII Database code Eric - For example, it is possible to encode SKIP1 as EXT4 with BlockType2 set to IIBT_SKIP1. This is consistent with the pattern found in all other HII database block types. The purpose of EXT1, EXT2 and EXT4 is to provide an alternate encoding of IIBT blocks where the size is explicit. The other blocks have an implicit type. UCST recognized this weakness, since in limits extensibility, and provided the alternate forms. Please see, for example, Figure 104 for SIBT blocks The point is: ImageId should not be incremented for EXT1, EXT2 or EXT4. Tim From: Dong, Eric [mailto:eric.d...@intel.com] Sent: Monday, November 04, 2013 1:32 AM To: edk2-devel@lists.sourceforge.net<mailto:edk2-devel@lists.sourceforge.net> Subject: Re: [edk2] Problems in Image Decoding in HII Database code Hi Tim, Add my comments below. Thanks, Eric From: Tim Lewis [mailto:tim.le...@insyde.com] Sent: Friday, November 01, 2013 1:16 AM To: edk2-devel@lists.sourceforge.net<mailto:edk2-devel@lists.sourceforge.net> Subject: [edk2] Problems in Image Decoding in HII Database code We noticed that the current HII Database code that processes the image packages incorrectly increments the ImageId for EXT1, EXT2 and EXT4 blocks. The specification allows these forms of the IIBT block types to be used for any other blocks, including DUP and SKIP1 and SKIP2. But currently, EXT1 does this: case EFI_HII_IIBT_EXT1: Length8 = *(ImageBlock + sizeof (EFI_HII_IMAGE_BLOCK) + sizeof (UINT8)); ImageBlock += Length8; ImageIdCurrent++; break; case EFI_HII_IIBT_EXT2: CopyMem ( &Length16, ImageBlock + sizeof (EFI_HII_IMAGE_BLOCK) + sizeof (UINT8), sizeof (UINT16) ); ImageBlock += Length16; ImageIdCurrent++; break; case EFI_HII_IIBT_EXT4: CopyMem ( &Length32, ImageBlock + sizeof (EFI_HII_IMAGE_BLOCK) + sizeof (UINT8), sizeof (UINT32) ); ImageBlock += Length32; ImageIdCurrent++; break; Notice the inconsistent behavior here between EXT1, EXT2 and EXT4. EXT1 does not copy the block, but EXT2 and EXT4 do. [[[Eric]]] I think CopyMem function is used to fix the potential alignment issue raised in IPF platform. Also, notice that ImageIdCurrent is incremented. This leads to inconsistent behavior when SKIP1 and SKIP2 or DUP are encoded using any of these forms. [[[Eric]]] Still not catch your means, what's the relationship between Ext1, EXT2, EXT4 with SKIP1, SKIP2 and DUP? You may ask: why would anyone do this? Well, one of the ways we are considering to extend the image package format is adding new image types (such as IIBIT_32BIT). One of the problems with this is that older tools may correctly skip the unknown block, but they the ImageId assignment would be incorrect (since the older tools don't know it should be incremented). So the solution we were thinking is that new image types should be followed by a IIBT_SKIP1 (1) so that even older tools would correctly handle the increment of the ImageId. But of course, that will not work if the current EDK2 implementation makes all EXT4 and EXT2 increment the ImageId! Tim -- November Webinars for C, C++, Fortran Developers Accelerate application performance with scalable programming models. Explore techniques for threading, error checking, porting, and tuning. Get the most from the latest Intel processors and coprocessors. See abstracts and register http://pubads.g.doubleclick.net/gampad/clk?id=60136231&iu=/4140/ostg.clktrk___ edk2-devel mailing list edk2-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/edk2-devel
Re: [edk2] Problems in Image Decoding in HII Database code
Eric - For example, it is possible to encode SKIP1 as EXT4 with BlockType2 set to IIBT_SKIP1. This is consistent with the pattern found in all other HII database block types. The purpose of EXT1, EXT2 and EXT4 is to provide an alternate encoding of IIBT blocks where the size is explicit. The other blocks have an implicit type. UCST recognized this weakness, since in limits extensibility, and provided the alternate forms. Please see, for example, Figure 104 for SIBT blocks The point is: ImageId should not be incremented for EXT1, EXT2 or EXT4. Tim From: Dong, Eric [mailto:eric.d...@intel.com] Sent: Monday, November 04, 2013 1:32 AM To: edk2-devel@lists.sourceforge.net Subject: Re: [edk2] Problems in Image Decoding in HII Database code Hi Tim, Add my comments below. Thanks, Eric From: Tim Lewis [mailto:tim.le...@insyde.com] Sent: Friday, November 01, 2013 1:16 AM To: edk2-devel@lists.sourceforge.net<mailto:edk2-devel@lists.sourceforge.net> Subject: [edk2] Problems in Image Decoding in HII Database code We noticed that the current HII Database code that processes the image packages incorrectly increments the ImageId for EXT1, EXT2 and EXT4 blocks. The specification allows these forms of the IIBT block types to be used for any other blocks, including DUP and SKIP1 and SKIP2. But currently, EXT1 does this: case EFI_HII_IIBT_EXT1: Length8 = *(ImageBlock + sizeof (EFI_HII_IMAGE_BLOCK) + sizeof (UINT8)); ImageBlock += Length8; ImageIdCurrent++; break; case EFI_HII_IIBT_EXT2: CopyMem ( &Length16, ImageBlock + sizeof (EFI_HII_IMAGE_BLOCK) + sizeof (UINT8), sizeof (UINT16) ); ImageBlock += Length16; ImageIdCurrent++; break; case EFI_HII_IIBT_EXT4: CopyMem ( &Length32, ImageBlock + sizeof (EFI_HII_IMAGE_BLOCK) + sizeof (UINT8), sizeof (UINT32) ); ImageBlock += Length32; ImageIdCurrent++; break; Notice the inconsistent behavior here between EXT1, EXT2 and EXT4. EXT1 does not copy the block, but EXT2 and EXT4 do. [[[Eric]]] I think CopyMem function is used to fix the potential alignment issue raised in IPF platform. Also, notice that ImageIdCurrent is incremented. This leads to inconsistent behavior when SKIP1 and SKIP2 or DUP are encoded using any of these forms. [[[Eric]]] Still not catch your means, what's the relationship between Ext1, EXT2, EXT4 with SKIP1, SKIP2 and DUP? You may ask: why would anyone do this? Well, one of the ways we are considering to extend the image package format is adding new image types (such as IIBIT_32BIT). One of the problems with this is that older tools may correctly skip the unknown block, but they the ImageId assignment would be incorrect (since the older tools don't know it should be incremented). So the solution we were thinking is that new image types should be followed by a IIBT_SKIP1 (1) so that even older tools would correctly handle the increment of the ImageId. But of course, that will not work if the current EDK2 implementation makes all EXT4 and EXT2 increment the ImageId! Tim -- Android is increasing in popularity, but the open development platform that developers love is also attractive to malware creators. Download this white paper to learn more about secure code signing practices that can help keep Android apps secure. http://pubads.g.doubleclick.net/gampad/clk?id=65839951&iu=/4140/ostg.clktrk___ edk2-devel mailing list edk2-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/edk2-devel
Re: [edk2] Problems in Image Decoding in HII Database code
Hi Tim, Add my comments below. Thanks, Eric From: Tim Lewis [mailto:tim.le...@insyde.com] Sent: Friday, November 01, 2013 1:16 AM To: edk2-devel@lists.sourceforge.net<mailto:edk2-devel@lists.sourceforge.net> Subject: [edk2] Problems in Image Decoding in HII Database code We noticed that the current HII Database code that processes the image packages incorrectly increments the ImageId for EXT1, EXT2 and EXT4 blocks. The specification allows these forms of the IIBT block types to be used for any other blocks, including DUP and SKIP1 and SKIP2. But currently, EXT1 does this: case EFI_HII_IIBT_EXT1: Length8 = *(ImageBlock + sizeof (EFI_HII_IMAGE_BLOCK) + sizeof (UINT8)); ImageBlock += Length8; ImageIdCurrent++; break; case EFI_HII_IIBT_EXT2: CopyMem ( &Length16, ImageBlock + sizeof (EFI_HII_IMAGE_BLOCK) + sizeof (UINT8), sizeof (UINT16) ); ImageBlock += Length16; ImageIdCurrent++; break; case EFI_HII_IIBT_EXT4: CopyMem ( &Length32, ImageBlock + sizeof (EFI_HII_IMAGE_BLOCK) + sizeof (UINT8), sizeof (UINT32) ); ImageBlock += Length32; ImageIdCurrent++; break; Notice the inconsistent behavior here between EXT1, EXT2 and EXT4. EXT1 does not copy the block, but EXT2 and EXT4 do. [[[Eric]]] I think CopyMem function is used to fix the potential alignment issue raised in IPF platform. Also, notice that ImageIdCurrent is incremented. This leads to inconsistent behavior when SKIP1 and SKIP2 or DUP are encoded using any of these forms. [[[Eric]]] Still not catch your means, what's the relationship between Ext1, EXT2, EXT4 with SKIP1, SKIP2 and DUP? You may ask: why would anyone do this? Well, one of the ways we are considering to extend the image package format is adding new image types (such as IIBIT_32BIT). One of the problems with this is that older tools may correctly skip the unknown block, but they the ImageId assignment would be incorrect (since the older tools don't know it should be incremented). So the solution we were thinking is that new image types should be followed by a IIBT_SKIP1 (1) so that even older tools would correctly handle the increment of the ImageId. But of course, that will not work if the current EDK2 implementation makes all EXT4 and EXT2 increment the ImageId! Tim -- Android is increasing in popularity, but the open development platform that developers love is also attractive to malware creators. Download this white paper to learn more about secure code signing practices that can help keep Android apps secure. http://pubads.g.doubleclick.net/gampad/clk?id=65839951&iu=/4140/ostg.clktrk___ edk2-devel mailing list edk2-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/edk2-devel
[edk2] Problems in Image Decoding in HII Database code
We noticed that the current HII Database code that processes the image packages incorrectly increments the ImageId for EXT1, EXT2 and EXT4 blocks. The specification allows these forms of the IIBT block types to be used for any other blocks, including DUP and SKIP1 and SKIP2. But currently, EXT1 does this: case EFI_HII_IIBT_EXT1: Length8 = *(ImageBlock + sizeof (EFI_HII_IMAGE_BLOCK) + sizeof (UINT8)); ImageBlock += Length8; ImageIdCurrent++; break; case EFI_HII_IIBT_EXT2: CopyMem ( &Length16, ImageBlock + sizeof (EFI_HII_IMAGE_BLOCK) + sizeof (UINT8), sizeof (UINT16) ); ImageBlock += Length16; ImageIdCurrent++; break; case EFI_HII_IIBT_EXT4: CopyMem ( &Length32, ImageBlock + sizeof (EFI_HII_IMAGE_BLOCK) + sizeof (UINT8), sizeof (UINT32) ); ImageBlock += Length32; ImageIdCurrent++; break; Notice the inconsistent behavior here between EXT1, EXT2 and EXT4. EXT1 does not copy the block, but EXT2 and EXT4 do. Also, notice that ImageIdCurrent is incremented. This leads to inconsistent behavior when SKIP1 and SKIP2 or DUP are encoded using any of these forms. You may ask: why would anyone do this? Well, one of the ways we are considering to extend the image package format is adding new image types (such as IIBIT_32BIT). One of the problems with this is that older tools may correctly skip the unknown block, but they the ImageId assignment would be incorrect (since the older tools don't know it should be incremented). So the solution we were thinking is that new image types should be followed by a IIBT_SKIP1 (1) so that even older tools would correctly handle the increment of the ImageId. But of course, that will not work if the current EDK2 implementation makes all EXT4 and EXT2 increment the ImageId! Tim -- Android is increasing in popularity, but the open development platform that developers love is also attractive to malware creators. Download this white paper to learn more about secure code signing practices that can help keep Android apps secure. http://pubads.g.doubleclick.net/gampad/clk?id=65839951&iu=/4140/ostg.clktrk___ edk2-devel mailing list edk2-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/edk2-devel
