linux-kernel - Re: [PATCH v2 3/3] objtool: Add noinstr validation for static branches/calls

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: <xhsmhed2p560i.mognet@vschneid-thinkpadt14sgen2i.remote.csb>
Date: Tue, 03 Dec 2024 13:02:21 +0100
From: Valentin Schneider <vschneid@...hat.com>
To: Josh Poimboeuf <jpoimboe@...nel.org>
Cc: linux-kernel@...r.kernel.org, Peter Zijlstra <peterz@...radead.org>
Subject: Re: [PATCH v2 3/3] objtool: Add noinstr validation for static
 branches/calls

On 26/11/24 20:47, Josh Poimboeuf wrote:
> @@ -3532,14 +3577,29 @@ static inline bool noinstr_call_dest(struct objtool_file *file,
>       return false;
>  }
>
> +static char *static_call_name(struct symbol *func)
> +{
> +	return func->name + strlen("__SCT__");
> +}
> +
>  static int validate_call(struct objtool_file *file,
>                        struct instruction *insn,
>                        struct insn_state *state)
>  {
> -	if (state->noinstr && state->instr <= 0 &&
> -	    !noinstr_call_dest(file, insn, insn_call_dest(insn))) {
> -		WARN_INSN(insn, "call to %s() leaves .noinstr.text section", call_dest_name(insn));
> -		return 1;
> +	if (state->noinstr && state->instr <= 0) {
> +		struct symbol *dest = insn_call_dest(insn);
> +

Interestingly only after I tried making __flush_tlb_all() noinstr did this
start causing a segfault due to a NULL dest. I added a NULL check here but
I wonder if I'm not just plastering over some other issue?

> +		if (dest->static_call_tramp) {
> +			if (!dest->noinstr_allowed) {
> +				WARN_INSN(insn, "%s: non-RO static call usage in noinstr",
> +					  static_call_name(dest));
> +			}
> +
> +		} else if (!noinstr_call_allowed(file, insn, dest)) {
> +			WARN_INSN(insn, "call to %s() leaves .noinstr.text section",
> +				  call_dest_name(insn));
> +			return 1;
> +		}
>       }
>
>       if (state->uaccess && !func_uaccess_safe(insn_call_dest(insn))) {