linux-kernel - Re: [PATCH v2 12/14] objtool: Allow archs to rewrite retpolines

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:   Thu, 18 Mar 2021 21:54:40 -0500
From:   Josh Poimboeuf <jpoimboe@...hat.com>
To:     Peter Zijlstra <peterz@...radead.org>
Cc:     x86@...nel.org, jgross@...e.com, mbenes@...e.com,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 12/14] objtool: Allow archs to rewrite retpolines

On Thu, Mar 18, 2021 at 06:11:15PM +0100, Peter Zijlstra wrote:
> @@ -1212,6 +1225,8 @@ static int handle_group_alt(struct objto
>  		dest_off = arch_jump_destination(insn);
>  		if (dest_off == special_alt->new_off + special_alt->new_len)
>  			insn->jump_dest = next_insn_same_sec(file, last_orig_insn);
> +		else
> +			insn->jump_dest = find_insn(file, insn->sec, dest_off);
>  
>  		if (!insn->jump_dest) {
>  			WARN_FUNC("can't find alternative jump destination",

So I assume this change is because of the ordering change: now this is
done before add_jump_destinations().

But doesn't that mean the alternative jump modification (changing the
dest to the end of the original insns) will get overwritten later?

Also the new hunk to be an oversimplified version of
add_jump_destinations().  I'm not quite convinced that it will always do
the right thing for this case.

> @@ -1797,11 +1812,15 @@ static int decode_sections(struct objtoo
>  	if (ret)
>  		return ret;
>  
> -	ret = add_jump_destinations(file);
> +	/*
> +	 * Must be before add_{jump,call}_destination; for they can add
> +	 * magic alternatives.
> +	 */
> +	ret = add_special_section_alts(file);

This reordering is unfortunate.  Maybe there's a better way, though I
don't have any ideas, at least until I get to the most controversial
patch.

-- 
Josh