PrePiLib's FwVol.c can't handle padding before volume header

PrePiLib's FwVol.c can't handle padding before volume header

[Michael Zimmermann]

'uefitool' shows me that there are 4 bytes of padding right before
FVMAIN when adding large(20MB) uncompressed FV's.

FwVol fails detecting that and complains about the signature not being correct.

Thanks
Michael

Re: PrePiLib's FwVol.c can't handle padding before volume header

[Ard Biesheuvel]

cc BaseTools maintainers

On 10 December 2017 at 20:58, Michael Zimmermann
<sigmaepsilon92@gmail.com> wrote:
> 'uefitool' shows me that there are 4 bytes of padding right before
> FVMAIN when adding large(20MB) uncompressed FV's.
>
> FwVol fails detecting that and complains about the signature not being correct.
>
> Thanks
> Michael

Re: PrePiLib's FwVol.c can't handle padding before volume header

[Ard Biesheuvel]

On 10 December 2017 at 21:22, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> cc BaseTools maintainers
>
> On 10 December 2017 at 20:58, Michael Zimmermann
> <sigmaepsilon92@gmail.com> wrote:
>> 'uefitool' shows me that there are 4 bytes of padding right before
>> FVMAIN when adding large(20MB) uncompressed FV's.
>>
>> FwVol fails detecting that and complains about the signature not being correct.
>>

I missed the 'PrePiLib' in the subject, apologies.

So could you elaborate please? What is the layout of the FV, and
when/how does PrePiLib complain about it?

Re: PrePiLib's FwVol.c can't handle padding before volume header

[Michael Zimmermann]

The layout is basically the same as in ArmVirtQemuKernel.fdf but I've
changed the size to 8MB and these are the usage stats:
FV Space Information
FVMAINDEVICE [99%Full] 57856 total, 57800 used, 56 free
FVMAINMSM8960 [99%Full] 222336 total, 222312 used, 24 free
FVMAIN_COMPACT [63%Full] 8380416 total, 5305944 used, 3074472 free
FVMAIN [99%Full] 17026304 total, 17026264 used, 40 free
FVMAINQCOM [99%Full] 37184 total, 37152 used, 32 free

and here's an uefitool report:
https://pastebin.com/pnHNSFz4
The padding in line 13 is the one causing trouble. The interesting
part is that it's not zero. it's value is '08 CD 03 01'.

PrePiLib fails in this line:
https://github.com/tianocore/edk2/blob/5a44a766b597e4c9960ac1936e6d18001c5e7ce2/EmbeddedPkg/Library/PrePiLib/FwVol.c#L682
The reason is simply that VolumeHandle is offset by 4 bytes.

On Sun, Dec 10, 2017 at 10:33 PM, Ard Biesheuvel
<ard.biesheuvel@linaro.org> wrote:
> On 10 December 2017 at 21:22, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>> cc BaseTools maintainers
>>
>> On 10 December 2017 at 20:58, Michael Zimmermann
>> <sigmaepsilon92@gmail.com> wrote:
>>> 'uefitool' shows me that there are 4 bytes of padding right before
>>> FVMAIN when adding large(20MB) uncompressed FV's.
>>>
>>> FwVol fails detecting that and complains about the signature not being correct.
>>>
>
> I missed the 'PrePiLib' in the subject, apologies.
>
> So could you elaborate please? What is the layout of the FV, and
> when/how does PrePiLib complain about it?

Re: PrePiLib's FwVol.c can't handle padding before volume header

[Ard Biesheuvel]

On 10 December 2017 at 21:45, Michael Zimmermann
<sigmaepsilon92@gmail.com> wrote:
> The layout is basically the same as in ArmVirtQemuKernel.fdf but I've
> changed the size to 8MB and these are the usage stats:
> FV Space Information
> FVMAINDEVICE [99%Full] 57856 total, 57800 used, 56 free
> FVMAINMSM8960 [99%Full] 222336 total, 222312 used, 24 free
> FVMAIN_COMPACT [63%Full] 8380416 total, 5305944 used, 3074472 free
> FVMAIN [99%Full] 17026304 total, 17026264 used, 40 free
> FVMAINQCOM [99%Full] 37184 total, 37152 used, 32 free
>
> and here's an uefitool report:
> https://pastebin.com/pnHNSFz4
> The padding in line 13 is the one causing trouble. The interesting
> part is that it's not zero. it's value is '08 CD 03 01'.
>
> PrePiLib fails in this line:
> https://github.com/tianocore/edk2/blob/5a44a766b597e4c9960ac1936e6d18001c5e7ce2/EmbeddedPkg/Library/PrePiLib/FwVol.c#L682
> The reason is simply that VolumeHandle is offset by 4 bytes.
>

That code is identical to PeiFfsFvPpiGetVolumeInfo() in PEI core so
this does not look specific to PrePiLib.

Does VolInfo complain as well?

Re: PrePiLib's FwVol.c can't handle padding before volume header

[Michael Zimmermann]

VolInfo doesn't seem to complain about FVMAIN_COMPACT.Fv:
https://pastebin.com/ueUnepXF

Does VolInfo support Fs's as well? because it fails on mine(even on a
working one).

On Sun, Dec 10, 2017 at 10:49 PM, Ard Biesheuvel
<ard.biesheuvel@linaro.org> wrote:
> On 10 December 2017 at 21:45, Michael Zimmermann
> <sigmaepsilon92@gmail.com> wrote:
>> The layout is basically the same as in ArmVirtQemuKernel.fdf but I've
>> changed the size to 8MB and these are the usage stats:
>> FV Space Information
>> FVMAINDEVICE [99%Full] 57856 total, 57800 used, 56 free
>> FVMAINMSM8960 [99%Full] 222336 total, 222312 used, 24 free
>> FVMAIN_COMPACT [63%Full] 8380416 total, 5305944 used, 3074472 free
>> FVMAIN [99%Full] 17026304 total, 17026264 used, 40 free
>> FVMAINQCOM [99%Full] 37184 total, 37152 used, 32 free
>>
>> and here's an uefitool report:
>> https://pastebin.com/pnHNSFz4
>> The padding in line 13 is the one causing trouble. The interesting
>> part is that it's not zero. it's value is '08 CD 03 01'.
>>
>> PrePiLib fails in this line:
>> https://github.com/tianocore/edk2/blob/5a44a766b597e4c9960ac1936e6d18001c5e7ce2/EmbeddedPkg/Library/PrePiLib/FwVol.c#L682
>> The reason is simply that VolumeHandle is offset by 4 bytes.
>>
>
> That code is identical to PeiFfsFvPpiGetVolumeInfo() in PEI core so
> this does not look specific to PrePiLib.
>
> Does VolInfo complain as well?

Re: PrePiLib's FwVol.c can't handle padding before volume header

[Ard Biesheuvel]

On 10 December 2017 at 21:58, Michael Zimmermann
<sigmaepsilon92@gmail.com> wrote:
> VolInfo doesn't seem to complain about FVMAIN_COMPACT.Fv:
> https://pastebin.com/ueUnepXF
>
> Does VolInfo support Fs's as well? because it fails on mine(even on a
> working one).
>

You mean to dissect the FFS that contains the FV? Not sure.

 So the _FVH signature is there but shifted by 4 bytes, right? Does
that only happen with large FVs?

Re: PrePiLib's FwVol.c can't handle padding before volume header

[Michael Zimmermann]

Exactly. If I shift the pointer by 4 bytes from within PrePiLib the device
boots just fine.

I'm not sure if the size is the root cause but right now it only happens
when adding a ~15mb binary efi to the end of fvmain.

On Dec 10, 2017 11:13 PM, "Ard Biesheuvel" <ard.biesheuvel@linaro.org>
wrote:

On 10 December 2017 at 21:58, Michael Zimmermann
<sigmaepsilon92@gmail.com> wrote:
> VolInfo doesn't seem to complain about FVMAIN_COMPACT.Fv:
> https://pastebin.com/ueUnepXF
>
> Does VolInfo support Fs's as well? because it fails on mine(even on a
> working one).
>

You mean to dissect the FFS that contains the FV? Not sure.

 So the _FVH signature is there but shifted by 4 bytes, right? Does
that only happen with large FVs?

Re: PrePiLib's FwVol.c can't handle padding before volume header

[Ard Biesheuvel]

On 10 December 2017 at 22:18, Michael Zimmermann
<sigmaepsilon92@gmail.com> wrote:
> Exactly. If I shift the pointer by 4 bytes from within PrePiLib the device
> boots just fine.
>
> I'm not sure if the size is the root cause but right now it only happens
> when adding a ~15mb binary efi to the end of fvmain.
>

It appears so.

So it would be good to check where the disparity originates.

The Ffs section containing the compressed FV looks like this for ArmVirtQemu

$ hexdump -C 9E21FD93-9C72-4c15-8C4B-E77F1DB2D792SEC1.1fv.sec |head

00000000  84 a8 48 17 00 00 00 00  00 00 00 00 00 00 00 00  |..H.............|
00000010  00 00 00 00 78 e5 8c 8c  3d 8a 1c 4f 99 35 89 61  |....x...=..O.5.a|
00000020  85 c3 2d d3 80 a8 48 00  00 00 00 00 5f 46 56 48  |..-...H....._FVH|
00000030  ff fe 04 00 48 00 24 2b  00 00 00 02 a2 22 01 00  |....H.$+....."..|
00000040  40 00 00 00 00 00 00 00  00 00 00 00 7f cb a2 d6  |@...............|
00000050  18 6a 2f 4e b4 3b 99 20  a7 33 70 0a 4d aa 05 00  |.j/N.;. .3p.M...|
00000060  30 c0 01 f8 04 c0 01 10  4d 5a 00 00 00 00 00 00  |0.......MZ......|
00000070  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
*
000000a0  00 00 00 00 58 0e 00 00  00 00 00 00 00 00 00 00  |....X...........|

Could you compare with your build?

Re: PrePiLib's FwVol.c can't handle padding before volume header

[Michael Zimmermann]

without the big file at the end it looks very similar:
00000000  c4 cc 4a 17 00 00 00 00  00 00 00 00 00 00 00 00  |..J.............|
00000010  00 00 00 00 78 e5 8c 8c  3d 8a 1c 4f 99 35 89 61  |....x...=..O.5.a|
00000020  85 c3 2d d3 c0 cc 4a 00  00 00 00 00 5f 46 56 48  |..-...J....._FVH|
00000030  ff fe 04 00 48 00 f1 fd  60 00 00 02 33 2b 01 00  |....H...`...3+..|
00000040  40 00 00 00 00 00 00 00  00 00 00 00 ff ff ff ff  |@...............|
00000050  ff ff ff ff ff ff ff ff  ff ff ff ff f4 aa f0 00  |................|
00000060  2c 00 00 f8 bd 1d f5 8f  56 b8 cb 4a b8 59 85 36  |,.......V..J.Y.6|
00000070  d8 92 ed 3a 14 00 00 00  ff ff ff ff e7 0e 51 fc  |...:..........Q.|
00000080  dc ff d4 11 bd 41 00 80  c7 3c 88 81 e6 aa 02 00  |.....A...<......|
00000090  8c 00 00 f8 74 00 00 19  57 72 cf 80 ab 87 f9 47  |....t...Wr.....G|

with the big file (when it's broken) it looks like this:
00000000  ff ff ff 17 08 cd 03 01  00 00 00 00 00 00 00 00  |................|
00000010  00 00 00 00 00 00 00 00  78 e5 8c 8c 3d 8a 1c 4f  |........x...=..O|
00000020  99 35 89 61 85 c3 2d d3  00 cd 03 01 00 00 00 00  |.5.a..-.........|
00000030  5f 46 56 48 ff fe 04 00  48 00 f4 18 60 00 00 02  |_FVH....H...`...|
00000040  34 0f 04 00 40 00 00 00  00 00 00 00 00 00 00 00  |4...@...........|
00000050  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|
00000060  f4 aa f0 00 2c 00 00 f8  bd 1d f5 8f 56 b8 cb 4a  |....,.......V..J|
00000070  b8 59 85 36 d8 92 ed 3a  14 00 00 00 ff ff ff ff  |.Y.6...:........|
00000080  e7 0e 51 fc dc ff d4 11  bd 41 00 80 c7 3c 88 81  |..Q......A...<..|
00000090  e6 aa 02 00 8c 00 00 f8  74 00 00 19 57 72 cf 80  |........t...Wr..|

While looking for the header format and checking what these ff's mean
I found that this is a EFI_COMMON_SECTION_HEADER2 which is used
because the max size of EFI_COMMON_SECTION_HEADER is 0xFFFFFF.
MdeModulePkg's FwVol.c also seems to have some code for handling v2 sections.
I'll send a patch to fix this in a bit.



On Mon, Dec 11, 2017 at 12:08 AM, Ard Biesheuvel
<ard.biesheuvel@linaro.org> wrote:
> On 10 December 2017 at 22:18, Michael Zimmermann
> <sigmaepsilon92@gmail.com> wrote:
>> Exactly. If I shift the pointer by 4 bytes from within PrePiLib the device
>> boots just fine.
>>
>> I'm not sure if the size is the root cause but right now it only happens
>> when adding a ~15mb binary efi to the end of fvmain.
>>
>
> It appears so.
>
> So it would be good to check where the disparity originates.
>
> The Ffs section containing the compressed FV looks like this for ArmVirtQemu
>
> $ hexdump -C 9E21FD93-9C72-4c15-8C4B-E77F1DB2D792SEC1.1fv.sec |head
>
> 00000000  84 a8 48 17 00 00 00 00  00 00 00 00 00 00 00 00  |..H.............|
> 00000010  00 00 00 00 78 e5 8c 8c  3d 8a 1c 4f 99 35 89 61  |....x...=..O.5.a|
> 00000020  85 c3 2d d3 80 a8 48 00  00 00 00 00 5f 46 56 48  |..-...H....._FVH|
> 00000030  ff fe 04 00 48 00 24 2b  00 00 00 02 a2 22 01 00  |....H.$+....."..|
> 00000040  40 00 00 00 00 00 00 00  00 00 00 00 7f cb a2 d6  |@...............|
> 00000050  18 6a 2f 4e b4 3b 99 20  a7 33 70 0a 4d aa 05 00  |.j/N.;. .3p.M...|
> 00000060  30 c0 01 f8 04 c0 01 10  4d 5a 00 00 00 00 00 00  |0.......MZ......|
> 00000070  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
> *
> 000000a0  00 00 00 00 58 0e 00 00  00 00 00 00 00 00 00 00  |....X...........|
>
> Could you compare with your build?