linux-hardening - Re: [RFC PATCH 09/11] kbuild: simplify commands in --dry-run mode

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20241114-intelligent-dexterous-dachshund-637fa9@lindesnes>
Date: Thu, 14 Nov 2024 11:48:07 +0100
From: Nicolas Schier <nicolas@...sle.eu>
To: Vegard Nossum <vegard.nossum@...cle.com>
Cc: Masahiro Yamada <masahiroy@...nel.org>, linux-kbuild@...r.kernel.org,
	Nathan Chancellor <nathan@...nel.org>,
	Michael Ellerman <mpe@...erman.id.au>,
	Morten Linderud <morten@...derud.pw>,
	Haelwenn Monnier <contact@...odan.eu>, Jann Horn <jannh@...gle.com>,
	Kees Cook <kees@...nel.org>,
	James Bottomley <James.Bottomley@...senpartnership.com>,
	Theodore Ts'o <tytso@....edu>, linux-hardening@...r.kernel.org
Subject: Re: [RFC PATCH 09/11] kbuild: simplify commands in --dry-run mode

On Mon, Aug 19, 2024 at 06:03:06PM +0200, Vegard Nossum wrote:
> - $filechk is used to check if a file is up to date.
> 
> - $cmd includes logic for echoing commands and deleting intermediate
>   files on interrupt. Skip all of that in --dry-run mode and just execute
>   the command.
> 
> - $cmd_and_savecmd executes the command and echoes it into .<target>.cmd.
> 
> - $if_changed_dep executes the command if any dependencies have changed.
> 
> - $cmd_and_fixdep executes the command and updates .<target>.cmd.
> 
> Skip all of that in --dry-run mode and just execute the command.

Why do you want to skip these checks?  The more mechanisms we change for
--dry-run, the less the output shell script is a good way to review with
regard to security issues.  And it "fools" those who want to know what
really happens if 'make' is run w/o --dry-run.

> 
> Signed-off-by: Vegard Nossum <vegard.nossum@...cle.com>
> ---
>  scripts/Kbuild.include | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
> index ed8a7493524b2..a1ef3b1828bb3 100644
> --- a/scripts/Kbuild.include
> +++ b/scripts/Kbuild.include
> @@ -94,6 +94,7 @@ kecho := $($(quiet)kecho)
>  # - If no file exist it is created
>  # - If the content differ the new file is used
>  # - If they are equal no change, and no timestamp update
> +ifndef dry_run
>  define filechk
>  	$(check-FORCE)
>  	$(Q)set -e;						\
> @@ -105,6 +106,14 @@ define filechk
>  		mv -f $(tmp-target) $@;				\
>  	fi
>  endef
> +else
> +# simplify and write the output directly if we're just printing
> +# the commands
> +define filechk
> +	mkdir -p $(dir $@)
> +	{ $(filechk_$(1)); } > $@

To prevent having incomplete or broken output files of filechk, I'd like
to have at least the 'trap' here, too.  E.g.:

	(						\
		set -e;					\
		mkdir -p $(dir $@);			\
		trap "rm -f $@" EXIT;			\
		{ $(filechk_$(1)); } >$@;		\
     	)

Did you see problems with the original filechk implementation?

Kind regards,
Nicolas