On 03.12.18 16:19, Michael Tretter wrote:
> On Mon, 03 Dec 2018 14:42:59 +0100, Alexander Graf wrote:
>> On 03.12.18 14:14, Michal Simek wrote:
>>> +Alex
>>>
>>> On 28. 11. 18 11:47, Michael Tretter wrote:
>>>> The FSBL copies "Total Partition Word Length" * 4 bytes from the boot.bin,
>>>> which implies that the partition size is 4 byte aligned. When writing the
>>>> partition, mkimage calculates "Total Partition Word Length" by dividing
>>>> the size by 4. This implicitly cuts unaligned bytes at the end of the
>>>> added binary.
>>>>
>>>> Instead of rounding down, the size must be round up to 4 bytes and the
>>>> binary padded accordingly.
>>>>
>>>> Signed-off-by: Michael Tretter <m.tret...@pengutronix.de>
>>>> ---
>>>> tools/zynqmpbif.c | 18 ++++++++++++++----
>>>> 1 file changed, 14 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/tools/zynqmpbif.c b/tools/zynqmpbif.c
>>>> index 6c8f66055d..885a037da6 100644
>>>> --- a/tools/zynqmpbif.c
>>>> +++ b/tools/zynqmpbif.c
>>>> @@ -316,19 +316,29 @@ static int bif_add_pmufw(struct bif_entry *bf, const
>>>> char *data, size_t len)
>>>> return 0;
>>>> }
>>>>
>>>> -static int bif_add_part(struct bif_entry *bf, const char *data, size_t
>>>> len)
>>>> +static int bif_add_part(struct bif_entry *bf, char *data, size_t len)
>>>> {
>>>> size_t parthdr_offset = 0;
>>>> + size_t len_padded = ROUND(len, 4);
>>>> +
>>>> struct partition_header parthdr = {
>>>> - .len_enc = cpu_to_le32(len / 4),
>>>> - .len_unenc = cpu_to_le32(len / 4),
>>>> - .len = cpu_to_le32(len / 4),
>>>> + .len_enc = cpu_to_le32(len_padded / 4),
>>>> + .len_unenc = cpu_to_le32(len_padded / 4),
>>>> + .len = cpu_to_le32(len_padded / 4),
>>>> .entry_point = cpu_to_le64(bf->entry),
>>>> .load_address = cpu_to_le64(bf->load),
>>>> };
>>>> int r;
>>>> uint32_t csum;
>>>>
>>>> + if (len != len_padded) {
>>>> + data = realloc(data, len_padded);
>>>> + while (len < len_padded) {
>>>> + data[len] = 0;
>>>> + len++;
>>>> + }
>>>> + }
>>>> +
>>>> if (bf->flags & (1ULL << BIF_FLAG_PMUFW_IMAGE))
>>>> return bif_add_pmufw(bf, data, len);
>>>>
>>>>
>>>
>>> I am not using this for creating boot image but I have tested it on
>>> regular example and it is still working.
>>>
>>> I have applied this to my branch.
>>> Alex: If you have any comment on this please let us know and I can
>>> remove it from my queue.
>>
>> I don't think I've ever had a not-multiple-of-4 payload, so I never ran
>> into this.
>>
>> The one thing that I'm not fully sure of is the realloc(). I don't think
>> that data is necessarily always a pointer that way allocated using malloc().
>>
>> If you look at bif_add_bit() for example the data pointer is an offset
>> into a pointer from read_full_file(). So realloc() won't work on it.
>>
>> I think it's safer to just allocate a completely new buffer here and
>> copy the payload if unaligned.
>
> Thanks. I thought about aligning data in read_full_file(), but the size
> read by read_full_file() is not used in bif_add_elf(). I further
> assumed that the bitstream is always correctly aligned (because I never
> saw anything else) and will never hit the realloc() path, but I'm
> not sure if this is a safe assumption.
>
> As realloc() does not work in bif_add_part(), I can only malloc() a
> new aligned buffer and cannot free() the old one. I guess that is fine.
Yes. We're already leaking the old buffer, so leaking yet another one
isn't a big issue either ;).
> I will send a v2.
Thanks! And please CC me on patches to zynqmpbif next time :).
Alex
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
