linux-kernel - Re: [RFC v5 12/57] objtool: check: Allow jumps from an alternative group to itself

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: <20200121173305.urv77ral76su26cs@treble>
Date:   Tue, 21 Jan 2020 11:33:05 -0600
From:   Josh Poimboeuf <jpoimboe@...hat.com>
To:     Julien Thierry <jthierry@...hat.com>
Cc:     linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        peterz@...radead.org, raphael.gault@....com,
        catalin.marinas@....com, will@...nel.org
Subject: Re: [RFC v5 12/57] objtool: check: Allow jumps from an alternative
 group to itself

On Thu, Jan 09, 2020 at 04:02:15PM +0000, Julien Thierry wrote:
> Alternatives can contain instructions that jump to another instruction
> in the same alternative group. This is actually a common pattern on
> arm64.
> 
> Keep track of instructions jumping within their own alternative group
> and carry on validating such branches.
> 
> Signed-off-by: Julien Thierry <jthierry@...hat.com>
> ---
>  tools/objtool/check.c | 48 ++++++++++++++++++++++++++++++++++---------
>  tools/objtool/check.h |  1 +
>  2 files changed, 39 insertions(+), 10 deletions(-)
> 
> diff --git a/tools/objtool/check.c b/tools/objtool/check.c
> index 8f2ff030936d..c7b3f1e2a628 100644
> --- a/tools/objtool/check.c
> +++ b/tools/objtool/check.c
> @@ -722,6 +722,14 @@ static int handle_group_alt(struct objtool_file *file,
>  	sec_for_each_insn_from(file, insn) {
>  		if (insn->offset >= special_alt->orig_off + special_alt->orig_len)
>  			break;
> +		/* Is insn a jump to an instruction within the alt_group */
> +		if (insn->jump_dest && insn->sec == insn->jump_dest->sec &&
> +		    (insn->type == INSN_JUMP_CONDITIONAL ||
> +		     insn->type == INSN_JUMP_UNCONDITIONAL)) {
> +			dest_off = insn->jump_dest->offset;
> +			insn->intra_group_jump = special_alt->orig_off <= dest_off &&
> +				dest_off < special_alt->orig_off + special_alt->orig_len;
> +		}

This patch adds some complexity, just so we can keep the

  "don't know how to handle branch to middle of alternative instruction group"

warning for the case where code from outside an alternative insn group
is branching to inside the group.  But I've never actually seen that
case in practice, and I get the feeling that warning isn't very useful
or realistic.

How about we just remove the warning?

-- 
Josh