linux-kernel - Re: [PATCH 3/3] objtool: optimize add_dead

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:   Tue, 21 Apr 2020 15:13:05 -0500
From:   Josh Poimboeuf <jpoimboe@...hat.com>
To:     Sami Tolvanen <samitolvanen@...gle.com>
Cc:     Peter Zijlstra <peterz@...radead.org>,
        Kees Cook <keescook@...omium.org>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/3] objtool: optimize add_dead_ends for split sections

On Tue, Apr 21, 2020 at 11:07:24AM -0700, Sami Tolvanen wrote:
> @@ -338,15 +351,13 @@ static int add_dead_ends(struct objtool_file *file)
>  		if (insn)
>  			insn = list_prev_entry(insn, list);
>  		else if (rela->addend == rela->sym->sec->len) {
> -			found = false;
> -			list_for_each_entry_reverse(insn, &file->insn_list, list) {
> -				if (insn->sec == rela->sym->sec) {
> -					found = true;
> -					break;
> -				}
> -			}
> +			struct section_info *sec_info = (struct section_info *)
> +				rela->sym->sec->section_info;
> +
> +			if (sec_info)
> +				insn = sec_info->last_insn;
>  
> -			if (!found) {
> +			if (!insn) {
>  				WARN("can't find unreachable insn at %s+0x%x",
>  				     rela->sym->sec->name, rela->addend);
>  				return -1;

Instead of the 'section_info' abstraction I think I'd rather just store
the 'last_insn' pointer directly in the section struct.

Also, the unreachable annotation at the end of a section is really an
edge case.  I'm sort of wondering if there's a way to accomplish the
same thing without storing the last_insn.

For example, I wonder if we could use find_insn() for some bytes at the
end of the section.  Most of the time I _think_ there will be a two-byte
UD2 instruction there anyway.  So maybe we could do something like:

	for (offset = rela->sym->sec->len - 1;
	     offset > rela->sym->sec->len - 10;
	     offset --) {

	     insn = find_insn(file, rela->sym->sec, offset);
	     if (insn)
	     	break;
	}

It's kind of ugly, but then we could maybe avoid the need for the
last_insn thing.

BTW, just curious, what's your use case for -ffunction-sections?  Is it
for fgkaslr?

-- 
Josh