Double free of initramfs

Double free of initramfs

[Ralf Baechle]

In all linker scripts we currently have something like this:

  __init_begin = .;
 ...
   . = ALIGN(4096);
  __initramfs_start = .;
  .init.ramfs : { *(.init.ramfs) }
  __initramfs_end = .;
  ...
  __init_end = .;

It seems all 25 linker scripts in the current bk tree are suffering from
this.  Which mean with CONFIG_BLK_DEV_INITRD enabled first free_initrd_mem
may be called to free the initram disk and just a little later
free_initmem will try to free the entire range again, so either the
linker scripts would need fixing or free_initrd_mem has become obsolete.

  Ralf

Re: Double free of initramfs

[Andrew Morton]

Ralf Baechle <ralf@linux-mips.org> wrote:
>
> In all linker scripts we currently have something like this:
> 
>   __init_begin = .;
>  ...
>    . = ALIGN(4096);
>   __initramfs_start = .;
>   .init.ramfs : { *(.init.ramfs) }
>   __initramfs_end = .;
>   ...
>   __init_end = .;
> 
> It seems all 25 linker scripts in the current bk tree are suffering from
> this.  Which mean with CONFIG_BLK_DEV_INITRD enabled first free_initrd_mem
> may be called to free the initram disk and just a little later
> free_initmem will try to free the entire range again, so either the
> linker scripts would need fixing or free_initrd_mem has become obsolete.
> 

Hard call.  I'd be 51% inclined to change the linker scripts so that the
freeing of the initram disk happens explicitly, rather than having it
magically hidden inside the main initmem segment.

Any objections from anyone?

(cc'ed Adrian, who might be interested in raising a patch ;))

Re: Double free of initramfs

[Adrian Bunk]

On Mon, Mar 21, 2005 at 06:21:46PM -0800, Andrew Morton wrote:
>...
> (cc'ed Adrian, who might be interested in raising a patch ;))

When did I get the MAINTAINERS entry
  DOES ALL DUMB JOBS
?  ;-)


If you two agree it's the correct solution I can prepare a patch.


cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: Double free of initramfs

[Ralf Baechle]

On Mon, Mar 21, 2005 at 06:21:46PM -0800, Andrew Morton wrote:

> > It seems all 25 linker scripts in the current bk tree are suffering from
> > this.  Which mean with CONFIG_BLK_DEV_INITRD enabled first free_initrd_mem
> > may be called to free the initram disk and just a little later
> > free_initmem will try to free the entire range again, so either the
> > linker scripts would need fixing or free_initrd_mem has become obsolete.
> > 
> 
> Hard call.  I'd be 51% inclined to change the linker scripts so that the
> freeing of the initram disk happens explicitly, rather than having it
> magically hidden inside the main initmem segment.
> 
> Any objections from anyone?
> 
> (cc'ed Adrian, who might be interested in raising a patch ;))

I've cooked that already.  Let me check if I can dig it up from here - I'm
supposed to be on vacation ;-)

  Ralf

Re: Double free of initramfs

[Sam Ravnborg]

On Tue, Mar 22, 2005 at 10:41:17PM +0000, Ralf Baechle wrote:
> On Mon, Mar 21, 2005 at 06:21:46PM -0800, Andrew Morton wrote:
> 
> > > It seems all 25 linker scripts in the current bk tree are suffering from
> > > this.  Which mean with CONFIG_BLK_DEV_INITRD enabled first free_initrd_mem
> > > may be called to free the initram disk and just a little later
> > > free_initmem will try to free the entire range again, so either the
> > > linker scripts would need fixing or free_initrd_mem has become obsolete.
> > > 
> > 
> > Hard call.  I'd be 51% inclined to change the linker scripts so that the
> > freeing of the initram disk happens explicitly, rather than having it
> > magically hidden inside the main initmem segment.
> > 
> > Any objections from anyone?
> > 
> > (cc'ed Adrian, who might be interested in raising a patch ;))
> 
> I've cooked that already.  Let me check if I can dig it up from here - I'm
> supposed to be on vacation ;-)
>    . = ALIGN(4096);
>   __initramfs_start = .;
>   .init.ramfs : { *(.init.ramfs) }
>   __initramfs_end = .;

Hi Ralf.

When you anyway hack on thins in your vacation please
move the definition of initramfs to asm-generic.

	Sam

Something like this:

===== include/asm-generic/vmlinux.lds.h 1.18 vs edited =====
--- 1.18/include/asm-generic/vmlinux.lds.h	2005-03-14 20:55:39 +01:00
+++ edited/include/asm-generic/vmlinux.lds.h	2005-03-22 23:55:17 +01:00
@@ -97,3 +97,12 @@
 		VMLINUX_SYMBOL(__lock_text_start) = .;			\
 		*(.spinlock.text)					\
 		VMLINUX_SYMBOL(__lock_text_end) = .;
+
+/* initramfs area. */
+# define INITRAMFS							\
+	. = ALIGN(4096);						\
+	.init.ramfs : {							\
+		VMLINUX_SYMBOL(__initramfs_start) = .;			\
+		*(.init.ramfs)						\
+		VMLINUX_SYMBOL(__initramfs_end) = .;			\
+	}

Re: Double free of initramfs

[Sam Ravnborg]

On Tue, Mar 22, 2005 at 11:57:25PM +0100, Sam Ravnborg wrote:
> Something like this:
> 
> ===== include/asm-generic/vmlinux.lds.h 1.18 vs edited =====
> --- 1.18/include/asm-generic/vmlinux.lds.h	2005-03-14 20:55:39 +01:00
> +++ edited/include/asm-generic/vmlinux.lds.h	2005-03-22 23:55:17 +01:00
> @@ -97,3 +97,12 @@
>  		VMLINUX_SYMBOL(__lock_text_start) = .;			\
>  		*(.spinlock.text)					\
>  		VMLINUX_SYMBOL(__lock_text_end) = .;
> +
> +/* initramfs area. */
> +# define INITRAMFS							\
> +	. = ALIGN(4096);						\

Based on Tony's very valid comment this should read:
> +	. = ALIGN(PAGE_SIZE);						\
(Assuming the 4096 was a page alingment).

	Sam

RE: Double free of initramfs

[Luck, Tony]

>+
>+/* initramfs area. */
>+# define INITRAMFS		\
>+	. = ALIGN(4096);		\
>+	.init.ramfs : {						

Why the 4096 alignment?  If you need it page aligned (and I can't
see why you would), then bear in mind that not all architectures
use a 4k pagesize.

-Tony