Hi Albert,
On 01/22/2013 09:37 AM, Albert ARIBAUD wrote:
Hi Nikita,
[...]
Barring that, we should at least protect lcd.c from this issue by
making some sort of check for affected targets, or maybe limit the
possible values for splashimage... This issue makes it way too easy
to accidentally break the boot process in a way that's hard to recover
from.
I suggest a few solutions:
1) enforce given load address alignment so that BMP header fields are
natively aligned, with a clear error message. Simple to implement,
difficut for users to understand and accept.
Yes I agree that from a user point of view this looks terrible, which is
why I prefer not to do something like this.
2) once the address provided by the user is known, if it is not
properly aligned, then the next properly aligned address should be
used, and the byte at given address should contain the offset from the
given address to the used address. This is a general solution that
works for any given load address, odd ones included:
Given address: First bytes: Used address:
10000000 2 x 'B' 'M' 10000002
10000001 1 'B' 'M' 10000002
10000002 'B' 'M' 10000002
10000003 3 x x 'B' 'M' 10000006
10000004 2 x 'B' 'M' 10000006
...
Note that if the user address is constrained to be 4-byte-aligned,
then only the "2 x 'B' 'M'" case would apply.
I think a simpler way to implement something like this is to just
use modulo 4 to check alignment and fix the address dynamically;
perhaps even fixing it in the environment.
This is a localized approach though. We will have to do this from
all the code paths that lead to a bmp header being probed in memory.
I would prefer a more localized solution.
3) define an internal 'BMP holder' structure which contains a
two-byte prefix before the 'BMP file' header and data. This way, if
the overall structure is aligned, then the fields in the BMP header are
aligned too.
4) Build a time machine and tell the designers of the BMP header format,
in great inventive and colorful detail, what horrible things will happen
to them if they don't order and size their fields so that they naturally
land on aligned offsets from the header start. This solution gives the
best results IMO.
The problem with 3 (and 4) is that it still doesn't protect the user
from bricking their board by choosing a non-aligned address for their
BMP. This might happen because the user:
- is unaware of the dangers of choosing a non-aligned address
- made a typo
- relies on a script or program that runs under U-Boot to setup stuff
- I"m sure there are other possibilities
In terms of usability it's a *big* regression. If we do not actually
prevent the user from setting splashimage to an unaligned address we
should make sure that all usable addresses are safe.
5) if none the above (including 4) is feasible for some reason, then
use unaligned accessors for this BMP fields, with a Big Fat Comment
about why this is so.
I think, once this feature is merged, I'll try to see if something nice
can be done with an approach like this.
For now I'll add a suggestion-#2-style check in lcd.c
Note that all solutions except 2 (and 4) depend on the given address
being constrained in some way -- such a constraint does not seem
excessive to me.
Amicalement,
--
Regards,
Nikita.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
