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

[P J P <ppandit@redhat.com>]

[-- 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