linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: Ard Biesheuvel <ardb@kernel.org>
Cc: Mark-PK Tsai <mark-pk.tsai@mediatek.com>,
	Linux Kbuild mailing list <linux-kbuild@vger.kernel.org>,
	linux-toolchains@vger.kernel.org,
	Linux ARM <linux-arm-kernel@lists.infradead.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	linux-mediatek@lists.infradead.org,
	Matthias Brugger <matthias.bgg@gmail.com>,
	Matt Helsley <mhelsley@vmware.com>,
	"Steven Rostedt (VMware)" <rostedt@goodmis.org>,
	Sami Tolvanen <samitolvanen@google.com>,
	yj.chiang@mediatek.com
Subject: Re: [PATCH] recordmcount: avoid using ABS symbol as reference
Date: Mon, 7 Jun 2021 13:44:21 +0200	[thread overview]
Message-ID: <YL4GlbfMJiuLkRhR@hirez.programming.kicks-ass.net> (raw)
In-Reply-To: <CAMj1kXGCoME4Wy4e3FNAjWLY=G56ivHzFTLrXRE0mLtnaBVEDQ@mail.gmail.com>

On Mon, Jun 07, 2021 at 11:50:40AM +0200, Ard Biesheuvel wrote:
> On Mon, 7 Jun 2021 at 10:06, Mark-PK Tsai <mark-pk.tsai@mediatek.com> wrote:
> >
> > > > On Mon, 7 Jun 2021 at 08:59, Mark-PK Tsai <mark-pk.tsai@mediatek.com> wrote:
> > > > >
> > > > > > > On Mon, 7 Jun 2021 at 04:42, Mark-PK Tsai <mark-pk.tsai@mediatek.com> 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.
> > >
> > > > > 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)

  parent reply	other threads:[~2021-06-07 11:44 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-07  2:38 [PATCH] recordmcount: avoid using ABS symbol as reference Mark-PK Tsai
2021-06-07  6:37 ` Ard Biesheuvel
2021-06-07  6:59   ` Mark-PK Tsai
2021-06-07  7:07     ` Ard Biesheuvel
2021-06-07  7:42       ` Mark-PK Tsai
2021-06-07  8:06         ` Mark-PK Tsai
2021-06-07  9:50           ` Ard Biesheuvel
2021-06-07 10:32             ` Mark-PK Tsai
2021-06-07 11:44             ` Peter Zijlstra [this message]
2021-06-07 13:18               ` Mark-PK Tsai
2021-06-07 13:44               ` Steven Rostedt
2021-06-07 17:31                 ` Mark-PK Tsai
2021-06-07 15:40               ` Peter Zijlstra
2021-06-08  1:15                 ` Mark-PK Tsai

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YL4GlbfMJiuLkRhR@hirez.programming.kicks-ass.net \
    --to=peterz@infradead.org \
    --cc=ardb@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kbuild@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mediatek@lists.infradead.org \
    --cc=linux-toolchains@vger.kernel.org \
    --cc=mark-pk.tsai@mediatek.com \
    --cc=matthias.bgg@gmail.com \
    --cc=mhelsley@vmware.com \
    --cc=rostedt@goodmis.org \
    --cc=samitolvanen@google.com \
    --cc=yj.chiang@mediatek.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).