linux-kernel - Re: initramfs: Always do fput() and load modules after rootfs populate

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sat, 6 May 2017 00:02:32 -0700
From:   Andrei Vagin <avagin@...tuozzo.com>
To:     Stafford Horne <shorne@...il.com>
CC:     LKML <linux-kernel@...r.kernel.org>,
        Lokesh Vutla <lokeshvutla@...com>,
        Al Viro <viro@...iv.linux.org.uk>,
        Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: initramfs: Always do fput() and load modules after rootfs
 populate

Hi,

I see the following error with this patch:

init/initramfs.c: In function 'populate_rootfs':
init/initramfs.c:644:2: error: label at end of compound statement
  done:
  ^
scripts/Makefile.build:294: recipe for target 'init/initramfs.o' failed
make[1]: *** [init/initramfs.o] Error 1

probably we need something like this:
diff --git a/init/initramfs.c b/init/initramfs.c
index 3a68715..9966c0f 100644
--- a/init/initramfs.c
+++ b/init/initramfs.c
@@ -641,7 +641,6 @@ static int __init populate_rootfs(void)
                        sys_close(fd);
                        free_initrd();
                }
-       done:
 #else
                printk(KERN_INFO "Unpacking initramfs...\n");
                err = unpack_to_rootfs((char *)initrd_start,
@@ -651,6 +650,7 @@ static int __init populate_rootfs(void)
                free_initrd();
 #endif
        }
+done:
        flush_delayed_fput();
        /*
         * Try loading default modules from initramfs.  This gives

Thanks,
Andrei

On Thu, May 04, 2017 at 09:47:00PM +0900, Stafford Horne wrote:
> In OpenRISC we do not have a bootloader passed initrd, but the built in
> initramfs does contain the /init and other binaries, including modules.
> The previous commit 08865514805d2 ("initramfs: finish fput() before
> accessing any binary from initramfs") made a change to only call fput()
> if the bootloader initrd was available, this caused intermittent crashes
> for OpenRISC.
> 
> This patch changes the fput() to happen unconditionally if any rootfs is
> loaded. Also, I added some comments to make it a bit more clear why we
> call unpack_to_rootfs() multiple times.
> 
> Fixes: 08865514805d2 ("initramfs: finish fput() before accessing any binary from initramfs")
> Cc: Lokesh Vutla <lokeshvutla@...com>
> Cc: Al Viro <viro@...iv.linux.org.uk>
> Signed-off-by: Stafford Horne <shorne@...il.com>
> Acked-by: Al Viro <viro@...iv.linux.org.uk>
> ---
>  init/initramfs.c | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/init/initramfs.c b/init/initramfs.c
> index 981f286..3a68715 100644
> --- a/init/initramfs.c
> +++ b/init/initramfs.c
> @@ -608,9 +608,11 @@ static void __init clean_rootfs(void)
>  
>  static int __init populate_rootfs(void)
>  {
> +	/* Load the built in initramfs */
>  	char *err = unpack_to_rootfs(__initramfs_start, __initramfs_size);
>  	if (err)
>  		panic("%s", err); /* Failed to decompress INTERNAL initramfs */
> +	/* If available load the bootloader supplied initrd */
>  	if (initrd_start) {
>  #ifdef CONFIG_BLK_DEV_RAM
>  		int fd;
> @@ -648,13 +650,14 @@ static int __init populate_rootfs(void)
>  			printk(KERN_EMERG "Initramfs unpacking failed: %s\n", err);
>  		free_initrd();
>  #endif
> -		flush_delayed_fput();
> -		/*
> -		 * Try loading default modules from initramfs.  This gives
> -		 * us a chance to load before device_initcalls.
> -		 */
> -		load_default_modules();
>  	}
> +	flush_delayed_fput();
> +	/*
> +	 * Try loading default modules from initramfs.  This gives
> +	 * us a chance to load before device_initcalls.
> +	 */
> +	load_default_modules();
> +
>  	return 0;
>  }
>  rootfs_initcall(populate_rootfs);