linux-kernel - Re: [PATCH 10/18] objtool: Extricate ibt from stack validation

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 14 Apr 2022 09:53:18 +0200
From:   Peter Zijlstra <peterz@...radead.org>
To:     Josh Poimboeuf <jpoimboe@...hat.com>
Cc:     x86@...nel.org, linux-kernel@...r.kernel.org,
        Miroslav Benes <mbenes@...e.cz>
Subject: Re: [PATCH 10/18] objtool: Extricate ibt from stack validation

On Wed, Apr 13, 2022 at 04:19:45PM -0700, Josh Poimboeuf wrote:
> Extricate ibt from validate_branch() in preparation for making stack
> validation optional.

It does a bit more..


> -		/* already done in validate_branch() */
> -		if (sec->sh.sh_flags & SHF_EXECINSTR)
> -			continue;
>  
> -		if (!sec->reloc)
>  			continue;
>  
> -		if (!strncmp(sec->name, ".orc", 4))
> -			continue;
>  
> -		if (!strncmp(sec->name, ".discard", 8))
>  			continue;
>  
> -		if (!strncmp(sec->name, ".debug", 6))
>  			continue;
>  
> -		if (!strcmp(sec->name, "_error_injection_whitelist"))
>  			continue;
>  
> -		if (!strcmp(sec->name, "_kprobe_blacklist"))
>  			continue;
>  
> -		is_data = strstr(sec->name, ".data") || strstr(sec->name, ".rodata");
>  
> -		list_for_each_entry(reloc, &sec->reloc->reloc_list, list) {
> -			struct instruction *dest;
>  
> -			dest = validate_ibt_reloc(file, reloc);
> -			if (is_data && dest && !dest->noendbr)
> -				warn_noendbr("data ", sec, reloc->offset, dest);
> -		}

So this iterates all sections and excludes a bunch, and only reports
fail for .data/.rodata.

> +static int validate_ibt(struct objtool_file *file)
> +{
> +	struct section *sec;
> +	struct reloc *reloc;
> +	struct instruction *insn;
> +	int warnings = 0;
> +
> +	for_each_insn(file, insn)
> +		warnings += validate_ibt_insn(file, insn);

So I specifically didn't do this because I wanted to reduce the amount
of loops we do over those instructions. But yeah, if you really want to
allow --ibt without --stack-validate (but why?) then I suppose so.

Esp. for the vmlinux.o case, iterating all insn can quickly add up to
significant time.

> +	for_each_sec(file, sec) {
> +
> +		if (!strstr(sec->name, ".data") && !strstr(sec->name, ".rodata"))
> +			continue;

But this only iterates .data/.rodata.

That's not the same, specifically, it'll not iterate stuff like ksymtab
that contains the EXPORT_SYMBOL* crud. The result being that we can now
seal EXPORT'ed symbols, which will make modules really sad.

There's also the .initcall sections, sealing initcalls typcally ends really
badly.

And there might be a few others I forgot about.

> +		if (!sec->reloc)
> +			continue;
> +
> +		list_for_each_entry(reloc, &sec->reloc->reloc_list, list)
> +			warnings += validate_ibt_data_reloc(file, reloc);
> +	}
> +
> +	return warnings;
>  }
>  
>  static int validate_reachable_instructions(struct objtool_file *file)
> -- 
> 2.34.1
>