linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
From: "Fāng-ruì Sòng" <maskray@google.com>
To: Michael Ellerman <mpe@ellerman.id.au>
Cc: linux-kernel@vger.kernel.org, clang-built-linux@googlegroups.com,
	Paul Mackerras <paulus@samba.org>,
	linuxppc-dev@lists.ozlabs.org
Subject: Re: [PATCH] powerpc/vdso32: mark __kernel_datapage_offset as STV_PROTECTED
Date: Mon, 10 Feb 2020 10:41:29 -0800	[thread overview]
Message-ID: <CAFP8O3JPJvnQhAcF+DWQQGPNOj9vF-nLovzi5uSQ4zrUP1DvtQ@mail.gmail.com> (raw)
In-Reply-To: <87pnemzoxa.fsf@mpe.ellerman.id.au>

On Mon, Feb 10, 2020 at 3:01 AM Michael Ellerman <mpe@ellerman.id.au> wrote:
>
> Fangrui Song <maskray@google.com> writes:
> > A PC-relative relocation (R_PPC_REL16_LO in this case) referencing a
> > preemptible symbol in a -shared link is not allowed.  GNU ld's powerpc
> > port is permissive and allows it [1], but lld will report an error after
> > https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git/commit/?id=ec0895f08f99515194e9fcfe1338becf6f759d38
> >
> > Make the symbol protected so that it is non-preemptible but still
> > exported.
>
> "preemptible" means something different to me, and I assume we're not
> using it to mean the same thing.
>
> Can you explain it using small words that a kernel developer can
> understand? :)
>
> cheers

The term used in the ELF specification is "preemptable". I heard from
Roland McGrathr that "preemptable" was a typo. The correct term is
"preemptible".
On a random article I found, it mentions that "preemptible" is used
more than "preemptable". So now I stick with "preemptible".

The word is overloaded and has a different meaning in the kernel, but
here we refer to within the ELF binary format context.

From http://www.sco.com/developers/gabi/latest/ch4.symtab.html
"The visibility of symbols with the STV_DEFAULT attribute is as
specified by the symbol's binding type. That is, global and weak
symbols are visible outside of their defining component (executable
file or shared object). Local symbols are hidden, as described below.
Global and weak symbols are also preemptable, that is, they may by
preempted by definitions of the same name in another component."

__kernel_datapage_offset is a STB_GLOBAL STV_DEFAULT symbol. In a
-shared link, it is considered preemptible. There are some methods
that make such symbols non-preemptible but none is used in this
context.

* -Bsymbolic
* -Bsymbolic-functions if STT_FUNC
* --dynamic-list is specified but the dynamic list does not name this symbol
* A --version-script makes the symbol local

__kernel_datapage_offset is accessed via some mechanism similar to
dlsym, so it has to be exported.

Given all the above, I chose STV_PROTECTED, which is the simplest and
least intrusive approach.

> > [1]: https://sourceware.org/bugzilla/show_bug.cgi?id=25500
> >
> > Link: https://github.com/ClangBuiltLinux/linux/issues/851
> > Signed-off-by: Fangrui Song <maskray@google.com>
>
> > ---
> >  arch/powerpc/kernel/vdso32/datapage.S | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/powerpc/kernel/vdso32/datapage.S b/arch/powerpc/kernel/vdso32/datapage.S
> > index 217bb630f8f9..2831a8676365 100644
> > --- a/arch/powerpc/kernel/vdso32/datapage.S
> > +++ b/arch/powerpc/kernel/vdso32/datapage.S
> > @@ -13,7 +13,8 @@
> >  #include <asm/vdso_datapage.h>
> >
> >       .text
> > -     .global __kernel_datapage_offset;
> > +     .global __kernel_datapage_offset
> > +     .protected      __kernel_datapage_offset
> >  __kernel_datapage_offset:
> >       .long   0
> >
> > --
> > 2.25.0.341.g760bfbb309-goog



-- 
宋方睿

      reply	other threads:[~2020-02-10 18:43 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-05  0:50 [PATCH] powerpc/vdso32: mark __kernel_datapage_offset as STV_PROTECTED Fangrui Song
2020-02-05  6:25 ` Christophe Leroy
2020-02-07  6:42   ` Nathan Chancellor
2020-06-03 23:50     ` Nick Desaulniers
2020-02-10 11:01 ` Michael Ellerman
2020-02-10 18:41   ` Fāng-ruì Sòng [this message]

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=CAFP8O3JPJvnQhAcF+DWQQGPNOj9vF-nLovzi5uSQ4zrUP1DvtQ@mail.gmail.com \
    --to=maskray@google.com \
    --cc=clang-built-linux@googlegroups.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=mpe@ellerman.id.au \
    --cc=paulus@samba.org \
    /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).