2013-09-15 09:04:16

by Prasad Pandit

[permalink] [raw]
Subject: [PATCH 1/2] Fix NULL pointer dereference while loading initramfs

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


Attachments:
0001-NULL-pointer-dereference-while-loading-initramfs.patch (1.79 kB)

2013-09-23 19:41:13

by Andrew Morton

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

On Sun, 15 Sep 2013 14:33:53 +0530 (IST) P J P <[email protected]> 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 <[email protected]>
>
> 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?

2013-09-24 01:38:12

by Rob Landley

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

On 09/23/2013 02:41:10 PM, Andrew Morton wrote:
> On Sun, 15 Sep 2013 14:33:53 +0530 (IST) P J P <[email protected]>
> 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-

2013-09-24 18:45:53

by Prasad Pandit

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

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


Attachments:
0001-Fix-NULL-pointer-dereference-while-loading-initramfs.patch (1.80 kB)

2013-09-24 19:02:10

by Prasad Pandit

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

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

2013-09-30 21:43:38

by Prasad Pandit

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

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

2013-10-05 20:42:33

by Prasad Pandit

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

Hello Andrew,

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

2013-10-05 21:27:49

by Andrew Morton

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

On Sun, 6 Oct 2013 02:12:15 +0530 (IST) P J P <[email protected]> wrote:

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

Not yet, but it's in the queue.

2013-10-06 08:09:34

by Prasad Pandit

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

Hi,

+-- On Sat, 5 Oct 2013, Andrew Morton wrote --+
| On Sun, 6 Oct 2013 02:12:15 +0530 (IST) P J P <[email protected]> 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