linux-kernel - Re: [PATCH 6/9] objtool: Add elf_create_undef

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: <alpine.LSU.2.21.2103171444570.23081@pobox.suse.cz>
Date:   Wed, 17 Mar 2021 14:52:23 +0100 (CET)
From:   Miroslav Benes <mbenes@...e.cz>
To:     Peter Zijlstra <peterz@...radead.org>
cc:     x86@...nel.org, jpoimboe@...hat.com, jgross@...e.com,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 6/9] objtool: Add elf_create_undef_symbol()

[ also correcting my e-mail address ]

On Fri, 12 Mar 2021, Peter Zijlstra wrote:

Just a remark regarding SHN_XINDEX...

> +static bool elf_symbol_add(struct elf *elf, struct symbol *sym, Elf32_Word shndx)
> +{
> +	struct list_head *entry;
> +	struct rb_node *pnode;
> +
> +	sym->type = GELF_ST_TYPE(sym->sym.st_info);
> +	sym->bind = GELF_ST_BIND(sym->sym.st_info);
> +
> +	if ((sym->sym.st_shndx > SHN_UNDEF &&
> +	     sym->sym.st_shndx < SHN_LORESERVE) ||
> +	    (shndx != SHN_XINDEX && sym->sym.st_shndx == SHN_XINDEX)) {
> +		if (sym->sym.st_shndx != SHN_XINDEX)
> +			shndx = sym->sym.st_shndx;
> +
> +		sym->sec = find_section_by_index(elf, shndx);
> +		if (!sym->sec) {
> +			WARN("couldn't find section for symbol %s",
> +			     sym->name);
> +			return false;
> +		}

...

> @@ -366,47 +414,11 @@ static int read_symbols(struct elf *elf)
>  			goto err;
>  		}
>  
> -		sym->type = GELF_ST_TYPE(sym->sym.st_info);
> -		sym->bind = GELF_ST_BIND(sym->sym.st_info);
> +		if (!shndx_data)
> +			shndx = SHN_XINDEX;
>  
> -		if ((sym->sym.st_shndx > SHN_UNDEF &&
> -		     sym->sym.st_shndx < SHN_LORESERVE) ||
> -		    (shndx_data && sym->sym.st_shndx == SHN_XINDEX)) {
> -			if (sym->sym.st_shndx != SHN_XINDEX)
> -				shndx = sym->sym.st_shndx;
> -
> -			sym->sec = find_section_by_index(elf, shndx);
> -			if (!sym->sec) {
> -				WARN("couldn't find section for symbol %s",
> -				     sym->name);
> -				goto err;

...

> +	if (!elf_symbol_add(elf, sym, SHN_XINDEX)) {
> +		WARN("elf_symbol_add");
> +		return NULL;
> +	}

SHN_XINDEX means that the extended section index is used. Above you seem 
to use it in the opposite sense too (assigning to shndx when shndx_data is 
NULL). While it makes the code easier to handle, it is a bit confusing 
(and maybe I am just confused now). Could you add a comment about that, 
please? elf_symbol_add() seems like a good place.

Miroslav