[PATCH 1/2] Fix NULL pointer dereference while loading initramfs

[PATCH 1/2] Fix NULL pointer dereference while loading initramfs

[P J P]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1091 bytes --]

   Hello,

While building the 3.11 kernel recently, I bumped into this issue.

Menuconfig allows one to choose compression format of an initial ramdisk 
image. If we select any compression other than the default gzip(1), it leads 
to a NULL pointer dereference while loading the initramfs image, at boot up.

Because - $ make install - does not pass on the selected compression format to 
dracut(8) tool, it generates the initramfs file compressed with the default 
gzip(1) format. But the kernel knows only to decompress the chosen format.

The attached patch herein, replaces the crash by an explicit panic() call with 
an appropriate error message. It keeps the kernel from eventually panicking in 
init/do_mounts.c: mount_root_block() with an inaccurate error message and 
which also looses the earlier printk output that hints about the missing 
gzip(1) decompression support.
   -> panic("VFS: Unable to mount root fs on %s", b);

Could someone please review this patch?

Thank you.
--
Prasad J Pandit / Red Hat Security Response Team
DB7A 84C5 D3F9 7CD1 B5EB  C939 D048 7860 3655 602B

[-- Attachment #2: Type: TEXT/PLAIN, Size: 1832 bytes --]

From 4ff1ddae358dff002080d753e45721a89d07b3af Mon Sep 17 00:00:00 2001
From: P J P <prasad@redhat.com>
Date: Sun, 15 Sep 2013 13:03:17 +0530
Subject: [PATCH 1/2] NULL pointer dereference while loading initramfs

Make menuconfig allows one to choose compression format of an
initial ramdisk image. But this choice does not result in duly
compressed initial ramdisk image. Because - $ make install - does
not pass on the selected compression choice to the dracut(8) tool,
which creates the initramfs file. dracut(8) generates the image
with the default compression, ie. gzip(1).

If a user chose any other compression instead of gzip(1), it leads
to a crash due to NULL pointer dereference in crd_load(), caused by
a NULL function pointer returned by the 'decompress_method()' routine.
Because the initramfs image is gzip(1) compressed, whereas the kernel
knows how decompress the chosen format and not gzip(1).

This patch replaces the crash by an explicit panic() call with an
appropriate error message. This shall prevent the kernel from
eventually panicking in: init/do_mounts.c: mount_block_root() with
  -> panic("VFS: Unable to mount root fs on %s", b);

Signed-off-by: P J P <prasad@redhat.com>

diff --git a/init/do_mounts_rd.c b/init/do_mounts_rd.c
index 6be2879..76faec1 100644
--- a/init/do_mounts_rd.c
+++ b/init/do_mounts_rd.c
@@ -342,6 +342,13 @@ static int __init crd_load(int in_fd, int out_fd, decompress_fn deco)
 	int result;
 	crd_infd = in_fd;
 	crd_outfd = out_fd;
+
+    if (!deco)
+    {
+        printk(KERN_EMERG "Invalid decompression routine address: %p\n", deco);
+        panic("Could not decompress initial ramdisk image.");
+    }
+
 	result = deco(NULL, 0, compr_fill, compr_flush, NULL, NULL, error);
 	if (decompress_error)
 		result = 1;
-- 
1.8.3.1

Re: [PATCH 1/2] Fix NULL pointer dereference while loading initramfs

[Andrew Morton]

On Sun, 15 Sep 2013 14:33:53 +0530 (IST) P J P <ppandit@redhat.com> wrote:

> Make menuconfig allows one to choose compression format of an
> initial ramdisk image. But this choice does not result in duly
> compressed initial ramdisk image. Because - $ make install - does
> not pass on the selected compression choice to the dracut(8) tool,
> which creates the initramfs file. dracut(8) generates the image
> with the default compression, ie. gzip(1).
> 
> If a user chose any other compression instead of gzip(1), it leads
> to a crash due to NULL pointer dereference in crd_load(), caused by
> a NULL function pointer returned by the 'decompress_method()' routine.
> Because the initramfs image is gzip(1) compressed, whereas the kernel
> knows how decompress the chosen format and not gzip(1).
> 
> This patch replaces the crash by an explicit panic() call with an
> appropriate error message. This shall prevent the kernel from
> eventually panicking in: init/do_mounts.c: mount_block_root() with
>   -> panic("VFS: Unable to mount root fs on %s", b);
> 
> Signed-off-by: P J P <prasad@redhat.com>
> 
> diff --git a/init/do_mounts_rd.c b/init/do_mounts_rd.c
> index 6be2879..76faec1 100644
> --- a/init/do_mounts_rd.c
> +++ b/init/do_mounts_rd.c
> @@ -342,6 +342,13 @@ static int __init crd_load(int in_fd, int out_fd, decompress_fn deco)
>  	int result;
>  	crd_infd = in_fd;
>  	crd_outfd = out_fd;
> +
> +    if (!deco)
> +    {
> +        printk(KERN_EMERG "Invalid decompression routine address: %p\n", deco);
> +        panic("Could not decompress initial ramdisk image.");
> +    }

A few things here.

- the coding style is very unconventional.  We'd do it like this:

static int __init crd_load(int in_fd, int out_fd, decompress_fn deco)
{
	int result;
	crd_infd = in_fd;
	crd_outfd = out_fd;

	if (!deco) {
		pr_emerg("Invalid decompression routine address: %p\n", deco);
		panic("Could not decompress initial ramdisk image.");
	}

	result = deco(NULL, 0, compr_fill, compr_flush, NULL, NULL, error);
	if (decompress_error)
		result = 1;
	return result;
}

- Note the use of the pr_emerg() shorthand, which prevents the
  statement from overflowing 80 columns.

- There isn't much point in printing the value of `deco' - we already
  know it's NULL.  Isn't there some more useful message we can display
  which will tell the user what he/she did wrong, and which explains
  how to fix it?

- Is anyone working on fixing up Kconfig/dracut(8) so the correct
  decompression method is used?

Re: [PATCH 1/2] Fix NULL pointer dereference while loading initramfs

[Rob Landley]

On 09/23/2013 02:41:10 PM, Andrew Morton wrote:
> On Sun, 15 Sep 2013 14:33:53 +0530 (IST) P J P <ppandit@redhat.com>  
> wrote:
> 
> > Make menuconfig allows one to choose compression format of an
> > initial ramdisk image. But this choice does not result in duly
> > compressed initial ramdisk image. Because - $ make install - does
> > not pass on the selected compression choice to the dracut(8) tool,
> > which creates the initramfs file. dracut(8) generates the image
> > with the default compression, ie. gzip(1).

I've been building kernels, including with initramfs, and I don't have  
dracut(8) installed? In today's git:

$ find . -type f | xargs grep -i dracut
./tools/testing/ktest/sample.conf:#POST_INSTALL = ssh user@target  
/sbin/dracut -f /boot/initramfs-test.img $KERNEL_VERSION
./tools/testing/ktest/examples/kvm.conf:# that uses dracut for its  
initfs. The POST_INSTALL will be executed
./tools/testing/ktest/examples/kvm.conf:POST_INSTALL = ${SSH}  
/sbin/dracut -f /boot/initramfs-test.img $KERNEL_VERSION
./Documentation/ABI/testing/evm:		 
Documentation/keys-trusted-encrypted.txt.  (A sample dracut

> - Is anyone working on fixing up Kconfig/dracut(8) so the correct
>   decompression method is used?

I've been messing with this area and maight be interested if I had any  
idea what this guy was talking about?

Rob

Re: [PATCH 1/2] Fix NULL pointer dereference while loading initramfs

[P J P]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1225 bytes --]

   Hello Andrew,

Thank you so much for reviewing these patches.

+-- On Mon, 23 Sep 2013, Andrew Morton wrote --+
| A few things here.
| - the coding style is very unconventional.  We'd do it like this:
| 	if (!deco) {
| 		pr_emerg("Invalid decompression routine address: %p\n", deco);
| 		panic("Could not decompress initial ramdisk image.");
| 	}
 
   Done; Please see an updated new patch attached herein.
 

| - There isn't much point in printing the value of `deco' - we already
|   know it's NULL.

   True, I just thought displaying 'deco' would help the person seeing the 
panic screen. Maybe while reporting an issue or trying to debug it.

|   Isn't there some more useful message we can display
|   which will tell the user what he/she did wrong, and which explains
|   how to fix it?

   Yes, the message from 'decompress_method()' says which compression method 
is required, but is not defined. I've also added a hint to select appropriate 
config option.
 
| - Is anyone working on fixing up Kconfig/dracut(8) so the correct
|   decompression method is used?

   Yes, I plan to do that.


Thank you so much!
--
Prasad J Pandit / Red Hat Security Response Team
DB7A 84C5 D3F9 7CD1 B5EB  C939 D048 7860 3655 602B

[-- Attachment #2: Type: TEXT/PLAIN, Size: 1840 bytes --]

From 842d328ba52cb53ec057b0703fbc8fbc2a2d6f1a Mon Sep 17 00:00:00 2001
From: P J P <prasad@redhat.com>
Date: Tue, 24 Sep 2013 23:59:21 +0530
Subject: [PATCH] Fix NULL pointer dereference while loading initramfs

Make menuconfig allows one to choose compression format of an
initial ramdisk image. But this choice does not result in duly
compressed initial ramdisk image. Because - $ make install - does
not pass on the selected compression choice to the dracut(8) tool,
which creates the initramfs file. dracut(8) generates the image
with the default compression, ie. gzip(1).

If a user chose any other compression instead of gzip(1), it leads
to a crash due to NULL pointer dereference in crd_load(), caused by
a NULL function pointer returned by the 'decompress_method()' routine.
Because the initramfs image is gzip(1) compressed, whereas the kernel
knows only to decompress the chosen format and not gzip(1).

This patch replaces the crash by an explicit panic() call with an
appropriate error message. This shall prevent the kernel from
eventually panicking in: init/do_mounts.c: mount_block_root() with
  -> panic("VFS: Unable to mount root fs on %s", b);

Signed-off-by: P J P <prasad@redhat.com>

diff --git a/init/do_mounts_rd.c b/init/do_mounts_rd.c
index 6be2879..bdaa345 100644
--- a/init/do_mounts_rd.c
+++ b/init/do_mounts_rd.c
@@ -342,6 +342,13 @@ static int __init crd_load(int in_fd, int out_fd, decompress_fn deco)
 	int result;
 	crd_infd = in_fd;
 	crd_outfd = out_fd;
+
+	if (!deco) {
+		pr_emerg("Invalid decompression routine: %p; "
+				 "Select appropriate config option.\n", deco);
+		panic("Could not decompress initial ramdisk image.");
+	}
+
 	result = deco(NULL, 0, compr_fill, compr_flush, NULL, NULL, error);
 	if (decompress_error)
 		result = 1;
-- 
1.8.3.1

Re: [PATCH 1/2] Fix NULL pointer dereference while loading initramfs

[P J P]

  Hello Rob,

+-- On Mon, 23 Sep 2013, Rob Landley wrote --+
| I've been building kernels, including with initramfs, and I don't have
| dracut(8) installed? In today's git:

  dracut(8) is a separate tool installed from package dracut.

 -> dracut-029-2.fc19.x86_64
 -> https://dracut.wiki.kernel.org/

 
| I've been messing with this area and maight be interested if I had any idea 
| what this guy was talking about?

  Kernel menuconfig selects gzip(1) as a default compression format for the 
initial ramdisk image.

  $ make menuconfig
   -> General setup
    -> Support initial ramdisk compressed using ...

If instead of gzip(1), you select bzip2(1) or xz(1) format and build a new 
kernel, the initramfs image would still be compressed with gzip(1), because 
currently there is no way to pass the selected compression format to dracut(8) 
via $ make install.

# file /boot/initramfs-3.11.0.img 
/boot/initramfs-3.11.0.img: LZMA compressed data, streamed

It shows LZMA because I've patched /sbin/new-kernel-pkg to use 
$INITRD_COMPRESS variable.

Thank you.
--
Prasad J Pandit / Red Hat Security Response Team
DB7A 84C5 D3F9 7CD1 B5EB  C939 D048 7860 3655 602B

Re: [PATCH 1/2] Fix NULL pointer dereference while loading initramfs

[P J P]

  Hello Andrew,

I was wondering if you had a chance to review the updated patch that I sent. 
You aren't waiting on me for something, are you?

Thank you.
--
Prasad J Pandit / Red Hat Security Response Team
DB7A 84C5 D3F9 7CD1 B5EB  C939 D048 7860 3655 602B

Re: [PATCH 1/2] Fix NULL pointer dereference while loading initramfs

[P J P]

  Hello Andrew,

Just to check, did you have chance to review an updated patch?
--
Prasad J Pandit / Red Hat Security Response Team

Re: [PATCH 1/2] Fix NULL pointer dereference while loading initramfs

[Andrew Morton]

On Sun, 6 Oct 2013 02:12:15 +0530 (IST) P J P <ppandit@redhat.com> wrote:

>   Hello Andrew,
> 
> Just to check, did you have chance to review an updated patch?

Not yet, but it's in the queue.

Re: [PATCH 1/2] Fix NULL pointer dereference while loading initramfs

[P J P]

  Hi,

+-- On Sat, 5 Oct 2013, Andrew Morton wrote --+
| On Sun, 6 Oct 2013 02:12:15 +0530 (IST) P J P <ppandit@redhat.com> wrote:
| Not yet, but it's in the queue.

  I see; Thank you for an update. I appreciate it.

Please let me know if there are changes to be done. Or if you think I could 
send similar patches for other architectures too. I thought once these are 
reviewed for one architecture, replicating the same changes for others would be 
safe & easy.


Thank you so much!
--
Prasad J Pandit / Red Hat Security Response Team