linux-kernel - Re: [PATCH v3 37/39] objtool: Optionally WARN about unused ANNOTATE

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]

Date:   Fri, 4 Mar 2022 10:27:00 -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, mbenes@...e.cz, rostedt@...dmis.org,
        mhiramat@...nel.org, alexei.starovoitov@...il.com
Subject: Re: [PATCH v3 37/39] objtool: Optionally WARN about unused
 ANNOTATE_NOENDBR

On Thu, Mar 03, 2022 at 12:23:58PM +0100, Peter Zijlstra wrote:
>  bool no_fp, no_unreachable, retpoline, module, backtrace, uaccess, stats,
>       lto, vmlinux, mcount, noinstr, backup, sls, dryrun,
> -     ibt;
> +     ibt, ibt_warn;
>  
>  static const char * const check_usage[] = {
>  	"objtool check [<options>] file.o",
> @@ -49,6 +49,7 @@ const struct option check_options[] = {
>  	OPT_BOOLEAN('S', "sls", &sls, "validate straight-line-speculation"),
>  	OPT_BOOLEAN(0, "dry-run", &dryrun, "don't write the modifications"),
>  	OPT_BOOLEAN(0, "ibt", &ibt, "validate ENDBR placement"),
> +	OPT_BOOLEAN(0, "ibt-warn", &ibt_warn, "warn about unused ANNOTATE_ENDBR"),

"ibt-warn" doesn't really describe it.  Most options warn.

"ibt-unused-annotation"?

>  	OPT_END(),
>  };
>  
> --- a/tools/objtool/check.c
> +++ b/tools/objtool/check.c
> @@ -1814,6 +1814,19 @@ static void set_func_state(struct cfi_st
>  	state->stack_size = initial_func_cfi.cfa.offset;
>  }
>  
> +static bool insn_is_endbr(struct instruction *insn)
> +{
> +	if (insn->type == INSN_ENDBR)
> +		return true;
> +
> +	if (insn->noendbr) {
> +		insn->noendbr_hit = 1;
> +		return true;
> +	}
> +
> +	return false;
> +}

Ew...

> @@ -3695,6 +3707,31 @@ static int validate_ibt(struct objtool_f
>  		}
>  	}
>  
> +	if (ibt_warn) {
> +		struct symbol *hypercall_page = find_symbol_by_name(file->elf, "hypercall_page");
> +		struct instruction *insn;
> +
> +		for_each_insn(file, insn) {
> +			if (!insn->noendbr || insn->noendbr_hit)
> +				continue;
> +
> +			if (hypercall_page) {
> +				/*
> +				 * The Xen hypercall page contains many
> +				 * hypercalls (and unused slots) that are never
> +				 * indirectly called. Still every slot has an
> +				 * annotation. Suppress complaints.
> +				 */
> +				if (insn->sec == hypercall_page->sec &&
> +				    insn->offset >= hypercall_page->offset &&
> +				    insn->offset <  hypercall_page->offset + hypercall_page->len)
> +					continue;
> +			}
> +
> +			WARN_FUNC("unused ANNOTATE_NOENDBR", insn->sec, insn->offset);
> +		}
> +	}

Ewww... :-/

I can see how this option was useful for development.  But I'm never
going to use it.  Are you?  I'd much rather just drop it.

-- 
Josh