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

[Peter Zijlstra]

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
>