linux-kernel - Re: [PATCH 21/29] objtool: Rename --duplicate to --lto

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: <20220226194209.bvv3t65hhtnwltmk@treble>
Date:   Sat, 26 Feb 2022 11:42:09 -0800
From:   Josh Poimboeuf <jpoimboe@...hat.com>
To:     Peter Zijlstra <peterz@...radead.org>
Cc:     x86@...nel.org, joao@...rdrivepizza.com, hjl.tools@...il.com,
        andrew.cooper3@...rix.com, linux-kernel@...r.kernel.org,
        ndesaulniers@...gle.com, keescook@...omium.org,
        samitolvanen@...gle.com, mark.rutland@....com,
        alyssa.milburn@...el.com
Subject: Re: [PATCH 21/29] objtool: Rename --duplicate to --lto

On Fri, Feb 18, 2022 at 05:49:23PM +0100, Peter Zijlstra wrote:
> In order to prepare for LTO like objtool runs for modules, rename the
> duplicate argument to lto.
> 
> Signed-off-by: Peter Zijlstra (Intel) <peterz@...radead.org>
> ---
>  scripts/link-vmlinux.sh                 |    2 +-
>  tools/objtool/builtin-check.c           |    4 ++--
>  tools/objtool/check.c                   |    7 ++++++-
>  tools/objtool/include/objtool/builtin.h |    2 +-
>  4 files changed, 10 insertions(+), 5 deletions(-)
> 
> --- a/scripts/link-vmlinux.sh
> +++ b/scripts/link-vmlinux.sh
> @@ -115,7 +115,7 @@ objtool_link()
>  			objtoolcmd="orc generate"
>  		fi
>  
> -		objtoolopt="${objtoolopt} --duplicate"
> +		objtoolopt="${objtoolopt} --lto"
>  
>  		if is_enabled CONFIG_FTRACE_MCOUNT_USE_OBJTOOL; then
>  			objtoolopt="${objtoolopt} --mcount"
> --- a/tools/objtool/builtin-check.c
> +++ b/tools/objtool/builtin-check.c
> @@ -20,7 +20,7 @@
>  #include <objtool/objtool.h>
>  
>  bool no_fp, no_unreachable, retpoline, module, backtrace, uaccess, stats,
> -     validate_dup, vmlinux, mcount, noinstr, backup, sls, dryrun;
> +     lto, vmlinux, mcount, noinstr, backup, sls, dryrun;
>  
>  static const char * const check_usage[] = {
>  	"objtool check [<options>] file.o",
> @@ -40,7 +40,7 @@ const struct option check_options[] = {
>  	OPT_BOOLEAN('b', "backtrace", &backtrace, "unwind on error"),
>  	OPT_BOOLEAN('a', "uaccess", &uaccess, "enable uaccess checking"),
>  	OPT_BOOLEAN('s', "stats", &stats, "print statistics"),
> -	OPT_BOOLEAN('d', "duplicate", &validate_dup, "duplicate validation for vmlinux.o"),
> +	OPT_BOOLEAN(0, "lto", &lto, "whole-archive like runs"),

"--lto" is a confusing name, since this "feature" isn't specific to LTO.

Also, it gives no indication of what it actually does.

What it does is, run objtool on vmlinux or module just like it's a
normal object, and *don't* do noinstr validation.  Right?

It's weird for the noinstr-only-mode to be the default.

BTW "--duplicate" had similar problems...

So how about:

- Default to normal mode on vmlinux/module, i.e. validate and/or
  generate ORC like any other object.  This default is more logically
  consistent and makes sense for the future once we get around to
  parallelizing objtool.

- Have "--noinstr", which does noinstr validation, in addition to all
  the other objtool validation/generation.  So it's additive, like any
  other cmdline option.  (Maybe this option isn't necessarily needed for
  now.)

- Have "--noinstr-only" which only does noinstr validation and nothing
  else.  (Alternatively, "--noinstr --dry-run")

?

-- 
Josh