Patchwork [U-Boot,1/5] Add bmp_layout module for accessing BMP header data

login
register
mail settings
Submitter Nikita Kiryanov
Date Feb. 4, 2013, 11:39 a.m.
Message ID <1359977979-28585-2-git-send-email-nikita@compulab.co.il>
Download mbox | patch
Permalink /patch/217875/
State Rejected
Delegated to: Wolfgang Denk
Headers show

Comments

Nikita Kiryanov - Feb. 4, 2013, 11:39 a.m.
Currently code that displays BMP files does two things:
* assume that any address is a valid load address for a BMP
* access in-memory BMP header fields directly

Since some BMP header fields are 32 bit wide, this has a potential
for causing data aborts when these fields are placed in unaligned
addresses.

Create an API for safely accessing BMP header data, and compile it with
$(PLATFORM_NO_UNALIGNED) to give it the ability to emulate unaligned memory
accesses.

Signed-off-by: Nikita Kiryanov <nikita@compulab.co.il>
Signed-off-by: Igor Grinberg <grinberg@compulab.co.il>
Cc: Anatolij Gustschin <agust@denx.de>
Cc: Wolfgang Denk <wd@denx.de>
Cc: Albert ARIBAUD <albert.u.boot@aribaud.net>
Cc: Jeroen Hofstee <jeroen@myspectrum.nl>
---
 common/Makefile      |    8 +++++
 common/bmp_layout.c  |   96 ++++++++++++++++++++++++++++++++++++++++++++++++++
 include/bmp_layout.h |   15 ++++++++
 3 files changed, 119 insertions(+)
 create mode 100644 common/bmp_layout.c
Wolfgang Denk - Feb. 4, 2013, 7:26 p.m.
Dear Nikita Kiryanov,

In message <1359977979-28585-2-git-send-email-nikita@compulab.co.il> you wrote:
> Currently code that displays BMP files does two things:
> * assume that any address is a valid load address for a BMP
> * access in-memory BMP header fields directly
> 
> Since some BMP header fields are 32 bit wide, this has a potential
> for causing data aborts when these fields are placed in unaligned
> addresses.
> 
> Create an API for safely accessing BMP header data, and compile it with
> $(PLATFORM_NO_UNALIGNED) to give it the ability to emulate unaligned memory
> accesses.

Frankly,  I think this is overkill.  U-Boot is a bootloader, and it is
supposed to be lean and eficient.  We don't have all levels of safety
systems and protective devices as in, for example, an aircraft.  You
are supposed to know what you are doing, and if you ignore the rules,
you will quickly see the results yourself.

There is plenty of other areas where correct opration requires certain
alignments, and none of these are enforced by U-Boot.  And actually I
think this is not only acceptable, but good as is.

"UNIX was not designed to stop you from doing stupid things,  because
that would also stop you from doing clever things."       - Doug Gwyn


You talk about BMP header - but we also have alignment requirements
for image headers, well, even for a plain "md" or "mw" command.  And
none of these provide any protection against accidsential (or
intentional) access to unaligned addresses.

My recommendation is: just don;t do it, then.

Best regards,

Wolfgang Denk
Albert ARIBAUD - Feb. 4, 2013, 9:26 p.m.
Hi Wolfgang,

On Mon, 04 Feb 2013 20:26:18 +0100, Wolfgang Denk <wd@denx.de> wrote:

> Dear Nikita Kiryanov,
> 
> In message <1359977979-28585-2-git-send-email-nikita@compulab.co.il> you wrote:
> > Currently code that displays BMP files does two things:
> > * assume that any address is a valid load address for a BMP
> > * access in-memory BMP header fields directly
> > 
> > Since some BMP header fields are 32 bit wide, this has a potential
> > for causing data aborts when these fields are placed in unaligned
> > addresses.
> > 
> > Create an API for safely accessing BMP header data, and compile it with
> > $(PLATFORM_NO_UNALIGNED) to give it the ability to emulate unaligned memory
> > accesses.
> 
> Frankly,  I think this is overkill.  U-Boot is a bootloader, and it is
> supposed to be lean and eficient.  We don't have all levels of safety
> systems and protective devices as in, for example, an aircraft.  You
> are supposed to know what you are doing, and if you ignore the rules,
> you will quickly see the results yourself.
> 
> There is plenty of other areas where correct opration requires certain
> alignments, and none of these are enforced by U-Boot.  And actually I
> think this is not only acceptable, but good as is.
> 
> "UNIX was not designed to stop you from doing stupid things,  because
> that would also stop you from doing clever things."       - Doug Gwyn
> 
> 
> You talk about BMP header - but we also have alignment requirements
> for image headers, well, even for a plain "md" or "mw" command.  And
> none of these provide any protection against accidsential (or
> intentional) access to unaligned addresses.
> 
> My recommendation is: just don;t do it, then.

The point about md not checking alignment is indeed valid: one should
know that a md.l requires a 4-byte-aligned address or it will abort.

OTOH, a cautious user may think that to ensure proper alignment, a BMP
should be loaded on a 4-byte boundary, but IIUC that it precisely what
will cause the load to fail, due to the sole 4-byte field of the BMP
header being misaligned by two bytes.

So if we leave BMP loading as it is now, the load address will need to
be 16-bit-but-not-32-bit-aligned, which is complicated enough to
require documentation.

Or, the BMP struct could be prepended with two bytes so that the
load address alignment requirement becomes a simple 4-byte boundary,
which most users are... bound... to choose naturally.

But ISTR the idea of prepending two bytes was already discussed and for
some reason it could not work. Jeroen?

> Best regards,
> 
> Wolfgang Denk

Amicalement,
Wolfgang Denk - Feb. 4, 2013, 11:11 p.m.
Dear Albert ARIBAUD,

In message <20130204222628.545da91e@lilith> you wrote:
> 
> The point about md not checking alignment is indeed valid: one should
> know that a md.l requires a 4-byte-aligned address or it will abort.

Or, one might do this _intentionally_ for some testing purposes.
For me it is of utmost importance that U-Boot does not come in my way
in such things.  It is supposed to do _exactly_ what I ask it to do,
even if this appears to be a stupid thing.

> OTOH, a cautious user may think that to ensure proper alignment, a BMP
> should be loaded on a 4-byte boundary, but IIUC that it precisely what
> will cause the load to fail, due to the sole 4-byte field of the BMP
> header being misaligned by two bytes.

Sole 4 byte field?  The bitmap file header has actually two 32-bit
entries: file_size, and data_offset. [The "reserved" entry as we are
using it should actually be two 16 bit entries, at least according to
[1]).

Yes, struct bmp_header is a packed array with non-natural order of
entries; see also section "Bitmap file header" at [1]; we may consider
this a really stupid thing to do, but we have to live with this fact.

[1] http://en.wikipedia.org/wiki/BMP_file_format

As I understand the problem comes from the fact, that this issue has
long been undetected, because the PTB that were/are responsible for
GCC on ARM had decided that any access to apacked structure would be
silently broken down into byte accesses, no matter what the actual
data type was.  While more recent versions of GCC started actually
attempting 16 or 32 bit accesses, which failed on some systems.

> So if we leave BMP loading as it is now, the load address will need to
> be 16-bit-but-not-32-bit-aligned, which is complicated enough to
> require documentation.

Indeed, this should be documented.  And eventually the bmp command
should print a warning message if it sees other alignment.

> Or, the BMP struct could be prepended with two bytes so that the
> load address alignment requirement becomes a simple 4-byte boundary,
> which most users are... bound... to choose naturally.

That would violate the "standard" defining the BMP header structure.


Best regards,

Wolfgang Denk
Nikita Kiryanov - Feb. 5, 2013, 6:52 a.m.
Dear Wolfgang Denk,

On 02/04/2013 09:26 PM, Wolfgang Denk wrote:
> Dear Nikita Kiryanov,
>
> In message <1359977979-28585-2-git-send-email-nikita@compulab.co.il> you wrote:
>> Currently code that displays BMP files does two things:
>> * assume that any address is a valid load address for a BMP
>> * access in-memory BMP header fields directly
>>
>> Since some BMP header fields are 32 bit wide, this has a potential
>> for causing data aborts when these fields are placed in unaligned
>> addresses.
>>
>> Create an API for safely accessing BMP header data, and compile it with
>> $(PLATFORM_NO_UNALIGNED) to give it the ability to emulate unaligned memory
>> accesses.
>
> Frankly,  I think this is overkill.  U-Boot is a bootloader, and it is
> supposed to be lean and eficient.  We don't have all levels of safety
> systems and protective devices as in, for example, an aircraft.  You
> are supposed to know what you are doing, and if you ignore the rules,
> you will quickly see the results yourself.

[...]

> You talk about BMP header - but we also have alignment requirements
> for image headers, well, even for a plain "md" or "mw" command.  And
> none of these provide any protection against accidsential (or
> intentional) access to unaligned addresses.

That's true, but when md traps you simply restart the board and
everything's fine. If displaying a splash screen traps- you're stuck.
I'm not saying we should start implementing protection against every
possible mistake, but when the repercussions are this serious I feel
that protection is in order.

There's a difference between a bicycle with no training wheels and one
that falls apart when you turn it the wrong way.

>
> My recommendation is: just don;t do it, then.
>
> Best regards,
>
> Wolfgang Denk
>
Igor Grinberg - Feb. 5, 2013, 8:07 a.m.
On 02/05/13 01:11, Wolfgang Denk wrote:
> Dear Albert ARIBAUD,
> 
> In message <20130204222628.545da91e@lilith> you wrote:
>>
>> The point about md not checking alignment is indeed valid: one should
>> know that a md.l requires a 4-byte-aligned address or it will abort.
> 
> Or, one might do this _intentionally_ for some testing purposes.
> For me it is of utmost importance that U-Boot does not come in my way
> in such things.  It is supposed to do _exactly_ what I ask it to do,
> even if this appears to be a stupid thing.

I agree on this one, except for the case where doing that stupid thing
bricks the board.

> 
>> OTOH, a cautious user may think that to ensure proper alignment, a BMP
>> should be loaded on a 4-byte boundary, but IIUC that it precisely what
>> will cause the load to fail, due to the sole 4-byte field of the BMP
>> header being misaligned by two bytes.
> 
> Sole 4 byte field?  The bitmap file header has actually two 32-bit
> entries: file_size, and data_offset. [The "reserved" entry as we are
> using it should actually be two 16 bit entries, at least according to
> [1]).
> 
> Yes, struct bmp_header is a packed array with non-natural order of
> entries; see also section "Bitmap file header" at [1]; we may consider
> this a really stupid thing to do, but we have to live with this fact.

It was not that stupid in times of DOS and Win 3.1
when int was 16 bits long (and DRAM was 64K or even less)...

> 
> [1] http://en.wikipedia.org/wiki/BMP_file_format
> 
> As I understand the problem comes from the fact, that this issue has
> long been undetected, because the PTB that were/are responsible for
> GCC on ARM had decided that any access to apacked structure would be
> silently broken down into byte accesses, no matter what the actual
> data type was.  While more recent versions of GCC started actually
> attempting 16 or 32 bit accesses, which failed on some systems.
> 
>> So if we leave BMP loading as it is now, the load address will need to
>> be 16-bit-but-not-32-bit-aligned, which is complicated enough to
>> require documentation.
> 
> Indeed, this should be documented.  And eventually the bmp command
> should print a warning message if it sees other alignment.

Agreed on this also, but again what about the board brick case?
Should we add the check for alignment and if it does not properly aligned
deny further bmp header processing along with printing a warning?

> 
>> Or, the BMP struct could be prepended with two bytes so that the
>> load address alignment requirement becomes a simple 4-byte boundary,
>> which most users are... bound... to choose naturally.
> 
> That would violate the "standard" defining the BMP header structure.

Yep, I would not want this to happen.
Wolfgang Denk - Feb. 5, 2013, 9:47 a.m.
Dear Nikita,

In message <5110AC16.9000306@compulab.co.il> you wrote:
> 
> That's true, but when md traps you simply restart the board and
> everything's fine. If displaying a splash screen traps- you're stuck.

In such a case you have forgotten to test your settings before
activation these as default, i. e. you have committed a sin that
carries with it its own punishment ;-)

> There's a difference between a bicycle with no training wheels and one
> that falls apart when you turn it the wrong way.

It's not the bicyle falling apart, it's the rider falling down (with
the risk of getting hurt) - and yes, exactly this happens in real
life when you disobey basic caution.

The same will happen for other stupid settings, too - like setting a
bootdelay of 0 with a non-working bootcmd; or incorrect update commands
that blow away U-Boot, or ...

This is a boot loader, and there are no seatbelts or airbags; nothing
prevents you to shoot yourself into the foot.

Best regards,

Wolfgang Denk
Wolfgang Denk - Feb. 5, 2013, 9:57 a.m.
Dear Igor Grinberg,

In message <5110BDB2.8040800@compulab.co.il> you wrote:
>
> > Yes, struct bmp_header is a packed array with non-natural order of
> > entries; see also section "Bitmap file header" at [1]; we may consider
> > this a really stupid thing to do, but we have to live with this fact.
> 
> It was not that stupid in times of DOS and Win 3.1
> when int was 16 bits long (and DRAM was 64K or even less)...

It was as stupid then, too.  At that time, a large number of systems
with similar alignment requirements existed, and experience with these
existed, too.

Do you remember the "The Ten Commandments for C Programmers"?  If not,
look them up and check how old these are.  Note especially the ``All
the world's a VAX'' part - this is exactly what we see here in
operation.

The design of the BMP header is just BRAINDEAD.

> > Indeed, this should be documented.  And eventually the bmp command
> > should print a warning message if it sees other alignment.
> 
> Agreed on this also, but again what about the board brick case?

There is a ton of ways to brick a board.  Why should we add protection
for one special case while we agree to leave the 50 other ways
onfixed?

> Should we add the check for alignment and if it does not properly aligned
> deny further bmp header processing along with printing a warning?

Why should we?  Who tells that this is not perfectly legal on the
running system?


Let me repeat it: U-Boot is a boot loader.  It is not intended for
meddling by avarage Johnny Loser, but for system programmers who know
what they are doing. And anyone doing such things is well adviced to
_test_ his settings on the command line before storing these for
automatic use.  As I mentioned before, omitting such tests is a sin
that carries with it its own punishment.


Best regards,

Wolfgang Denk
Igor Grinberg - Feb. 5, 2013, 11:38 a.m.
On 02/05/13 11:57, Wolfgang Denk wrote:
> Dear Igor Grinberg,
> 
> In message <5110BDB2.8040800@compulab.co.il> you wrote:
>>
>>> Yes, struct bmp_header is a packed array with non-natural order of
>>> entries; see also section "Bitmap file header" at [1]; we may consider
>>> this a really stupid thing to do, but we have to live with this fact.
>>
>> It was not that stupid in times of DOS and Win 3.1
>> when int was 16 bits long (and DRAM was 64K or even less)...
> 
> It was as stupid then, too.  At that time, a large number of systems
> with similar alignment requirements existed, and experience with these
> existed, too.
> 
> Do you remember the "The Ten Commandments for C Programmers"?  If not,
> look them up and check how old these are.  Note especially the ``All
> the world's a VAX'' part - this is exactly what we see here in
> operation.

Yep. Agreed on this.
Although, I don't know, if anyone of us would tell the same 20+ years ago...
It is now we can...

> 
> The design of the BMP header is just BRAINDEAD.

That is for sure.

> 
>>> Indeed, this should be documented.  And eventually the bmp command
>>> should print a warning message if it sees other alignment.
>>
>> Agreed on this also, but again what about the board brick case?
> 
> There is a ton of ways to brick a board.  Why should we add protection
> for one special case while we agree to leave the 50 other ways
> onfixed?

Because, I think this one is a bit different because of the bmp header
and also is of very high demand.

> 
>> Should we add the check for alignment and if it does not properly aligned
>> deny further bmp header processing along with printing a warning?
> 
> Why should we?  Who tells that this is not perfectly legal on the
> running system?

It might be perfectly legal, but we also consider a tons of work
explaining why and how this should be done (needless to mention the
amount of bricked boards).
Instead of simplifying the case, and I think this is a special case,
at least because of the high demand and user (who is not a programmer)
selectable address, you want us to teach the whole world about the bmp
header alignment issues?

> 
> 
> Let me repeat it: U-Boot is a boot loader.  It is not intended for
> meddling by avarage Johnny Loser, but for system programmers who know
> what they are doing. And anyone doing such things is well adviced to
> _test_ his settings on the command line before storing these for
> automatic use.  As I mentioned before, omitting such tests is a sin
> that carries with it its own punishment.

What are you trying to say?
Is it that the environment variables change and in particular
the splash screen installation _must_ be done by a programmer?
Hmm.. that does not sound good...
Wolfgang Denk - Feb. 5, 2013, 1:20 p.m.
Dear Igor,

In message <5110EF4F.3080308@compulab.co.il> you wrote:
>
> > Why should we?  Who tells that this is not perfectly legal on the
> > running system?
> 
> It might be perfectly legal, but we also consider a tons of work
> explaining why and how this should be done (needless to mention the
> amount of bricked boards).

Please understand that I will not really buy this "bricked bord"
argument.  This is an issue where system builders and users are
involved.  Apparently the system builders agree that performance
is so important that they compile with optimizer options that do
not tolerate unaligned accesses, thus introducing the problem.
This is OK for systems where only educated users have access.  If you
open the U-Boot console interface to uneducated users, you are always
running some risk that a stupid command will brick the board or at
least make it no longer usable to that user.  And as a user you should
well be aware that bad things can happen, and that it is an excellent
idea to actually test any new settings or operations before installing
these.  If users ignore even such basic rules, then the situation is
f*cked up and cannot be helped - if it's not the spalsh screen, then
these users will find other ways to run into trouble.

> > Let me repeat it: U-Boot is a boot loader.  It is not intended for
> > meddling by avarage Johnny Loser, but for system programmers who know
> > what they are doing. And anyone doing such things is well adviced to
> > _test_ his settings on the command line before storing these for
> > automatic use.  As I mentioned before, omitting such tests is a sin
> > that carries with it its own punishment.
> 
> What are you trying to say?
> Is it that the environment variables change and in particular
> the splash screen installation _must_ be done by a programmer?

I tried to be clear: people who work on such a level are supposed to
know what they are doing.


I find it interesting that a lot of arguments get raised here how
important this issue is (is it? who has actually bricked a system
this way?), and how that is a special case (here I disagree), but
so far you all appear to ignore my argument of testing settings
before putting these to use.

I see little excuse for neglecting such really basic diligence.

Best regards,

Wolfgang Denk
Igor Grinberg - Feb. 5, 2013, 2:49 p.m.
On 02/05/13 15:20, Wolfgang Denk wrote:
> Dear Igor,
> 
> In message <5110EF4F.3080308@compulab.co.il> you wrote:
>>
>>> Why should we?  Who tells that this is not perfectly legal on the
>>> running system?
>>
>> It might be perfectly legal, but we also consider a tons of work
>> explaining why and how this should be done (needless to mention the
>> amount of bricked boards).
> 
> Please understand that I will not really buy this "bricked bord"
> argument.  This is an issue where system builders and users are
> involved.  Apparently the system builders agree that performance
> is so important that they compile with optimizer options that do
> not tolerate unaligned accesses, thus introducing the problem.
> This is OK for systems where only educated users have access.  If you
> open the U-Boot console interface to uneducated users, you are always
> running some risk that a stupid command will brick the board or at
> least make it no longer usable to that user.  And as a user you should
> well be aware that bad things can happen, and that it is an excellent
> idea to actually test any new settings or operations before installing
> these.  If users ignore even such basic rules, then the situation is
> f*cked up and cannot be helped - if it's not the spalsh screen, then
> these users will find other ways to run into trouble.

Totally... I agree the bricked bored ;-) should not be an argument,
but the tons of work might be...

> 
>>> Let me repeat it: U-Boot is a boot loader.  It is not intended for
>>> meddling by avarage Johnny Loser, but for system programmers who know
>>> what they are doing. And anyone doing such things is well adviced to
>>> _test_ his settings on the command line before storing these for
>>> automatic use.  As I mentioned before, omitting such tests is a sin
>>> that carries with it its own punishment.
>>
>> What are you trying to say?
>> Is it that the environment variables change and in particular
>> the splash screen installation _must_ be done by a programmer?
> 
> I tried to be clear: people who work on such a level are supposed to
> know what they are doing.
> 
> 
> I find it interesting that a lot of arguments get raised here how
> important this issue is (is it? who has actually bricked a system
> this way?),

This is a relatively new default setting for the compiler,
and I think this is the reason why you (still) haven't heard
about it.
Also, do you really expect that whoever gets the board bricked
will go complaining to the mailing list?
I know many users, that don't even know about the mailing list
existence at all and they don't care...
What they do care is for the feature to work and have a simple
yet usable user API.

> and how that is a special case (here I disagree), but
> so far you all appear to ignore my argument of testing settings
> before putting these to use.

Loading of the splash screen has been a part of the U-Boot boot
sequence for ages, so the test of the feature is roughly just place
the bmp image in the right place on the storage device, set the
splashimage variable and reset the board.
They don't think about the new compiler right away and they don't
think about the bmp header. All they think about is: "I always did
it like this, so lets do it the same way...", and here comes the
new compiler default...

Now, I agree with you, that the above might be not the best way.
And I agree that U-Boot, as a boot loader, should be just a dumb
piece of code that does whatever the user/programmer tells it to do.

Is there any argument, from what was said in this (and other) thread,
you agree with?
Can you propose a feasible (yet not too expensive and not out of
mainline tree) solution?
Wolfgang Denk - Feb. 5, 2013, 7:27 p.m.
Dear Igor,

In message <51111C0F.60404@compulab.co.il> you wrote:
>
> This is a relatively new default setting for the compiler,
> and I think this is the reason why you (still) haven't heard
> about it.
> Also, do you really expect that whoever gets the board bricked
> will go complaining to the mailing list?

Then how do we kno if this is just an anticipated or a real problem?

> Loading of the splash screen has been a part of the U-Boot boot
> sequence for ages, so the test of the feature is roughly just place
> the bmp image in the right place on the storage device, set the
> splashimage variable and reset the board.

Maybe I'm so weird then - I think I have never blindly installed a
splash screen without checking before that it actually works and looks
good, using the "bmp" command...

> Can you propose a feasible (yet not too expensive and not out of
> mainline tree) solution?

Well, if I understand the situation correctly, the problem we have is
that the bitmap address for the splash screen should be aligned to an
even 32 bit boundary + 1.  The address is stored in an environment
variable.

So why don't we implement a callback on that variable that checks
itd's value when it gets set?  We have all the features in place now;
and we can even overwrite such a default setting on boards where it's
not needed / wanted.

So my recommendation is: add a new alignment-checking extension as a
callback to the variable handling code.

Best regards,

Wolfgang Denk

--
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
"Have you lived in this village all your life?"        "No, not yet."

Patch

diff --git a/common/Makefile b/common/Makefile
index 54fcc81..2a972c8 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -187,7 +187,14 @@  endif
 ifdef CONFIG_SPD_EEPROM
 SPD := y
 endif
+ifdef CONFIG_CMD_BMP
+BMP_LAYOUT := y
+endif
+ifdef CONFIG_SPLASH_SCREEN
+BMP_LAYOUT := y
+endif
 COBJS-$(SPD) += ddr_spd.o
+COBJS-$(BMP_LAYOUT) += bmp_layout.o
 COBJS-$(CONFIG_HWCONFIG) += hwconfig.o
 COBJS-$(CONFIG_BOOTSTAGE) += bootstage.o
 COBJS-$(CONFIG_CONSOLE_MUX) += iomux.o
@@ -250,6 +257,7 @@  $(obj)../tools/envcrc:
 # SEE README.arm-unaligned-accesses
 $(obj)hush.o: CFLAGS += $(PLATFORM_NO_UNALIGNED)
 $(obj)fdt_support.o: CFLAGS += $(PLATFORM_NO_UNALIGNED)
+$(obj)bmp_layout.o: CFLAGS += $(PLATFORM_NO_UNALIGNED)
 
 #########################################################################
 
diff --git a/common/bmp_layout.c b/common/bmp_layout.c
new file mode 100644
index 0000000..c55b653
--- /dev/null
+++ b/common/bmp_layout.c
@@ -0,0 +1,96 @@ 
+/* (C) Copyright 2013
+ * Nikita Kiryanov, Compulab, nikita@compulab.co.il.
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#include <asm/types.h>
+#include <bmp_layout.h>
+#include <compiler.h>
+
+int bmp_signature_valid(bmp_image_t *bmp)
+{
+	return bmp->header.signature[0] == 'B' &&
+		bmp->header.signature[1] == 'M';
+}
+
+__u32 bmp_get_file_size(bmp_image_t *bmp)
+{
+	return le32_to_cpu(bmp->header.file_size);
+}
+
+__u32 bmp_get_data_offset(bmp_image_t *bmp)
+{
+	return le32_to_cpu(bmp->header.data_offset);
+}
+
+__u32 bmp_get_size(bmp_image_t *bmp)
+{
+	return le32_to_cpu(bmp->header.size);
+}
+
+__u32 bmp_get_width(bmp_image_t *bmp)
+{
+	return le32_to_cpu(bmp->header.width);
+}
+
+__u32 bmp_get_height(bmp_image_t *bmp)
+{
+	return le32_to_cpu(bmp->header.height);
+}
+
+__u16 bmp_get_planes(bmp_image_t *bmp)
+{
+	return le16_to_cpu(bmp->header.planes);
+}
+
+__u16 bmp_get_bit_count(bmp_image_t *bmp)
+{
+	return le16_to_cpu(bmp->header.bit_count);
+}
+
+__u32 bmp_get_compression(bmp_image_t *bmp)
+{
+	return le32_to_cpu(bmp->header.compression);
+}
+
+__u32 bmp_get_image_size(bmp_image_t *bmp)
+{
+	return le32_to_cpu(bmp->header.image_size);
+}
+
+__u32 bmp_get_x_pixels_per_m(bmp_image_t *bmp)
+{
+	return le32_to_cpu(bmp->header.x_pixels_per_m);
+}
+
+__u32 bmp_get_y_pixels_per_m(bmp_image_t *bmp)
+{
+	return le32_to_cpu(bmp->header.y_pixels_per_m);
+}
+
+__u32 bmp_get_colors_used(bmp_image_t *bmp)
+{
+	return le32_to_cpu(bmp->header.colors_used);
+}
+
+__u32 bmp_get_colors_important(bmp_image_t *bmp)
+{
+	return le32_to_cpu(bmp->header.colors_important);
+}
diff --git a/include/bmp_layout.h b/include/bmp_layout.h
index d823de9..a983147 100644
--- a/include/bmp_layout.h
+++ b/include/bmp_layout.h
@@ -74,4 +74,19 @@  typedef struct bmp_image {
 #define BMP_BI_RLE8	1
 #define BMP_BI_RLE4	2
 
+int bmp_signature_valid(bmp_image_t *bmp);
+__u32 bmp_get_file_size(bmp_image_t *bmp);
+__u32 bmp_get_data_offset(bmp_image_t *bmp);
+__u32 bmp_get_size(bmp_image_t *bmp);
+__u32 bmp_get_width(bmp_image_t *bmp);
+__u32 bmp_get_height(bmp_image_t *bmp);
+__u16 bmp_get_planes(bmp_image_t *bmp);
+__u16 bmp_get_bit_count(bmp_image_t *bmp);
+__u32 bmp_get_compression(bmp_image_t *bmp);
+__u32 bmp_get_image_size(bmp_image_t *bmp);
+__u32 bmp_get_x_pixels_per_m(bmp_image_t *bmp);
+__u32 bmp_get_y_pixels_per_m(bmp_image_t *bmp);
+__u32 bmp_get_colors_used(bmp_image_t *bmp);
+__u32 bmp_get_colors_important(bmp_image_t *bmp);
+
 #endif							/* _BMP_H_ */