linux-kernel - Re: [PATCH 2/2] initramfs: Encode dependency on KBUILD_BUILD

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAK7LNASxnyFmM1JhuDatQfb4WMy616LzAqS3JzmNxb3wJGA52g@mail.gmail.com>
Date:   Sun, 16 Apr 2023 13:06:36 +0900
From:   Masahiro Yamada <masahiroy@...nel.org>
To:     Benjamin Gray <bgray@...ux.ibm.com>
Cc:     linux-kernel@...r.kernel.org, linux-kbuild@...r.kernel.org
Subject: Re: [PATCH 2/2] initramfs: Encode dependency on KBUILD_BUILD_TIMESTAMP

On Mon, Mar 20, 2023 at 1:09 PM Benjamin Gray <bgray@...ux.ibm.com> wrote:
>
> gen_initramfs.sh has an internal dependency on KBUILD_BUILD_TIMESTAMP
> for generating file mtimes that is not exposed to make, so changing
> KBUILD_BUILD_TIMESTAMP will not trigger a rebuild of the archive.
>
> Declare the mtime date as a new parameter to gen_initramfs.sh to encode
> KBUILD_BUILD_TIMESTAMP in the shell command, thereby making make aware
> of the dependency.
>
> It will rebuild if KBUILD_BUILD_TIMESTAMP changes or is newly set/unset.
> It will _not_ rebuild if KBUILD_BUILD_TIMESTAMP is unset before and
> after. This should be fine for anyone who doesn't care about setting
> specific build times in the first place.
>
> Signed-off-by: Benjamin Gray <bgray@...ux.ibm.com>
>
> ---
>
> Ran into this while debugging the issue in the first patch. Made for a
> very perplexing debug session before we worked out the state wasn't
> being rebuilt.
> ---
>  usr/Makefile         |  2 ++
>  usr/gen_initramfs.sh | 16 +++++++++-------
>  2 files changed, 11 insertions(+), 7 deletions(-)
>
> diff --git a/usr/Makefile b/usr/Makefile
> index 59d9e8b07a01..2aa386cf48d6 100644
> --- a/usr/Makefile
> +++ b/usr/Makefile
> @@ -64,6 +64,7 @@ quiet_cmd_initfs = GEN     $@
>         $(CONFIG_SHELL) $< -o $@ -l $(obj)/.initramfs_data.cpio.d \
>         $(if $(CONFIG_INITRAMFS_ROOT_UID), -u $(CONFIG_INITRAMFS_ROOT_UID)) \
>         $(if $(CONFIG_INITRAMFS_ROOT_GID), -g $(CONFIG_INITRAMFS_ROOT_GID)) \
> +       $(if $(KBUILD_BUILD_TIMESTAMP), -d $(KBUILD_BUILD_TIMESTAMP)) \
>         $(ramfs-input)



If I apply this patch, I see a build error.


$ make -j16 KBUILD_BUILD_TIMESTAMP="$(date)"
[snip]
  GEN     usr/initramfs_data.cpio
  usr/gen_initramfs.sh: Cannot open 'Apr'
make[2]: *** [usr/Makefile:76: usr/initramfs_data.cpio] Error 1
make[1]: *** [scripts/Makefile.build:494: usr] Error 2
make[1]: *** Waiting for unfinished jobs....




Because KBUILD_BUILD_TIMESTAMP may contain spaces
depending on the format, init/Makefile and
kernel/gen_kheaders.sh surround the date with
double-quotes.


Double-quoting it in the same way fixes the issue.

$(if $(KBUILD_BUILD_TIMESTAMP), -d "$(KBUILD_BUILD_TIMESTAMP)") \







>  # We rebuild initramfs_data.cpio if:
> @@ -71,6 +72,7 @@ quiet_cmd_initfs = GEN     $@
>  # 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
> +# 5) KBUILD_BUILD_TIMESTAMP is changed


This is unneeded because the change of KBUILD_BUILD_TIMESTAMP
is contained in

  4) Arguments to gen_initramfs.sh changes







>  $(obj)/initramfs_data.cpio: $(src)/gen_initramfs.sh $(obj)/gen_init_cpio $(deps_initramfs) FORCE
>         $(call if_changed,initfs)
>
> diff --git a/usr/gen_initramfs.sh b/usr/gen_initramfs.sh
> index 63476bb70b41..14b5782f961a 100755
> --- a/usr/gen_initramfs.sh
> +++ b/usr/gen_initramfs.sh
> @@ -23,6 +23,7 @@ $0 [-o <file>] [-l <dep_list>] [-u <uid>] [-g <gid>] {-d | <cpio_source>} ...
>         -g <gid>       Group ID to map to group ID 0 (root).
>                        <gid> is only meaningful if <cpio_source> is a
>                        directory.  "squash" forces all files to gid 0.
> +       -d <date>      Use date for all file mtime values
>         <cpio_source>  File list or directory for cpio archive.
>                        If <cpio_source> is a .cpio file it will be used
>                        as direct input to initramfs.
> @@ -190,6 +191,7 @@ prog=$0
>  root_uid=0
>  root_gid=0
>  dep_list=
> +timestamp=
>  cpio_list=$(mktemp ${TMPDIR:-/tmp}/cpiolist.XXXXXX)
>  output="/dev/stdout"
>
> @@ -218,6 +220,13 @@ while [ $# -gt 0 ]; do
>                         [ "$root_gid" = "-1" ] && root_gid=$(id -g || echo 0)
>                         shift
>                         ;;
> +               "-d")   # date for file mtimes
> +                       timestamp="$(date -d"$1" +%s || :)"
> +                       if test -n "$timestamp"; then
> +                               timestamp="-t $timestamp"
> +                       fi
> +                       shift
> +                       ;;
>                 "-h")
>                         usage
>                         exit 0
> @@ -237,11 +246,4 @@ done
>
>  # If output_file is set we will generate cpio archive
>  # we are careful to delete tmp files
> -timestamp=
> -if test -n "$KBUILD_BUILD_TIMESTAMP"; then
> -       timestamp="$(date -d"$KBUILD_BUILD_TIMESTAMP" +%s || :)"
> -       if test -n "$timestamp"; then
> -               timestamp="-t $timestamp"
> -       fi
> -fi
>  usr/gen_init_cpio $timestamp $cpio_list > $output
> --
> 2.39.2
>


-- 
Best Regards
Masahiro Yamada