linux-kernel - Re: [PATCH v2 2/2] objtool: Move sync check to a script

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: <20171107094621.kfpykhbncikmeyms@gmail.com>
Date:   Tue, 7 Nov 2017 10:46:21 +0100
From:   Ingo Molnar <mingo@...nel.org>
To:     Josh Poimboeuf <jpoimboe@...hat.com>
Cc:     linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 2/2] objtool: Move sync check to a script


* Josh Poimboeuf <jpoimboe@...hat.com> wrote:

> Replace the nasty diff checks in the objtool Makefile with a clean bash
> script, and make the warnings more specific.
> 
> Heavily inspired by tools/perf/check-headers.sh.
> 
> Suggested-by: Ingo Molnar <mingo@...nel.org>
> Signed-off-by: Josh Poimboeuf <jpoimboe@...hat.com>
> ---
>  tools/objtool/Makefile      | 16 +---------------
>  tools/objtool/sync-check.sh | 29 +++++++++++++++++++++++++++++
>  2 files changed, 30 insertions(+), 15 deletions(-)
>  create mode 100755 tools/objtool/sync-check.sh
> 
> diff --git a/tools/objtool/Makefile b/tools/objtool/Makefile
> index c6a19d946ec1..6aaed251b4ed 100644
> --- a/tools/objtool/Makefile
> +++ b/tools/objtool/Makefile
> @@ -43,22 +43,8 @@ include $(srctree)/tools/build/Makefile.include
>  $(OBJTOOL_IN): fixdep FORCE
>  	@$(MAKE) $(build)=objtool
>  
> -# Busybox's diff doesn't have -I, avoid warning in that case
> -#
>  $(OBJTOOL): $(LIBSUBCMD) $(OBJTOOL_IN)
> -	@(diff -I 2>&1 | grep -q 'option requires an argument' && \
> -	test -d ../../kernel -a -d ../../tools -a -d ../objtool && (( \
> -	diff arch/x86/lib/insn.c ../../arch/x86/lib/insn.c >/dev/null && \
> -	diff arch/x86/lib/inat.c ../../arch/x86/lib/inat.c >/dev/null && \
> -	diff arch/x86/lib/x86-opcode-map.txt ../../arch/x86/lib/x86-opcode-map.txt >/dev/null && \
> -	diff arch/x86/tools/gen-insn-attr-x86.awk ../../arch/x86/tools/gen-insn-attr-x86.awk >/dev/null && \
> -	diff arch/x86/include/asm/insn.h ../../arch/x86/include/asm/insn.h >/dev/null && \
> -	diff arch/x86/include/asm/inat.h ../../arch/x86/include/asm/inat.h >/dev/null && \
> -	diff arch/x86/include/asm/inat_types.h ../../arch/x86/include/asm/inat_types.h >/dev/null) \
> -	|| echo "warning: objtool: x86 instruction decoder differs from kernel" >&2 )) || true
> -	@(test -d ../../kernel -a -d ../../tools -a -d ../objtool && (( \
> -	diff ../../arch/x86/include/asm/orc_types.h arch/x86/include/asm/orc_types.h >/dev/null) \
> -	|| echo "warning: objtool: orc_types.h differs from kernel" >&2 )) || true
> +	@./sync-check.sh
>  	$(QUIET_LINK)$(CC) $(OBJTOOL_IN) $(LDFLAGS) -o $@
>  
>  
> diff --git a/tools/objtool/sync-check.sh b/tools/objtool/sync-check.sh
> new file mode 100755
> index 000000000000..f2e5f8b0460c
> --- /dev/null
> +++ b/tools/objtool/sync-check.sh
> @@ -0,0 +1,29 @@
> +#!/bin/sh
> +# SPDX-License-Identifier: GPL-2.0
> +
> +FILES='
> +arch/x86/lib/insn.c
> +arch/x86/lib/inat.c
> +arch/x86/lib/x86-opcode-map.txt
> +arch/x86/tools/gen-insn-attr-x86.awk
> +arch/x86/include/asm/insn.h
> +arch/x86/include/asm/inat.h
> +arch/x86/include/asm/inat_types.h
> +arch/x86/include/asm/orc_types.h
> +'
> +
> +check()
> +{
> +	local file=$1
> +
> +	diff $file ../../$file &> /dev/null ||
> +		echo "Warning: synced file at 'tools/objtool/$file' differs from latest kernel version at '$file'"
> +}
> +
> +if [ ! -d ../../kernel ] || [ ! -d ../../tools ] || [ ! -d ../objtool ]; then
> +	exit 0
> +fi
> +
> +for i in $FILES; do
> +  check $i
> +done

Hm, this doesn't actually warn - it outputs the diff:

 triton:~/tip/tools/objtool> ./sync-check.sh 
 triton:~/tip/tools/objtool> 99a100,109
 > /* Identifiers for segment registers */
 > #define INAT_SEG_REG_IGNORE   0
 > #define INAT_SEG_REG_DEFAULT  1
 > #define INAT_SEG_REG_CS               2
 > #define INAT_SEG_REG_SS               3
 > #define INAT_SEG_REG_DS               4
 > #define INAT_SEG_REG_ES               5
 > #define INAT_SEG_REG_FS               6
 > #define INAT_SEG_REG_GS               7
 > 

I fixed it to do:

        diff $file ../../$file > /dev/null ||

(note the removal of '&')

Then it outputs the right thing:

  triton:~/tip/tools/objtool> ./sync-check.sh 
  Warning: synced file at 'tools/objtool/arch/x86/include/asm/inat.h' differs from latest kernel version at 'arch/x86/include/asm/inat.h'

Thanks,

	Ingo