Re: [PATCH] recordmcount: avoid using ABS symbol as reference

On Mon, Jun 07, 2021 at 11:50:40AM +0200, Ard Biesheuvel wrote:

On Mon, 7 Jun 2021 at 10:06, Mark-PK Tsai [off-list ref] wrote:

quoted

quoted

quoted

On Mon, 7 Jun 2021 at 08:59, Mark-PK Tsai [off-list ref] wrote:

quoted

quoted

quoted

On Mon, 7 Jun 2021 at 04:42, Mark-PK Tsai [off-list ref] wrote:

Avoid using ABS symbol, which won't be relocate, as reference.

On arm64 platform, if there's shndx equals SHN_ABS(0xfff1).

Section Headers:
[Nr]    Name                         Type      Address          Off      Size   ES  Flg Lk     Inf    Al
[65521] .text.n_tty_receive_buf      PROGBITS  0000000000000000 3cdab520 000054 00  AX  0      0      4
[65522] .rela.text.n_tty_receive_buf RELA      0000000000000000 3cdab578 000030 18  I   152076 65521  8

A RELA section's r_info field points to the section to which it
applies. This is why in the example above section #65522 points to
section #65521. This has nothing to do with the numerical value of
SHN_ABS.

If the r_info of RELA section is 65521(0xfff1),

Oh sorry, I mean sh_info here.

quoted

quoted

find_secsym_ndx() will use it to find the base symbol.

But what does that have to do with the sh_info field of the RELA
section's Elf_Shdr struct? IOW, what is the relevance of section
#65521 here?

So what I mean is the problem occur if the sh_info of a RELA section
is #65521.

Actually the problem occur if the sh_info of a RELA section is in
the special section index range(SHN_LORESERVE ~ SHN_HIRESERVE).
Maybe I should add a is_shndx_special() to check this like
scripts/mod/modpost.h did?

So if I understand all of this correctly, we are running into a
fundamental issue here, where the linker emits more sections than the
sh_info field can describe, overflowing into the reserved range.

I don't think papering over it like this is going to be maintainable
going forward.

There's an extended section header index section for just that. And
recordmcount actually seems to use that as well.

I can't seem to find enough of the thread to figure out what the actual
problem is though. The lore archive doesn't have anything prior to this
message.

One should only use st_shndx when >SHN_UDEF and <SHN_LORESERVE. When
SHN_XINDEX, then use .symtab_shndx.

Apparently you've found a case where neither is true? In that case
objtool seems to use shndx 0. A matching recordmcount patch would be
something like this.

diff --git a/scripts/recordmcount.h b/scripts/recordmcount.h
index f9b19524da11..d99cc0aed6fe 100644
--- a/scripts/recordmcount.h
+++ b/scripts/recordmcount.h

@@ -194,13 +194,18 @@ static unsigned int get_symindex(Elf_Sym const *sym, Elf32_Word const *symtab,
 	unsigned long offset;
 	int index;
 
-	if (sym->st_shndx != SHN_XINDEX)
+	if (sym->st_shndx > SHN_UDEF &&
+	    sym->st_shndx < SHN_LORESERVE)
 		return w2(sym->st_shndx);
 
-	offset = (unsigned long)sym - (unsigned long)symtab;
-	index = offset / sizeof(*sym);
+	if (sym->st_shndx == SHN_XINDEX) {
+		offset = (unsigned long)sym - (unsigned long)symtab;
+		index = offset / sizeof(*sym);
 
-	return w(symtab_shndx[index]);
+		return w(symtab_shndx[index]);
+	}
+
+	return 0;
 }
 
 static unsigned int get_shnum(Elf_Ehdr const *ehdr, Elf_Shdr const *shdr0)

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel