linux-kernel - Re: [PATCH] scripts/setlocalversion: make git describe output more reliable

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: <91104720-e389-a53d-37f6-1de4d665474f@roeck-us.net>
Date:   Thu, 10 Sep 2020 07:28:25 -0700
From:   Guenter Roeck <linux@...ck-us.net>
To:     Rasmus Villemoes <linux@...musvillemoes.dk>,
        Masahiro Yamada <yamada.masahiro@...ionext.com>,
        Randy Dunlap <rdunlap@...radead.org>,
        Brian Norris <briannorris@...omium.org>,
        Bhaskar Chowdhury <unixbhaskar@...il.com>
Cc:     linux-kernel@...r.kernel.org
Subject: Re: [PATCH] scripts/setlocalversion: make git describe output more
 reliable

On 9/10/20 4:26 AM, Rasmus Villemoes wrote:
> When building for an embedded target using Yocto, we're sometimes
> observing that the version string that gets built into vmlinux (and
> thus what uname -a reports) differs from the path under /lib/modules/
> where modules get installed in the rootfs, but only in the length of
> the -gabc123def suffix. Hence modprobe always fails.
> 
> The problem is that Yocto has the concept of "sstate" (shared state),
> which allows different developers/buildbots/etc. to share build
> artifacts, based on a hash of all the metadata that went into building
> that artifact - and that metadata includes all dependencies (e.g. the
> compiler used etc.). That normally works quite well; usually a clean
> build (without using any sstate cache) done by one developer ends up
> being binary identical to a build done on another host. However, one
> thing that can cause two developers to end up with different builds
> [and thus make one's vmlinux package incompatible with the other's
> kernel-dev package], which is not captured by the metadata hashing, is
> this `git describe`: The output of that can be affected by
> 
> (1) git version: before 2.11 git defaulted to a minimum of 7, since
> 2.11 (git.git commit e6c587) the default is dynamic based on the
> number of objects in the repo
> (2) hence even if both run the same git version, the output can differ
> based on how many remotes are being tracked (or just lots of local
> development branches or plain old garbage)
> (3) and of course somebody could have a core.abbrev config setting in
> ~/.gitconfig
> 
> So in order to avoid `uname -a` output relying on such random details
> of the build environment which are rather hard to ensure are
> consistent between developers and buildbots, use an explicit
> --abbrev=15 option (and for consistency, also use rev-parse --short=15
> for the unlikely case of no signed tags being usable).
> 
> Now, why is 60 bits enough for everyone? It's not mathematically
> guaranteed that git won't have to use 16 in some git repo, but it is
> beyond unlikely: Even in a repo with 100M objects, the probability
> that any given commit (i.e. the one being described) clashes with some
> other object in the first 15 hex chars is less than 1e-10, and
> currently a git repo tracking Linus', -stable and -rt only has around
> 10M objects.
> 
> Signed-off-by: Rasmus Villemoes <linux@...musvillemoes.dk>

Makes sense to me.

Acked-by: Guenter Roeck <linux@...ck-us.net>

> ---
> I could probably fix things by adding a 'git config --local
> core.abbrev 15' step to the Yocto build process after the repo to
> build from has been cloned but before building has started. But in the
> interest of binary reproducibility outside of just Yocto builds, I
> think it's better if this lives in the kernel.
> 
>  scripts/setlocalversion | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/scripts/setlocalversion b/scripts/setlocalversion
> index 20f2efd57b11..c5262f0d953d 100755
> --- a/scripts/setlocalversion
> +++ b/scripts/setlocalversion
> @@ -45,7 +45,7 @@ scm_version()
>  
>  	# Check for git and a git repo.
>  	if test -z "$(git rev-parse --show-cdup 2>/dev/null)" &&
> -	   head=$(git rev-parse --verify --short HEAD 2>/dev/null); then
> +	   head=$(git rev-parse --verify --short=15 HEAD 2>/dev/null); then
>  
>  		# If we are at a tagged commit (like "v2.6.30-rc6"), we ignore
>  		# it, because this version is defined in the top level Makefile.
> @@ -59,7 +59,7 @@ scm_version()
>  			fi
>  			# If we are past a tagged commit (like
>  			# "v2.6.30-rc5-302-g72357d5"), we pretty print it.
> -			if atag="$(git describe 2>/dev/null)"; then
> +			if atag="$(git describe --abbrev=15 2>/dev/null)"; then
>  				echo "$atag" | awk -F- '{printf("-%05d-%s", $(NF-1),$(NF))}'
>  
>  			# If we don't have a tag at all we print -g{commitish}.
>