[U-Boot-Users] Question on do_bootm in cmd_bootm.c

[U-Boot-Users] Question on do_bootm in cmd_bootm.c

[Nick Barendt]

	Can someone, maybe Wolfgang, comment on the maximum 4MB uncompressed size 
of gunzip (and, possibly, bzip2) images?
	I've been working on a Coldfire 5272 system for quite some time with 
u-boot and uClinux.  Both have been remarkably stable.  We use the uClinux 
default build wherein the root filesystem (romfs) is appended to the end of 
the kernel image, starting where the bss would normally be.  The kernel 
startup code relocates the romfs image to just beyond the bss, then, and 
everything runs smoothly.  I know that many folks in the PPC world use 
separate kernel and root filesystem images, for many good reasons, but many 
(most?) of the uClinux platforms use this concatenated kernel+romfs image.
	Recently, we've been adding more content to the root filesystem.  As our 
root filesystem grew larger (say about 3MB), however, we started seeing 
boot problems where the kernel would boot fine but the root filesystem was 
clearly corrupt.  I've spent the better part of two days trying to track 
down where things go wrong, from the kernel to u-boot.  Of course, I used a 
hardware debugger, looked at u-boot's memory assignments, image load 
addresses, entry points, malloc areas, etc.
	I just located the source of my trouble, a hard-coded 4MB maximum 
uncompressed size in do_bootm:

int do_bootm (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
{
	ulong	iflag;
	ulong	addr;
	ulong	data, len, checksum;
	ulong  *len_ptr;
	uint	unc_len = 0x400000;
                          ^^^^^^^^^
	int	i, verify;

SNIP

	case IH_COMP_GZIP:
		printf ("   Uncompressing %s ... ", name);
		if (gunzip ((void *)ntohl(hdr->ih_load), unc_len,
							^^^^^^^^
			    (uchar *)data, (int *)&len) != 0) {
			printf ("GUNZIP ERROR - must RESET board to recover\n");
			SHOW_BOOT_PROGRESS (-6);
			do_reset (cmdtp, flag, argc, argv);
		}
		break;

SNIP

	Increasing unc_len to larger values (like 6MB) seems to solve my problem.
	With an uncompressed image larger than 4MB, gunzip doesn't seem to 
generate any errors (granted, I haven't stepped through gunzip and zlib), 
but merrily runs along, although the data beyond the 4MB boundary seems to 
be missing or corrupt.  It looks like there's a truncation (about line 1875 
in zlib.c) to z.avail_out in zlib that prevents data from being copied 
beyond unc_len - maybe that should be an error instead of silently 
truncating data?  Just a thought.  I'll into submitting a patch for that.
	I was wondering if there was some reason for the 0x400000 magic number for 
unc_len.  Is that just a magic number?  Is that larger than anyone uses for 
images?  I guess I'm a little surprised that this hasn't bitten anyone else 
so far (nothing I could see on the mailing list archive, at least).  Our 
1MB kernel and 3MB romfs root filesystem might be exceptionally large, I 
don't know.
	Would there be any issues with making unc_len a CFG_ macro, if I submitted 
a patch?

Nick Barendt

[U-Boot-Users] Question on do_bootm in cmd_bootm.c

[Dan Malek]

On Oct 21, 2004, at 8:08 PM, Nick Barendt wrote:

>
> 	Can someone, maybe Wolfgang, comment on the maximum 4MB uncompressed 
> size of gunzip (and, possibly, bzip2) images?

This was probably copied from the PowerPC Linux "bootloader" source
code, which was created a long, long time ago.  We used this value
in that code because long ago when compressed images were necessary
the embedded boards were lucky to have 8M of RAM.  Based on the (then
fixed) load address and sizes of memory, you couldn't uncompress images
larger than this without writing over the uncompressing code.  To 
prevent
large images from just hanging the system, we stopped short and 
continued
to boot.  At least this way we got an error later that the ramdisk isn't
correct.  Back then it was common to know this kind of failure.

> 	Would there be any issues with making unc_len a CFG_ macro, if I 
> submitted a patch?

I don't think a configuration macro is appropriate, just submit a patch
with a much larger and still reasonable value, maybe 16M or so.

Thanks.


	-- Dan

[U-Boot-Users] Question on do_bootm in cmd_bootm.c

[Craig Hughes]

"16 Meg ought to be enough for anybody"

C

On Oct 22, 2004, at 10:49 AM, Dan Malek wrote:

> I don't think a configuration macro is appropriate, just submit a patch
> with a much larger and still reasonable value, maybe 16M or so.

[U-Boot-Users] Question on do_bootm in cmd_bootm.c

[Wolfgang Denk]

In message <2687F354-245C-11D9-9164-000A95A5473A@gumstix.com> you wrote:
> "16 Meg ought to be enough for anybody"

For how long?

Best regards,

Wolfgang Denk

-- 
Software Engineering:  Embedded and Realtime Systems,  Embedded Linux
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
If the hours are long enough and the pay  is  short  enough,  someone
will say it's women's work.

[U-Boot-Users] Question on do_bootm in cmd_bootm.c

[Nick Barendt]

At 09:12 PM 10/22/2004 +0200, you wrote:
>In message <2687F354-245C-11D9-9164-000A95A5473A@gumstix.com> you wrote:
> > "16 Meg ought to be enough for anybody"
>
>For how long?


I think this should be handled in a very similar manner to the 
CFG_MALLOC_LEN, but I don't want to make everyone update their config 
header files.  So, I propose a patch that leaves 4MB as a default, allows 
board config header files to override the 4MB default, and makes zlib 
return an error if the output buffer fills up so these types of errors are 
painfully obvious.  How does that sound?


>Best regards,
>
>Wolfgang Denk
>
>--
>Software Engineering:  Embedded and Realtime Systems,  Embedded Linux
>Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
>If the hours are long enough and the pay  is  short  enough,  someone
>will say it's women's work.
>
>
>-------------------------------------------------------
>This SF.net email is sponsored by: IT Product Guide on ITManagersJournal
>Use IT products in your business? Tell us what you think of them. Give us
>Your Opinions, Get Free ThinkGeek Gift Certificates! Click to find out more
>http://productguide.itmanagersjournal.com/guidepromo.tmpl
>_______________________________________________
>U-Boot-Users mailing list
>U-Boot-Users at lists.sourceforge.net
>https://lists.sourceforge.net/lists/listinfo/u-boot-users

[U-Boot-Users] Question on do_bootm in cmd_bootm.c

[Dan Malek]

On Oct 22, 2004, at 3:12 PM, Wolfgang Denk wrote:

> For how long?

:-)  For many of such constants, I was hoping to be retired before
I had to care when they changed again.  Unfortunately, my choice
of start up companies has caused my need to continue working. :-)


	-- Dan

[U-Boot-Users] Question on do_bootm in cmd_bootm.c

[Dan Malek]

On Oct 22, 2004, at 3:22 PM, Nick Barendt wrote:

> .... So, I propose a patch ...

Let's see it :-)

> .... and makes zlib return an error if the output buffer fills up so 
> these types of errors are painfully obvious.

It may already return some error status if we choose to test it.


	-- Dan

[U-Boot-Users] Question on do_bootm in cmd_bootm.c

[Nick Barendt]

At 03:58 PM 10/22/2004 -0400, Dan Malek wrote:

>On Oct 22, 2004, at 3:22 PM, Nick Barendt wrote:
>
>>.... So, I propose a patch ...
>
>Let's see it :-)


I've made all of the mods, but I won't have time to get a patch together 
(crossing my t's, dotting my i's, CHANGELOG entry, etc.) today, and I'm out 
of the office next week.  I'll get the patch together and mail it out when 
I get back.


>>.... and makes zlib return an error if the output buffer fills up so 
>>these types of errors are painfully obvious.
>
>It may already return some error status if we choose to test it.
>

Nope, no error is returned.  inflate_flush() in zlib.c just silently 
truncates the output:

local int inflate_flush(s, z, r)
inflate_blocks_statef *s;
z_stream *z;
int r;
{

SNIP

   /* compute number of bytes to copy as far as end of window */
   n = (uInt)((q <= s->write ? s->write : s->end) - q);
   if (n > z->avail_out) n = z->avail_out;
   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

SNIP

The bzip2 code actually returns a specific error for this, 
BZ_OUTPUT_BUFF_FULL.  I've modified zlib to behave similarly.

>         -- Dan
>

[U-Boot-Users] Question on do_bootm in cmd_bootm.c

[Wolfgang Denk]

Dear Nick,

in message <5.2.1.1.0.20041022160135.0355c7d8@mail.atiba.com> you wrote:
> 
> I've made all of the mods, but I won't have time to get a patch together 
> (crossing my t's, dotting my i's, CHANGELOG entry, etc.) today, and I'm out 
> of the office next week.  I'll get the patch together and mail it out when 
> I get back.

I guess you forgot about this, but it's still open in my list of issues.

Can you please submit your patch?

...

> The bzip2 code actually returns a specific error for this, 
> BZ_OUTPUT_BUFF_FULL.  I've modified zlib to behave similarly.

I'd really like to add this...

Best regards,

Wolfgang Denk

-- 
Software Engineering:  Embedded and Realtime Systems,  Embedded Linux
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Single tasking: Just Say No.