[PATCH] Revert "mkimage: fit: Do not tail-pad fitImage with external data"

[PATCH] Revert "mkimage: fit: Do not tail-pad fitImage with external data"

[Tom Rini]

This has been reported to break booting of U-Boot from SPL on a number
of platforms due to a lack of alignment of the external data.  The
issues this commit is addressing will need to be resolved another way.

This reverts commit 20a154f95bfe0a3b5bfba90bea7f001c58217536.

Reported-by: Alex Kiernan <alex.kiernan@gmail.com>
Reported-by: Michael Walle <michael@walle.cc>
Signed-off-by: Tom Rini <trini@konsulko.com>
---
 tools/fit_image.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/fit_image.c b/tools/fit_image.c
index 1e0f1e9fce8b..88ff093d05be 100644
--- a/tools/fit_image.c
+++ b/tools/fit_image.c
@@ -435,7 +435,7 @@ static int fit_extract_data(struct image_tool_params *params, const char *fname)
 	int image_number;
 	int align_size;
 
-	align_size = params->bl_len ? params->bl_len : 1;
+	align_size = params->bl_len ? params->bl_len : 4;
 	fd = mmap_fdt(params->cmdname, fname, 0, &fdt, &sbuf, false, false);
 	if (fd < 0)
 		return -EIO;
@@ -493,6 +493,7 @@ static int fit_extract_data(struct image_tool_params *params, const char *fname)
 	fdt_pack(fdt);
 
 	new_size = fdt_totalsize(fdt);
+	new_size = ALIGN(new_size, align_size);
 	fdt_set_totalsize(fdt, new_size);
 	debug("Size reduced from %x to %x\n", fit_size, fdt_totalsize(fdt));
 	debug("External data size %x\n", buf_ptr);
-- 
2.17.1

[PATCH] Revert "mkimage: fit: Do not tail-pad fitImage with external data"

[Marek Vasut]

On 5/6/20 5:05 PM, Tom Rini wrote:
> This has been reported to break booting of U-Boot from SPL on a number
> of platforms due to a lack of alignment of the external data.  The
> issues this commit is addressing will need to be resolved another way.
> 
> This reverts commit 20a154f95bfe0a3b5bfba90bea7f001c58217536.
> 
> Reported-by: Alex Kiernan <alex.kiernan@gmail.com>
> Reported-by: Michael Walle <michael@walle.cc>
> Signed-off-by: Tom Rini <trini@konsulko.com>

The commit message should also warn that this re-opens the data leak in
the padding.

[PATCH] Revert "mkimage: fit: Do not tail-pad fitImage with external data"

[Tom Rini]

On Wed, May 06, 2020 at 05:08:12PM +0200, Marek Vasut wrote:
> On 5/6/20 5:05 PM, Tom Rini wrote:
> > This has been reported to break booting of U-Boot from SPL on a number
> > of platforms due to a lack of alignment of the external data.  The
> > issues this commit is addressing will need to be resolved another way.
> > 
> > This reverts commit 20a154f95bfe0a3b5bfba90bea7f001c58217536.
> > 
> > Reported-by: Alex Kiernan <alex.kiernan@gmail.com>
> > Reported-by: Michael Walle <michael@walle.cc>
> > Signed-off-by: Tom Rini <trini@konsulko.com>
> 
> The commit message should also warn that this re-opens the data leak in
> the padding.

Sure.  Are you going to send a patch switching to calloc or should I?

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200506/3d244773/attachment.sig>

[PATCH] Revert "mkimage: fit: Do not tail-pad fitImage with external data"

[Marek Vasut]

On 5/6/20 5:19 PM, Tom Rini wrote:
> On Wed, May 06, 2020 at 05:08:12PM +0200, Marek Vasut wrote:
>> On 5/6/20 5:05 PM, Tom Rini wrote:
>>> This has been reported to break booting of U-Boot from SPL on a number
>>> of platforms due to a lack of alignment of the external data.  The
>>> issues this commit is addressing will need to be resolved another way.
>>>
>>> This reverts commit 20a154f95bfe0a3b5bfba90bea7f001c58217536.
>>>
>>> Reported-by: Alex Kiernan <alex.kiernan@gmail.com>
>>> Reported-by: Michael Walle <michael@walle.cc>
>>> Signed-off-by: Tom Rini <trini@konsulko.com>
>>
>> The commit message should also warn that this re-opens the data leak in
>> the padding.
> 
> Sure.  Are you going to send a patch switching to calloc or should I?

I think the discussion about the padding requirement isn't finished yet?
See my remark about the load = <>; part.

[PATCH] Revert "mkimage: fit: Do not tail-pad fitImage with external data"

[Tom Rini]

On Wed, May 06, 2020 at 05:23:55PM +0200, Marek Vasut wrote:
> On 5/6/20 5:19 PM, Tom Rini wrote:
> > On Wed, May 06, 2020 at 05:08:12PM +0200, Marek Vasut wrote:
> >> On 5/6/20 5:05 PM, Tom Rini wrote:
> >>> This has been reported to break booting of U-Boot from SPL on a number
> >>> of platforms due to a lack of alignment of the external data.  The
> >>> issues this commit is addressing will need to be resolved another way.
> >>>
> >>> This reverts commit 20a154f95bfe0a3b5bfba90bea7f001c58217536.
> >>>
> >>> Reported-by: Alex Kiernan <alex.kiernan@gmail.com>
> >>> Reported-by: Michael Walle <michael@walle.cc>
> >>> Signed-off-by: Tom Rini <trini@konsulko.com>
> >>
> >> The commit message should also warn that this re-opens the data leak in
> >> the padding.
> > 
> > Sure.  Are you going to send a patch switching to calloc or should I?
> 
> I think the discussion about the padding requirement isn't finished yet?
> See my remark about the load = <>; part.

I'm unsure what more you need to provide a potential fix to the
platforms that have reported breakage.  I suspect you might even have a
platform handy that is broken, given the imx platform.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200506/f7ce3be8/attachment.sig>

[PATCH] Revert "mkimage: fit: Do not tail-pad fitImage with external data"

[Marek Vasut]

On 5/6/20 5:31 PM, Tom Rini wrote:
> On Wed, May 06, 2020 at 05:23:55PM +0200, Marek Vasut wrote:
>> On 5/6/20 5:19 PM, Tom Rini wrote:
>>> On Wed, May 06, 2020 at 05:08:12PM +0200, Marek Vasut wrote:
>>>> On 5/6/20 5:05 PM, Tom Rini wrote:
>>>>> This has been reported to break booting of U-Boot from SPL on a number
>>>>> of platforms due to a lack of alignment of the external data.  The
>>>>> issues this commit is addressing will need to be resolved another way.
>>>>>
>>>>> This reverts commit 20a154f95bfe0a3b5bfba90bea7f001c58217536.
>>>>>
>>>>> Reported-by: Alex Kiernan <alex.kiernan@gmail.com>
>>>>> Reported-by: Michael Walle <michael@walle.cc>
>>>>> Signed-off-by: Tom Rini <trini@konsulko.com>
>>>>
>>>> The commit message should also warn that this re-opens the data leak in
>>>> the padding.
>>>
>>> Sure.  Are you going to send a patch switching to calloc or should I?
>>
>> I think the discussion about the padding requirement isn't finished yet?
>> See my remark about the load = <>; part.
> 
> I'm unsure what more you need to provide a potential fix to the
> platforms that have reported breakage.  I suspect you might even have a
> platform handy that is broken, given the imx platform.

Let's continue the discussion in one thread.

I think we should only ever enforce the padding if really required.
Apply this one and add the calloc fix, but we should fix the root cause,
not go back to papering over this problem.

[PATCH] Revert "mkimage: fit: Do not tail-pad fitImage with external data"

[Tom Rini]

On Wed, May 06, 2020 at 11:05:17AM -0400, Tom Rini wrote:

> This has been reported to break booting of U-Boot from SPL on a number
> of platforms due to a lack of alignment of the external data.  The
> issues this commit is addressing will need to be resolved another way.
> 
> This reverts commit 20a154f95bfe0a3b5bfba90bea7f001c58217536.
> 
> Reported-by: Alex Kiernan <alex.kiernan@gmail.com>
> Reported-by: Michael Walle <michael@walle.cc>
> Signed-off-by: Tom Rini <trini@konsulko.com>

With a note about the information leak re-added:

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200507/56f6d079/attachment.sig>