linux-kernel - Re: [PATCH] objtool: Fix SLS checks

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: <20220503211510.ytdv5l4l2zwjdwg7@treble>
Date:   Tue, 3 May 2022 14:15:10 -0700
From:   Josh Poimboeuf <jpoimboe@...hat.com>
To:     Peter Zijlstra <peterz@...radead.org>
Cc:     x86@...nel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] objtool: Fix SLS checks

On Mon, May 02, 2022 at 10:17:39PM +0200, Peter Zijlstra wrote:
> > +++ b/tools/objtool/check.c
> > @@ -3842,9 +3842,6 @@ static int validate_sls(struct objtool_file *file)
> >  	for_each_insn(file, insn) {
> >  		next_insn = next_insn_same_sec(file, insn);
> >  
> > -		if (insn->retpoline_safe)
> > -			continue;
> > -
> >  		switch (insn->type) {
> >  		case INSN_RETURN:
> >  			if (!next_insn || next_insn->type != INSN_TRAP) {
> 
> Yes, agreed. But perhaps with something like this on top?

Yup, I missed those...  Looks good.  Just one comment:

> +++ b/arch/x86/kernel/alternative.c
> @@ -452,6 +452,17 @@ static int patch_retpoline(void *addr, struct insn *insn, u8 *bytes)
>  		return ret;
>  	i += ret;
>  
> +#ifdef CONFIG_SLS
> +	/*
> +	 * Ideally this would be unconditional, except in case of
> +	 * RETPOLINE_LFENCE we don't have sufficient space. Additionally,
> +	 * -mharden-sls=all should be extended to emit INT3 after
> +	 * direct jumps too, which will then cover that case.
> +	 */

I don't quite follow this 2nd sentence and how it's related here, since
this function doesn't actually deal with direct jumps.

Speaking of, I guess we'll eventually need to hack this SLS mess into
jump labels :-/

> +	if (i < insn->length)
> +		bytes[i++] = INT3_INSN_OPCODE;
> +#endif

-- 
Josh