| 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
| ||
|
Date: Thu, 7 Sep 2017 21:36:25 -0700
From: Florian Fainelli <f.fainelli@...il.com>
To: Nicholas Piggin <npiggin@...il.com>
Cc: "Francisco Blas Izquierdo Riera (klondike)" <klondike@...ndike.es>,
linux-kernel@...r.kernel.org, ppandit@...hat.com,
pebolle@...cali.nl, mmarek@...e.cz,
Andrew Morton <akpm@...ux-foundation.org>,
Daniel Thompson <daniel.thompson@...aro.org>
Subject: Re: [PATCH] initramfs: Fix disabling of initramfs (and its
compression)
On 09/07/2017 06:19 AM, Nicholas Piggin wrote:
> On Thu, 7 Sep 2017 05:50:30 -0700
> Florian Fainelli <f.fainelli@...il.com> wrote:
>
>> On 08/28/2017 08:09 PM, Nicholas Piggin wrote:
>>> On Mon, 28 Aug 2017 13:03:31 -0700
>>> Florian Fainelli <f.fainelli@...il.com> wrote:
>>>
>>>> On 05/21/2017 07:46 PM, Nicholas Piggin wrote:
>>>>> On Sat, 20 May 2017 20:33:35 -0700
>>>>> Florian Fainelli <f.fainelli@...il.com> wrote:
>>>>>
>>>>>> Commit db2aa7fd15e8 ("initramfs: allow again choice of the embedded
>>>>>> initram compression algorithm") introduced the possibility to select the
>>>>>> initramfs compression algorithm from Kconfig and while this is a nice
>>>>>> feature it broke the use case described below.
>>>>>>
>>>>>> Here is what my build system does:
>>>>>>
>>>>>> - kernel is initially configured not to have an initramfs included
>>>>>> - build the user space root file system
>>>>>> - re-configure the kernel to have an initramfs included
>>>>>> (CONFIG_INITRAMFS_SOURCE="/path/to/romfs") and set relevant
>>>>>> CONFIG_INITRAMFS options, in my case, no compression option
>>>>>> (CONFIG_INITRAMFS_COMPRESSION_NONE)
>>>>>> - kernel is re-built with these options -> kernel+initramfs image is
>>>>>> copied
>>>>>> - kernel is re-built again without these options -> kernel image is
>>>>>> copied
>>>>>>
>>>>>> Building a kernel without an initramfs means setting this option:
>>>>>>
>>>>>> CONFIG_INITRAMFS_SOURCE="" (and this one only)
>>>>>>
>>>>>> whereas building a kernel with an initramfs means setting these options:
>>>>>>
>>>>>> CONFIG_INITRAMFS_SOURCE="/home/fainelli/work/uclinux-rootfs/romfs
>>>>>> /home/fainelli/work/uclinux-rootfs/misc/initramfs.dev"
>>>>>> CONFIG_INITRAMFS_ROOT_UID=1000
>>>>>> CONFIG_INITRAMFS_ROOT_GID=1000
>>>>>> CONFIG_INITRAMFS_COMPRESSION_NONE=y
>>>>>> CONFIG_INITRAMFS_COMPRESSION=""
>>>>>>
>>>>>> Commit db2aa7fd15e857891cefbada8348c8d938c7a2bc ("initramfs: allow again
>>>>>> choice of the embedded initram compression algorithm") is problematic
>>>>>> because CONFIG_INITRAMFS_COMPRESSION which is used to determine the
>>>>>> initramfs_data.cpio extension/compression is a string, and due to how
>>>>>> Kconfig works it will evaluate in order, how to assign it.
>>>>>>
>>>>>> Setting CONFIG_INITRAMFS_COMPRESSION_NONE with
>>>>>> CONFIG_INITRAMFS_SOURCE="" cannot possibly work (because of the depends
>>>>>> on INITRAMFS_SOURCE!="" imposed on CONFIG_INITRAMFS_COMPRESSION ) yet we
>>>>>> still get CONFIG_INITRAMFS_COMPRESSION assigned to ".gz" because
>>>>>> CONFIG_RD_GZIP=y is set in my kernel, even when there is no initramfs
>>>>>> being built.
>>>>>>
>>>>>> So we basically end-up generating two initramfs_data.cpio* files, one
>>>>>> without extension, and one with .gz. This causes usr/Makefile to track
>>>>>> usr/initramfs_data.cpio.gz, and not usr/initramfs_data.cpio anymore,
>>>>>> that is also largely problematic after
>>>>>> 9e3596b0c6539e28546ff7c72a06576627068353 ("kbuild: initramfs cleanup,
>>>>>> set target from Kconfig") because we used to track all possible
>>>>>> initramfs_data files in the $(targets) variable before that commit.
>>>>>>
>>>>>> The end result is that the kernel with an initramfs clearly does not
>>>>>> contain what we expect it to, it has a stale initramfs_data.cpio file
>>>>>> built into it, and we keep re-generating an initramfs_data.cpio.gz file
>>>>>> which is not the one that we want to include in the kernel image proper.
>>>>>>
>>>>>> The fix consists in hiding CONFIG_INITRAMFS_COMPRESSION when
>>>>>> CONFIG_INITRAMFS_SOURCE="". This puts us back in a state to the pre-4.10
>>>>>> behavior where we can properly disable and re-enable initramfs within
>>>>>> the same kernel .config file, and be in control of what
>>>>>> CONFIG_INITRAMFS_COMPRESSION is set to.
>>>>>>
>>>>>> Fixes: db2aa7fd15e8 ("initramfs: allow again choice of the embedded initram compression algorithm")
>>>>>> Fixes: 9e3596b0c653 ("kbuild: initramfs cleanup, set target from Kconfig")
>>>>>> Signed-off-by: Florian Fainelli <f.fainelli@...il.com>
>>>>>
>>>>> This is very thorough, thank you for tracking it down and fixing it.
>>>>>
>>>>> I can't say I've worked through the problem in the code, but your
>>>>> changelog and the proposed fix seem reasonable to me. So for what
>>>>> it's worth:
>>>>>
>>>>> Acked-by: Nicholas Piggin <npiggin@...il.com>
>>>>
>>>> Well, I am looking at this again, and it's still broken, the same test
>>>> case is involved, except this time, I am switching beween no-initramfs
>>>> and initramfs with gzip compression (the key thing is using a
>>>> compression of some sort). The end result is the following:
>>>>
>>>> - change stuff in the rootfs
>>>> - build the kernel with initramfs, CONFIG_INITRAMFS_COMPRESSION_GZIP=y,
>>>> usr/initramfs_data.cpio.gz gets generated correctly the first time
>>>> - build the kernel without initramfs,
>>>> CONFIG_INITRAMFS_COMPRESSION_NONE=y, usr/initramfs_data.cpio gets generated
>>>>
>>>> Now back to step 1 add some files, and we can see that
>>>> usr/initramfs_data.cpio.gz is now stale from before...
>>>>
>>>> So while my earlier fix switched the initramfs w/o compression to no
>>>> initramfs rebuild, now this does not work because we still have two
>>>> files left to be tracked:
>>>>
>>>> usr/initramfs_data.cpio (no compression, or when initramfs is disabled)
>>>> and usr/initramfs_data.cpio.$(suffix_y)
>>>>
>>>> How would you go about solving this?
>>>
>>> I don't see the problem. When I change back to .gz, modify the source
>>> directory, and rebuild, it rebuilds a new initramfs_data.cpio.gz here.
>>
>> This is really puzzling me, so here is what is going on my side:
>>
>> First time build w/ initramfs enabled:
>>
>> grep INITRAMFS .config
>> CONFIG_INITRAMFS_SOURCE="/home/fainelli/work/uclinux-rootfs/romfs
>> /home/fainelli/work/uclinux-rootfs/misc/initramfs.dev"
>> CONFIG_INITRAMFS_ROOT_UID=1000
>> CONFIG_INITRAMFS_ROOT_GID=1000
>> # CONFIG_INITRAMFS_COMPRESSION_NONE is not set
>> CONFIG_INITRAMFS_COMPRESSION_GZIP=y
>> # CONFIG_INITRAMFS_COMPRESSION_BZIP2 is not set
>> # CONFIG_INITRAMFS_COMPRESSION_LZMA is not set
>> # CONFIG_INITRAMFS_COMPRESSION_XZ is not set
>> # CONFIG_INITRAMFS_COMPRESSION_LZO is not set
>> # CONFIG_INITRAMFS_COMPRESSION_LZ4 is not set
>> CONFIG_INITRAMFS_COMPRESSION=".gz"
>>
>> fainelli@...nelli-laptop:[~/../linux]$ ls -la usr
>> total 6.8M
>> drwxrwxr-x 2 fainelli fainelli 4.0K Sep 7 05:37 ./
>> drwxrwxr-x 27 fainelli fainelli 16K Sep 7 05:38 ../
>> -rw-rw-r-- 1 fainelli fainelli 146 Sep 7 05:37 built-in.o
>> -rw-rw-r-- 1 fainelli fainelli 112 Sep 7 05:37 .built-in.o.cmd
>> -rwxrwxr-x 1 fainelli fainelli 23K Sep 7 05:36 gen_init_cpio*
>> -rw-rw-r-- 1 fainelli fainelli 13K Oct 13 2016 gen_init_cpio.c
>> -rw-rw-r-- 1 fainelli fainelli 3.3K Sep 7 05:36 .gen_init_cpio.cmd
>> -rw-rw-r-- 1 fainelli fainelli 151 Aug 28 2016 .gitignore
>> -rw-rw-r-- 1 fainelli fainelli 9.6K Sep 7 05:36 .initramfs_data.cpio.d
>> -rw-rw-r-- 1 fainelli fainelli 3.3M Sep 7 05:37 initramfs_data.cpio.gz
>> -rw-rw-r-- 1 fainelli fainelli 220 Sep 7 05:37
>> .initramfs_data.cpio.gz.cmd
>> -rw-rw-r-- 1 fainelli fainelli 3.3M Sep 7 05:37 initramfs_data.o
>> -rw-rw-r-- 1 fainelli fainelli 2.7K Sep 7 05:37 .initramfs_data.o.cmd
>> -rw-rw-r-- 1 fainelli fainelli 1.3K Aug 28 2016 initramfs_data.S
>> -rw-rw-r-- 1 fainelli fainelli 7.8K Aug 18 19:25 Kconfig
>> -rw-rw-r-- 1 fainelli fainelli 2.0K Aug 31 20:46 Makefile
>> -rw-rw-r-- 1 fainelli fainelli 0 Sep 7 05:34 modules.builtin
>> -rw-rw-r-- 1 fainelli fainelli 0 Sep 7 05:35 modules.order
>>
>> Now let's reconfigure this kernel w/o initramfs and rebuild:
>>
>> grep INITRAMFS .config
>> CONFIG_INITRAMFS_SOURCE=""
>>
>> fainelli@...nelli-laptop:[~/../linux]$ ls -la usr
>> total 3.6M
>> drwxrwxr-x 2 fainelli fainelli 4.0K Sep 7 05:44 ./
>> drwxrwxr-x 27 fainelli fainelli 16K Sep 7 05:44 ../
>> -rw-rw-r-- 1 fainelli fainelli 146 Sep 7 05:44 built-in.o
>> -rw-rw-r-- 1 fainelli fainelli 112 Sep 7 05:44 .built-in.o.cmd
>> -rwxrwxr-x 1 fainelli fainelli 23K Sep 7 05:36 gen_init_cpio*
>> -rw-rw-r-- 1 fainelli fainelli 13K Oct 13 2016 gen_init_cpio.c
>> -rw-rw-r-- 1 fainelli fainelli 3.3K Sep 7 05:36 .gen_init_cpio.cmd
>> -rw-rw-r-- 1 fainelli fainelli 151 Aug 28 2016 .gitignore
>> -rw-rw-r-- 1 fainelli fainelli 512 Sep 7 05:44 initramfs_data.cpio
>> -rw-rw-r-- 1 fainelli fainelli 105 Sep 7 05:44 .initramfs_data.cpio.cmd
>> -rw-rw-r-- 1 fainelli fainelli 52 Sep 7 05:44 .initramfs_data.cpio.d
>> -rw-rw-r-- 1 fainelli fainelli 3.3M Sep 7 05:37 initramfs_data.cpio.gz
>> -rw-rw-r-- 1 fainelli fainelli 220 Sep 7 05:37
>> .initramfs_data.cpio.gz.cmd
>> -rw-rw-r-- 1 fainelli fainelli 1.3K Sep 7 05:44 initramfs_data.o
>> -rw-rw-r-- 1 fainelli fainelli 2.7K Sep 7 05:44 .initramfs_data.o.cmd
>> -rw-rw-r-- 1 fainelli fainelli 1.3K Aug 28 2016 initramfs_data.S
>> -rw-rw-r-- 1 fainelli fainelli 7.8K Aug 18 19:25 Kconfig
>> -rw-rw-r-- 1 fainelli fainelli 2.0K Aug 31 20:46 Makefile
>> -rw-rw-r-- 1 fainelli fainelli 0 Sep 7 05:34 modules.builtin
>> -rw-rw-r-- 1 fainelli fainelli 0 Sep 7 05:35 modules.order
>>
>> Now let's rebuild with a new application added to the root filesystem:
>>
>> grep INITRAMFS .config
>> CONFIG_INITRAMFS_SOURCE="/home/fainelli/work/uclinux-rootfs/romfs
>> /home/fainelli/work/uclinux-rootfs/misc/initramfs.dev"
>> CONFIG_INITRAMFS_ROOT_UID=1000
>> CONFIG_INITRAMFS_ROOT_GID=1000
>> # CONFIG_INITRAMFS_COMPRESSION_NONE is not set
>> CONFIG_INITRAMFS_COMPRESSION_GZIP=y
>> # CONFIG_INITRAMFS_COMPRESSION_BZIP2 is not set
>> # CONFIG_INITRAMFS_COMPRESSION_LZMA is not set
>> # CONFIG_INITRAMFS_COMPRESSION_XZ is not set
>> # CONFIG_INITRAMFS_COMPRESSION_LZO is not set
>> # CONFIG_INITRAMFS_COMPRESSION_LZ4 is not set
>> CONFIG_INITRAMFS_COMPRESSION=".gz"
>>
>> We are not fine here:
>> scripts/kconfig/conf --silentoldconfig Kconfig
>> #
>> # configuration written to .config
>> #
>> CHK include/config/kernel.release
>> CHK include/generated/uapi/linux/version.h
>> CHK scripts/mod/devicetable-offsets.h
>> CHK include/generated/utsrelease.h
>> CHK include/generated/timeconst.h
>> CHK include/generated/bounds.h
>> CHK include/generated/asm-offsets.h
>> CALL scripts/checksyscalls.sh
>> CHK include/generated/compile.h
>> AS usr/initramfs_data.o
>>
>> ^===== we should have called gzip to re-generate the initramfs, we did not
>>
>> AR usr/built-in.o
>> GEN .version
>> CHK include/generated/compile.h
>> UPD include/generated/compile.h
>> CC init/version.o
>> AR init/built-in.o
>> AR built-in.o
>> LD vmlinux.o
>> MODPOST vmlinux.o
>>
>> And the timestamps here confirm that:
>>
>> fainelli@...nelli-laptop:[~/../linux]$ ls -la usr
>> total 6.9M
>> drwxrwxr-x 2 fainelli fainelli 4.0K Sep 7 05:46 ./
>> drwxrwxr-x 27 fainelli fainelli 16K Sep 7 05:46 ../
>> -rw-rw-r-- 1 fainelli fainelli 146 Sep 7 05:46 built-in.o
>> -rw-rw-r-- 1 fainelli fainelli 112 Sep 7 05:46 .built-in.o.cmd
>> -rwxrwxr-x 1 fainelli fainelli 23K Sep 7 05:36 gen_init_cpio*
>> -rw-rw-r-- 1 fainelli fainelli 13K Oct 13 2016 gen_init_cpio.c
>> -rw-rw-r-- 1 fainelli fainelli 3.3K Sep 7 05:36 .gen_init_cpio.cmd
>> -rw-rw-r-- 1 fainelli fainelli 151 Aug 28 2016 .gitignore
>> -rw-rw-r-- 1 fainelli fainelli 512 Sep 7 05:44 initramfs_data.cpio
>> -rw-rw-r-- 1 fainelli fainelli 105 Sep 7 05:44 .initramfs_data.cpio.cmd
>> -rw-rw-r-- 1 fainelli fainelli 11K Sep 7 05:46 .initramfs_data.cpio.d
>> -rw-rw-r-- 1 fainelli fainelli 3.3M Sep 7 05:37 initramfs_data.cpio.gz
>> -rw-rw-r-- 1 fainelli fainelli 220 Sep 7 05:37
>> .initramfs_data.cpio.gz.cmd
>> -rw-rw-r-- 1 fainelli fainelli 3.3M Sep 7 05:46 initramfs_data.o
>> -rw-rw-r-- 1 fainelli fainelli 2.7K Sep 7 05:46 .initramfs_data.o.cmd
>> -rw-rw-r-- 1 fainelli fainelli 1.3K Aug 28 2016 initramfs_data.S
>> -rw-rw-r-- 1 fainelli fainelli 7.8K Aug 18 19:25 Kconfig
>> -rw-rw-r-- 1 fainelli fainelli 2.0K Aug 31 20:46 Makefile
>> -rw-rw-r-- 1 fainelli fainelli 0 Sep 7 05:46 modules.builtin
>> -rw-rw-r-- 1 fainelli fainelli 0 Sep 7 05:46 modules.order
>>
>> Worse yet, usr/.initramfs_data.cpio.d does list the binary that was just
>> added, but for some reason, this did not trigger a rebuild of
>> initramfs_data.cpio.gz do you see any reasons why?
>
> This rule
>
> $(obj)/$(datafile_y): $(obj)/gen_init_cpio $(deps_initramfs) klibcdirs
> $(Q)$(initramfs) -l $(ramfs-input) > $(obj)/.initramfs_data.cpio.d
> $(call if_changed,initfs)
>
> does not have FORCE on the end, so I guess is not picking up the dependency
> properly, perhaps?
I tried that:
diff --git a/usr/Makefile b/usr/Makefile
index 0b87e71c00fc..28a958bebd27 100644
--- a/usr/Makefile
+++ b/usr/Makefile
@@ -51,6 +51,6 @@ $(deps_initramfs): klibcdirs
# 2) There are changes in which files are included (added or deleted)
# 3) If gen_init_cpio are newer than initramfs_data.cpio
# 4) arguments to gen_initramfs.sh changes
-$(obj)/$(datafile_y): $(obj)/gen_init_cpio $(deps_initramfs) klibcdirs
+$(obj)/$(datafile_y): $(obj)/gen_init_cpio $(deps_initramfs) klibcdirs
FORCE
$(Q)$(initramfs) -l $(ramfs-input) > $(obj)/.initramfs_data.cpio.d
$(call if_changed,initf
but this did not change anything.
We are re-generating .initramfs_data.cpio.d and its timestamp is clearly
updated so it's got to be cmd_initfs which does not get invoked.
.initramfs_data.cpio.gz.cmd is correct:
cmd_usr/initramfs_data.cpio.gz := /bin/bash
./scripts/gen_initramfs_list.sh -o usr/initramfs_data.cpio.gz -u 1000
-g 1000 /home/fainelli/work/uclinux-rootfs/romfs
/home/fainelli/work/uclinux-rootfs/misc/initramfs.dev
and was generated the first time we did generate the gzip initramfs, so
the command has not changed, nor its arguments, so we just don't call it.
I did try to use if_changed_dep, which rightfully pointed that we don't
have a .initramfs_data.cpio.gz.d, but this gets us very close from the
only thing that I could get working thus far. Another way to possibly
solve it is to void generating a cmd_$($@) and use the same name
irrespective of the compression type used, that should force a change
everytime...
Thanks Nick!
diff --git a/usr/Makefile b/usr/Makefile
index 0b87e71c00fc..2d7d7f4b91f1 100644
--- a/usr/Makefile
+++ b/usr/Makefile
@@ -7,6 +7,7 @@ PHONY += klibcdirs
suffix_y = $(subst $\",,$(CONFIG_INITRAMFS_COMPRESSION))
datafile_y = initramfs_data.cpio$(suffix_y)
+datafile_d_y = .$(datafile_y).d
AFLAGS_initramfs_data.o += -DINITRAMFS_IMAGE="usr/$(datafile_y)"
@@ -29,12 +30,12 @@ ramfs-args := \
$(if $(CONFIG_INITRAMFS_ROOT_UID), -u
$(CONFIG_INITRAMFS_ROOT_UID)) \
$(if $(CONFIG_INITRAMFS_ROOT_GID), -g $(CONFIG_INITRAMFS_ROOT_GID))
-# .initramfs_data.cpio.d is used to identify all files included
+# $(datafile_d_y) is used to identify all files included
# in initramfs and to detect if any files are added/removed.
# Removed files are identified by directory timestamp being updated
# The dependency list is generated by gen_initramfs.sh -l
-ifneq ($(wildcard $(obj)/.initramfs_data.cpio.d),)
- include $(obj)/.initramfs_data.cpio.d
+ifneq ($(wildcard $(obj)/$(datafile_d_y)),)
+ include $(obj)/$(datafile_d_y)
endif
quiet_cmd_initfs = GEN $@
@@ -52,5 +53,5 @@ $(deps_initramfs): klibcdirs
# 3) If gen_init_cpio are newer than initramfs_data.cpio
# 4) arguments to gen_initramfs.sh changes
$(obj)/$(datafile_y): $(obj)/gen_init_cpio $(deps_initramfs) klibcdirs
- $(Q)$(initramfs) -l $(ramfs-input) > $(obj)/.initramfs_data.cpio.d
+ $(Q)$(initramfs) -l $(ramfs-input) > $(obj)/$(datafile_d_y)
$(call if_changed,initfs)
--
Florian
Powered by blists - more mailing lists