Broken initrd compression settings in 3.13

Broken initrd compression settings in 3.13

[Linus Torvalds]

So commit 1bf49dd4be0b ("./Makefile: export initial ramdisk
compression config option") seems to be totally broken.

And I'm not saying that because Jan fixed a make-3.80 incompatibility
in commit 7ac181568342 ("fix build with make 3.80")

I'm saying that because it sets and exports the INITRD_COMPRESS
environment variable completely incorrectly, as far as I can tell.

And it looks like nobody noticed, because apparently dracut didn't use
to care about that INITRD_COMPRESS environment variable at all. But as
of a F20 update yesterday, apparently dracut actually does care, and
as a result nothing actually works.

That setting is f*cking moronic. It sets INITRD_COMPRESS to 'lz4' if
RD_LZ4 is set. But RD_LZ4 is *always* set (unless you go into some
expert settings), so basically what that commit does is to always set
INITRD_COMPRESS to lz4.

Why is that wrong?

It's wrong because

 (a) most sane people don't even have lz4 _installed_, so dracut won't
actually succeed

 (b) there's no way to select the compression level (unlike the
INITRAMFS_COMPRESSION thing that actually has a choice)

 (c) even if you *do* have lz4, it doesn't actually work, because
while that causes the new F20 dracut to compress the initramfs with
lz4, the end result is completely broken, because the F20 "lsinitrd"
scripts don't understand the end result, so now the whole kernel
install fails.

Now, it could be argued that this is a F20 bug, and the fact that F20
has a dracut that can generate a lz4 initrd, but has other tools that
then cannot handle it is arguably indeed a buglet in F20. So the (c)
part is arguably a F20 problem.

HOWEVER.

(a) and (b) are very much kernel bugs.

I'm going to remove that export of INITRD_COMPRESS, since right now
that value is broken and useless. No way does it make sense to
mindlessly default to a compressor that most people don't have, and
that doesn't work with most tools.

             Linus

Re: Broken initrd compression settings in 3.13

[Andrew Morton]

On Fri, 20 Dec 2013 16:41:43 -0800 Linus Torvalds <torvalds@linux-foundation.org> wrote:

> So commit 1bf49dd4be0b ("./Makefile: export initial ramdisk
> compression config option") seems to be totally broken.
> 
> And I'm not saying that because Jan fixed a make-3.80 incompatibility
> in commit 7ac181568342 ("fix build with make 3.80")
> 
> I'm saying that because it sets and exports the INITRD_COMPRESS
> environment variable completely incorrectly, as far as I can tell.
> 
> And it looks like nobody noticed, because apparently dracut didn't use
> to care about that INITRD_COMPRESS environment variable at all. But as
> of a F20 update yesterday, apparently dracut actually does care, and
> as a result nothing actually works.
> 
> That setting is f*cking moronic. It sets INITRD_COMPRESS to 'lz4' if
> RD_LZ4 is set. But RD_LZ4 is *always* set (unless you go into some
> expert settings), so basically what that commit does is to always set
> INITRD_COMPRESS to lz4.
> 
> Why is that wrong?
> 
> It's wrong because
> 
>  (a) most sane people don't even have lz4 _installed_, so dracut won't
> actually succeed
> 
>  (b) there's no way to select the compression level (unlike the
> INITRAMFS_COMPRESSION thing that actually has a choice)
> 
>  (c) even if you *do* have lz4, it doesn't actually work, because
> while that causes the new F20 dracut to compress the initramfs with
> lz4, the end result is completely broken, because the F20 "lsinitrd"
> scripts don't understand the end result, so now the whole kernel
> install fails.
> 
> Now, it could be argued that this is a F20 bug, and the fact that F20
> has a dracut that can generate a lz4 initrd, but has other tools that
> then cannot handle it is arguably indeed a buglet in F20. So the (c)
> part is arguably a F20 problem.
> 
> HOWEVER.
> 
> (a) and (b) are very much kernel bugs.
> 
> I'm going to remove that export of INITRD_COMPRESS, since right now
> that value is broken and useless. No way does it make sense to
> mindlessly default to a compressor that most people don't have, and
> that doesn't work with most tools.

Jeff sent the below this morning.  Will that fix (a)?


From: Jeff Layton <jlayton@redhat.com>
Subject: Makefile: fix order of INITRD_COMPRESS-y assignments

Commit 7ac181568342ec ("fix build with make 3.80") changed how the
Makefile handles the INITRD_COMPRESS-y assignment in order to make it work
properly with older versions of make.  Unfortunately, it also changes the
behavior of how that assignment works when multiple CONFIG_RD_* options
are set, such that they are now preferred in the reverse order of the way
they were before.

Reverse the order of assignments to try and preserve the earlier
precedence in this situation.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: P J P <ppandit@redhat.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 Makefile |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff -puN Makefile~makefile-fix-order-of-initrd_compress-y-assignments Makefile
--- a/Makefile~makefile-fix-order-of-initrd_compress-y-assignments
+++ a/Makefile
@@ -733,11 +733,11 @@ export mod_strip_cmd
 # This shall be used by the dracut(8) tool while creating an initramfs image.
 #
 INITRD_COMPRESS-y                  := gzip
-INITRD_COMPRESS-$(CONFIG_RD_BZIP2) := bzip2
-INITRD_COMPRESS-$(CONFIG_RD_LZMA)  := lzma
-INITRD_COMPRESS-$(CONFIG_RD_XZ)    := xz
-INITRD_COMPRESS-$(CONFIG_RD_LZO)   := lzo
 INITRD_COMPRESS-$(CONFIG_RD_LZ4)   := lz4
+INITRD_COMPRESS-$(CONFIG_RD_LZO)   := lzo
+INITRD_COMPRESS-$(CONFIG_RD_XZ)    := xz
+INITRD_COMPRESS-$(CONFIG_RD_LZMA)  := lzma
+INITRD_COMPRESS-$(CONFIG_RD_BZIP2) := bzip2
 export INITRD_COMPRESS := $(INITRD_COMPRESS-y)
 
 ifdef CONFIG_MODULE_SIG_ALL
_

Re: Broken initrd compression settings in 3.13

[Linus Torvalds]

On Fri, Dec 20, 2013 at 5:39 PM, Andrew Morton
<akpm@linux-foundation.org> wrote:
> On Fri, 20 Dec 2013 16:41:43 -0800 Linus Torvalds <torvalds@linux-foundation.org> wrote:
>>
>>  (a) most sane people don't even have lz4 _installed_, so dracut won't
>> actually succeed
>>
>>  (b) there's no way to select the compression level (unlike the
>> INITRAMFS_COMPRESSION thing that actually has a choice)
>>
>>  (c) even if you *do* have lz4, it doesn't actually work, because
>> while that causes the new F20 dracut to compress the initramfs with
>> lz4, the end result is completely broken, because the F20 "lsinitrd"
>> scripts don't understand the end result, so now the whole kernel
>> install fails.
>>
>> (a) and (b) are very much kernel bugs.
>
> Jeff sent the below this morning.  Will that fix (a)?

Yes, it fixes (a), at least to some degree, in that at least
defaulting to bzip2 is a lot more sane than defaulting to lz4. I
suspect most everybody has bzip2 installed. And at least on my current
F20 install, it looks like lsinitrd understands to use zcat, bzcat or
xzcat on the resulting initrd image (and bzcat does that bzip2
decoding).

So I think Jeff's patch at least fixes the symptoms.

That said, I think it does nothing *but* fix the symptoms, and we're
actually still better off with the 3.12 behavior which was to never
set INITRD_COMPRESS at all. Because quite frankly, there's currently
no way for the kernel to know what the right compressor is. bz2 may
well work, but can you guarantee it? I certainly can't..

Now, if we asked the user, that would be a different thing. But right
now we very much don't ask the user, and we just pick one at random.

We're better off not picking a compression method at all, at which
point the distro "installkernel" will do whatever the distro does.

            Linus

Re: Broken initrd compression settings in 3.13

[Jeff Layton]

On Fri, 20 Dec 2013 18:15:29 -0800
Linus Torvalds <torvalds@linux-foundation.org> wrote:

> On Fri, Dec 20, 2013 at 5:39 PM, Andrew Morton
> <akpm@linux-foundation.org> wrote:
> > On Fri, 20 Dec 2013 16:41:43 -0800 Linus Torvalds <torvalds@linux-foundation.org> wrote:
> >>
> >>  (a) most sane people don't even have lz4 _installed_, so dracut won't
> >> actually succeed
> >>
> >>  (b) there's no way to select the compression level (unlike the
> >> INITRAMFS_COMPRESSION thing that actually has a choice)
> >>
> >>  (c) even if you *do* have lz4, it doesn't actually work, because
> >> while that causes the new F20 dracut to compress the initramfs with
> >> lz4, the end result is completely broken, because the F20 "lsinitrd"
> >> scripts don't understand the end result, so now the whole kernel
> >> install fails.
> >>
> >> (a) and (b) are very much kernel bugs.
> >
> > Jeff sent the below this morning.  Will that fix (a)?
> 
> Yes, it fixes (a), at least to some degree, in that at least
> defaulting to bzip2 is a lot more sane than defaulting to lz4. I
> suspect most everybody has bzip2 installed. And at least on my current
> F20 install, it looks like lsinitrd understands to use zcat, bzcat or
> xzcat on the resulting initrd image (and bzcat does that bzip2
> decoding).
> 
> So I think Jeff's patch at least fixes the symptoms.
> 
> That said, I think it does nothing *but* fix the symptoms, and we're
> actually still better off with the 3.12 behavior which was to never
> set INITRD_COMPRESS at all. Because quite frankly, there's currently
> no way for the kernel to know what the right compressor is. bz2 may
> well work, but can you guarantee it? I certainly can't..
> 
> Now, if we asked the user, that would be a different thing. But right
> now we very much don't ask the user, and we just pick one at random.
> 
> We're better off not picking a compression method at all, at which
> point the distro "installkernel" will do whatever the distro does.
> 
>             Linus

That works for me, you can drop my patch...

But, what's the point of setting INITRD_COMPRESS-y at all if you're not
going to export INITRD_COMPRESS? Would it be better to just remove that
entire block in the Makefile (i.e. just revert PJP's patch?)

-- 
Jeff Layton <jlayton@redhat.com>

Re: Broken initrd compression settings in 3.13

[Linus Torvalds]

On Fri, Dec 20, 2013 at 6:23 PM, Jeff Layton <jlayton@redhat.com> wrote:
>
> But, what's the point of setting INITRD_COMPRESS-y at all if you're not
> going to export INITRD_COMPRESS? Would it be better to just remove that
> entire block in the Makefile (i.e. just revert PJP's patch?)

I ended up committing the minimal change that I was testing, leaving
the core code in, just disabling it so that everything works for me on
F20.

I may happen to think that the kernel shouldn't try to pick the
format, but I'm willing to leave the door open for somebody arguing
for that approach.

               Linus

Re: Broken initrd compression settings in 3.13

[Jeff Layton]

On Fri, 20 Dec 2013 18:15:29 -0800
Linus Torvalds <torvalds@linux-foundation.org> wrote:

> On Fri, Dec 20, 2013 at 5:39 PM, Andrew Morton
> <akpm@linux-foundation.org> wrote:
> > On Fri, 20 Dec 2013 16:41:43 -0800 Linus Torvalds <torvalds@linux-foundation.org> wrote:
> >>
> >>  (a) most sane people don't even have lz4 _installed_, so dracut won't
> >> actually succeed
> >>
> >>  (b) there's no way to select the compression level (unlike the
> >> INITRAMFS_COMPRESSION thing that actually has a choice)
> >>
> >>  (c) even if you *do* have lz4, it doesn't actually work, because
> >> while that causes the new F20 dracut to compress the initramfs with
> >> lz4, the end result is completely broken, because the F20 "lsinitrd"
> >> scripts don't understand the end result, so now the whole kernel
> >> install fails.
> >>
> >> (a) and (b) are very much kernel bugs.
> >
> > Jeff sent the below this morning.  Will that fix (a)?
> 
> Yes, it fixes (a), at least to some degree, in that at least
> defaulting to bzip2 is a lot more sane than defaulting to lz4. I
> suspect most everybody has bzip2 installed. And at least on my current
> F20 install, it looks like lsinitrd understands to use zcat, bzcat or
> xzcat on the resulting initrd image (and bzcat does that bzip2
> decoding).
> 
> So I think Jeff's patch at least fixes the symptoms.
> 
> That said, I think it does nothing *but* fix the symptoms, and we're
> actually still better off with the 3.12 behavior which was to never
> set INITRD_COMPRESS at all. Because quite frankly, there's currently
> no way for the kernel to know what the right compressor is. bz2 may
> well work, but can you guarantee it? I certainly can't..
> 
> Now, if we asked the user, that would be a different thing. But right
> now we very much don't ask the user, and we just pick one at random.
> 
> We're better off not picking a compression method at all, at which
> point the distro "installkernel" will do whatever the distro does.
> 
>             Linus

Perhaps a better solution for this would be to instead export an
env var with a list of the compression algorithms that the kernel
supports. Then installkernel or dracut could use that info to make a
semi-intelligent decision based on that and what tools are installed.

...or maybe a separate env var for each one that it supports:

    $INITRD_COMPRESS_LZ4
    $INITRD_COMPRESS_BZIP2
    $INITRD_COMPRESS_GZIP

...etc.

-- 
Jeff Layton <jlayton@redhat.com>

Re: Broken initrd compression settings in 3.13

[Linus Torvalds]

On Fri, Dec 20, 2013 at 6:33 PM, Jeff Layton <jlayton@redhat.com> wrote:
>
> Perhaps a better solution for this would be to instead export an
> env var with a list of the compression algorithms that the kernel
> supports. Then installkernel or dracut could use that info to make a
> semi-intelligent decision based on that and what tools are installed.
>
> ...or maybe a separate env var for each one that it supports:
>
>     $INITRD_COMPRESS_LZ4
>     $INITRD_COMPRESS_BZIP2
>     $INITRD_COMPRESS_GZIP
>
> ...etc.

Agreed, either of those would work.

Of course, so does just "distro selects whatever compression method it
thinks is best, and when you compile a kernel for that distro, you
need to make sure that that kernel knows how to uncompress the
initrd".

Which quite frankly is the sanest approach of all. *Especially*
considering that right now we default to supporting all the initrd
compression methods.

And it has the advantage of not needing anything like this at all.

When you compile a kernel, you already need to compile in support for
the stuff the distro needs. This is no different, really.

So I think the whole "kernel tells the distro what compression method
to use" approach is broken and silly, and the wrong way around.

              Linus

Re: Broken initrd compression settings in 3.13

[P J P]

   Hi,

+-- On Fri, 20 Dec 2013, Linus Torvalds wrote --+
| So commit 1bf49dd4be0b ("./Makefile: export initial ramdisk
| compression config option") seems to be totally broken.
| 
| And I'm not saying that because Jan fixed a make-3.80 incompatibility
| in commit 7ac181568342 ("fix build with make 3.80")
| 
| I'm saying that because it sets and exports the INITRD_COMPRESS
| environment variable completely incorrectly, as far as I can tell.
| 
| And it looks like nobody noticed, because apparently dracut didn't use
| to care about that INITRD_COMPRESS environment variable at all. But as
| of a F20 update yesterday, apparently dracut actually does care, and
| as a result nothing actually works.

  This issue was indeed noticed earlier and is fixed in commit '9ba4bcb645' 
which defaults to '.gz' compression when all CONFIG_RD_* options are set.

  -> https://lkml.org/lkml/2013/10/30/535

The issue occurs due the order in which variables are examined in 
usr/Makefile.

| That setting is f*cking moronic. It sets INITRD_COMPRESS to 'lz4' if
| RD_LZ4 is set. But RD_LZ4 is *always* set (unless you go into some
| expert settings), so basically what that commit does is to always set
| INITRD_COMPRESS to lz4.

  No, it does not! It would set INITRD_COMPRESS=lz4 when *only* CONFIG_RD_LZ4 
option is set and none other. That would be moronic thing to do. But that is 
almost impossible to happen via a distro kernel config because even if you 
could build a kernel with only CONFIG_RD_LZ4, it would not boot because the 
de-compressor in 'lib/decompress_unlz4.c' is broken since they changed the LZ4 
file format

 -> http://fastcompression.blogspot.fr/2013/04/lz4-streaming-format-final.html

|  (b) there's no way to select the compression level (unlike the
| INITRAMFS_COMPRESSION thing that actually has a choice)

   Ummn...?

|  (c) even if you *do* have lz4, it doesn't actually work, because
| while that causes the new F20 dracut to compress the initramfs with
| lz4, the end result is completely broken, because the F20 "lsinitrd"
| scripts don't understand the end result, so now the whole kernel
| install fails.

   Strange! I'm running kernel-3.12.0 which seems to have commit 
'1bf49dd4be0b' which exports INITRD_COMPRESS, but not commit '9ba4bcb645' 
which fixes the usr/Makefile. For the test I enabled all CONFIG_RD_ options

===
...
CONFIG_RD_GZIP=y
CONFIG_RD_BZIP2=y
CONFIG_RD_LZMA=y
CONFIG_RD_XZ=y
CONFIG_RD_LZO=y
CONFIG_RD_LZ4=y

...
# make install
/usr/src/kernels/linux-3.12/arch/x86/boot/install.sh: INITRD_COMPRESS: bzip2
/sbin/dracut: compress: bzip2 -9 (bzip2)
# 
# file /boot/initramfs-3.12.0.img 
/boot/initramfs-3.12.0.img: bzip2 compressed data, block size = 900k
# 
===

So, the final initramfs-3.12.0 is still .bzip2 compressed, which lsinitrd(1) 
does understand. Yet, it won't boot because the lz4 de-compressor is broken. I 
tried to look into it briefly to create a fix, but then got occupied with 
other work and am yet to get back to it. :(

| I'm going to remove that export of INITRD_COMPRESS, since right now
| that value is broken and useless. No way does it make sense to
| mindlessly default to a compressor that most people don't have, and
| that doesn't work with most tools.

  Or include the patch '9ba4bcb645' which fixes this issue. I see it already 
merged actually.

--
Prasad J Pandit / Red Hat Security Response Team