initrd_size inflation breaks initramfs

initrd_size inflation breaks initramfs

[dann frazier]

hey,
  I've been trying to track down why initramfs seems to be broken on
ia64 when passed using the initrd= flag.  This appears to be due to an
inflated ia64_boot_param->initrd_size.  In my case, initrd_size got set
to 1957888 (which happens to be a multiple of 4K), when my initramfs
file is actually 1957415 bytes.

unpack_to_rootfs() expects gunzip() to succeed until it has reached the
initrd_size.  Since it thinks the image is larger than it actually is,
it runs off the end of the initrd and the initramfs footprinting fails.
I can get unpack_to_rootfs to succeed by hardcoding my initrd_size.

My guess is that this is a bug in elilo.

initrd.c has:
        /* round up to get required number of pages (4KB) */
        initrd->pgcnt = pgcnt = EFI_SIZE_TO_PAGES(size);

initrd->pgcnt is later used to calculate the initrd_size boot param, but
it is now (size % 4096) bytes too big.
-- 
dann frazier <dannf@hp.com>

Re: initrd_size inflation breaks initramfs

[Andreas Schwab]

dann frazier <dannf@hp.com> writes:

> My guess is that this is a bug in elilo.

Try this patch:

--- initrd.c
+++ initrd.c
@@ -94,6 +94,9 @@
 
 	initrd->start_addr = start_addr;
 
+	/* Clear extra memory since we don't pass the exact size.  */
+	Memset(start_addr + size, 0, (pgcnt << EFI_PAGE_SHIFT) - size);
+
 	return ELILO_LOAD_SUCCESS;
 
 error:

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: initrd_size inflation breaks initramfs

[dann frazier]

On Mon, 2005-11-28 at 10:44 +0100, Andreas Schwab wrote:
> dann frazier <dannf@hp.com> writes:
> 
> > My guess is that this is a bug in elilo.
> 
> Try this patch:
> 
> --- initrd.c
> +++ initrd.c
> @@ -94,6 +94,9 @@
>  
>  	initrd->start_addr = start_addr;
>  
> +	/* Clear extra memory since we don't pass the exact size.  */
> +	Memset(start_addr + size, 0, (pgcnt << EFI_PAGE_SHIFT) - size);
> +
>  	return ELILO_LOAD_SUCCESS;
>  
>  error:
> 

Thanks Andreas, that works.  Though I wonder if it wouldn't be better to
keep track of the actual initrd size?

-- 
dann frazier <dannf@hp.com>

Re: initrd_size inflation breaks initramfs

[Jesse Barnes]

On Sunday, November 27, 2005 10:23 pm, dann frazier wrote:
> hey,
>   I've been trying to track down why initramfs seems to be broken on
> ia64 when passed using the initrd= flag.  This appears to be due to an
> inflated ia64_boot_param->initrd_size.  In my case, initrd_size got
> set to 1957888 (which happens to be a multiple of 4K), when my
> initramfs file is actually 1957415 bytes.
>
> unpack_to_rootfs() expects gunzip() to succeed until it has reached
> the initrd_size.  Since it thinks the image is larger than it actually
> is, it runs off the end of the initrd and the initramfs footprinting
> fails. I can get unpack_to_rootfs to succeed by hardcoding my
> initrd_size.
>
> My guess is that this is a bug in elilo.
>
> initrd.c has:
>         /* round up to get required number of pages (4KB) */
>         initrd->pgcnt = pgcnt = EFI_SIZE_TO_PAGES(size);
>
> initrd->pgcnt is later used to calculate the initrd_size boot param,
> but it is now (size % 4096) bytes too big.

I thought this bug was fixed?  I remember Erik J. and I seeing the same 
thing, and iirc we corrected elilo and sent the patch upstream.  Or is 
this a different problem maybe?

Jesse

Re: initrd_size inflation breaks initramfs

[dann frazier]

On Mon, 2005-11-28 at 08:15 -0800, Jesse Barnes wrote:
> On Sunday, November 27, 2005 10:23 pm, dann frazier wrote:
> > hey,
> >   I've been trying to track down why initramfs seems to be broken on
> > ia64 when passed using the initrd= flag.  This appears to be due to an
> > inflated ia64_boot_param->initrd_size.  In my case, initrd_size got
> > set to 1957888 (which happens to be a multiple of 4K), when my
> > initramfs file is actually 1957415 bytes.
> >
> > unpack_to_rootfs() expects gunzip() to succeed until it has reached
> > the initrd_size.  Since it thinks the image is larger than it actually
> > is, it runs off the end of the initrd and the initramfs footprinting
> > fails. I can get unpack_to_rootfs to succeed by hardcoding my
> > initrd_size.
> >
> > My guess is that this is a bug in elilo.
> >
> > initrd.c has:
> >         /* round up to get required number of pages (4KB) */
> >         initrd->pgcnt = pgcnt = EFI_SIZE_TO_PAGES(size);
> >
> > initrd->pgcnt is later used to calculate the initrd_size boot param,
> > but it is now (size % 4096) bytes too big.
> 
> I thought this bug was fixed?  I remember Erik J. and I seeing the same 
> thing, and iirc we corrected elilo and sent the patch upstream.  Or is 
> this a different problem maybe?

I'm seeing this with Debian's 3.4-9 - 3.4 appears to be the latest
release here:
  ftp://ftp.hpl.hp.com/pub/linux-ia64/

But it appears to be > 2 years old.

Re: initrd_size inflation breaks initramfs

[Matthew Wilcox]

On Mon, Nov 28, 2005 at 09:24:33AM -0700, dann frazier wrote:
> I'm seeing this with Debian's 3.4-9 - 3.4 appears to be the latest
> release here:
>   ftp://ftp.hpl.hp.com/pub/linux-ia64/
> 
> But it appears to be > 2 years old.

Upstream moved:

http://elilo.sourceforge.net/cgi-bin/blosxom

3.5-pre1 says:
 1) Support for compressed ia32 kernels.
 2) Support for subnet-specific config files.
 3) initrd handling bug fixed.

Re: initrd_size inflation breaks initramfs

[dann frazier]

On Mon, 2005-11-28 at 09:24 -0700, dann frazier wrote:
> I'm seeing this with Debian's 3.4-9 - 3.4 appears to be the latest
> release here:
>   ftp://ftp.hpl.hp.com/pub/linux-ia64/
> 
> But it appears to be > 2 years old.

Ah - found the sourceforge page - looks like a patch to fix this went in
in February, but just missed the 3.5pre1 release.
-- 
dann frazier <dannf@hp.com>

Re: initrd_size inflation breaks initramfs

[Brett Johnson]

[-- Attachment #1: Type: text/plain, Size: 331 bytes --]

On Mon, 2005-11-28 at 09:28 -0700, dann frazier wrote:
> Ah - found the sourceforge page - looks like a patch to fix this went in
> in February, but just missed the 3.5pre1 release.

Here's the actual patch that I applied to 3.5pre1 to fix this bug.  Man,
I really need to make another release...

-- 
Brett Johnson <brett@hp.com>

[-- Attachment #2: initrd-changes.diff --]
[-- Type: text/x-patch, Size: 3491 bytes --]

Index: elilo.c
===================================================================
RCS file: /cvsroot/elilo/elilo/elilo.c,v
retrieving revision 1.3
retrieving revision 1.6
diff -b -B -u -r1.3 -r1.6
--- elilo.c	20 Feb 2004 22:30:37 -0000	1.3
+++ elilo.c	28 Feb 2005 17:57:28 -0000	1.6
@@ -164,7 +163,7 @@
 
 	for(;;) {
 		kname[0] = cmdline_tmp[0] = cmdline[0] = CHAR_NULL;
-		imem.start_addr = 0; imem.pgcnt = 0;
+		imem.start_addr = 0; imem.pgcnt = 0; imem.size = 0;
 		elilo_opt.sys_img_opts = NULL;
 
 		if (kernel_chooser(argv, argc, index, kname, cmdline_tmp) == -1) goto exit_error;
Index: elilo.h
===================================================================
RCS file: /cvsroot/elilo/elilo/elilo.h,v
retrieving revision 1.3
retrieving revision 1.4
diff -b -B -u -r1.3 -r1.4
--- elilo.h	20 Feb 2004 22:30:37 -0000	1.3
+++ elilo.h	28 Feb 2005 17:57:28 -0000	1.4
@@ -112,6 +112,7 @@
 typedef struct {
 	VOID 	*start_addr;
 	UINTN	pgcnt;
+	UINTN	size;
 } memdesc_t;
 
 typedef struct {
Index: initrd.c
===================================================================
RCS file: /cvsroot/elilo/elilo/initrd.c,v
retrieving revision 1.1.1.1
retrieving revision 1.2
diff -b -B -u -r1.1.1.1 -r1.2
--- initrd.c	19 Aug 2003 16:47:18 -0000	1.1.1.1
+++ initrd.c	28 Feb 2005 17:57:28 -0000	1.2
@@ -66,10 +66,10 @@
 		goto error;
 	}
 	
-	/* round up to get required number of pages (4KB) */
-	initrd->pgcnt = pgcnt = EFI_SIZE_TO_PAGES(size);
-
+	initrd->size = size;
 
+	/* round up to get required number of pages (4KB) */
+	initrd->pgcnt = pgcnt = EFI_SIZE_TO_PAGES(initrd->size);
 
 	start_addr = alloc_pages(pgcnt, EfiLoaderData, start_addr ? AllocateAddress : AllocateAnyPages, start_addr);
 	if (start_addr == NULL) {
@@ -77,11 +77,11 @@
 		goto error;
 	}
 	VERB_PRT(2, Print(L"initrd: total_size: %ld bytes base: 0x%lx pages %d\n", 
-			size, (UINT64)start_addr, pgcnt));
+			initrd->size, (UINTN)start_addr, pgcnt));
 
 	Print(L"Loading initrd %s...", filename);
 
-	ret = read_file(fd, size, start_addr);	
+	ret = read_file(fd, initrd->size, start_addr);	
 
 	fops_close(fd);
 
@@ -105,6 +105,7 @@
 	 */
 	initrd->start_addr = 0;
 	initrd->pgcnt      = 0;
+	initrd->size       = 0;
 
 	return ret;
 }
Index: ia32/system.c
===================================================================
RCS file: /cvsroot/elilo/elilo/ia32/system.c,v
retrieving revision 1.2
retrieving revision 1.3
diff -b -B -u -r1.2 -r1.3
--- ia32/system.c	17 Feb 2004 23:42:41 -0000	1.2
+++ ia32/system.c	28 Feb 2005 17:57:30 -0000	1.3
@@ -270,7 +270,7 @@
 	if (initrd->start_addr && initrd->pgcnt) {
 		/* %%TBD - This will probably have to be changed. */
 		bp->s.initrd_start = (UINT32)initrd->start_addr;
-		bp->s.initrd_size = (UINT32)(initrd->pgcnt * EFI_PAGE_SIZE);
+		bp->s.initrd_size = (UINT32)(initrd->size);
 
 		/*
 		 * This is the RAMdisk root device for RedHat 2.2.x
Index: ia64/system.c
===================================================================
RCS file: /cvsroot/elilo/elilo/ia64/system.c,v
retrieving revision 1.1.1.1
retrieving revision 1.2
diff -b -B -u -r1.1.1.1 -r1.2
--- ia64/system.c	19 Aug 2003 16:46:49 -0000	1.1.1.1
+++ ia64/system.c	28 Feb 2005 17:57:30 -0000	1.2
@@ -64,7 +64,7 @@
 	bp->efi_memdesc_version = mdesc.desc_version;
 	bp->command_line	= (UINTN)cmdline;
 	bp->initrd_start	= (UINTN) initrd->start_addr;
-	bp->initrd_size		= initrd->pgcnt << EFI_PAGE_SHIFT;
+	bp->initrd_size		= initrd->size;
 
 	/* fetch console parameters: */
 	conout = systab->ConOut;