linux-kernel - Re: [PATCH] arm64: vdso: use $(LD) instead of $(CC) to link VDSO

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: <CAK7LNAT3=tbf+1KZ+VQRdiBKk9p90EqXL9D8Hk6Ucbv92CRyNQ@mail.gmail.com>
Date:   Fri, 12 Apr 2019 10:21:38 +0900
From:   Masahiro Yamada <yamada.masahiro@...ionext.com>
To:     Nick Desaulniers <ndesaulniers@...gle.com>
Cc:     Will Deacon <will.deacon@....com>,
        Linux ARM <linux-arm-kernel@...ts.infradead.org>,
        Olof Johansson <olof@...om.net>, Arnd Bergmann <arnd@...db.de>,
        Kees Cook <keescook@...omium.org>,
        Catalin Marinas <catalin.marinas@....com>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] arm64: vdso: use $(LD) instead of $(CC) to link VDSO

Hi Nick,

On Fri, Apr 12, 2019 at 3:20 AM Nick Desaulniers
<ndesaulniers@...gle.com> wrote:
>
> On Thu, Apr 11, 2019 at 2:30 AM Masahiro Yamada
> <yamada.masahiro@...ionext.com> wrote:
> >
> > We use $(LD) to link vmlinux, modules, decompressors, etc.
> >
> > VDSO is the only exceptional case where $(CC) is used as the linker
> > driver, but I do not know why we need to do so. VDSO uses a special
> > linker script, and does not link standard libraries at all.
> >
> > I changed the Makefile to use $(LD) rather than $(CC). I tested this,
> > and VDSO worked for me.
> >
> > Users will be able to use their favorite linker (e.g. lld instead of
> > of bfd) by passing LD= from the command line.
> >
> > My plan is to rewrite all VDSO Makefiles to use $(LD), then delete
> > cc-ldoption.
> >
> > Signed-off-by: Masahiro Yamada <yamada.masahiro@...ionext.com>
> > ---
> >
> >  arch/arm64/kernel/vdso/Makefile | 13 +++----------
> >  1 file changed, 3 insertions(+), 10 deletions(-)
> >
> > diff --git a/arch/arm64/kernel/vdso/Makefile b/arch/arm64/kernel/vdso/Makefile
> > index a0af6bf6c11b..744b9dbaba03 100644
> > --- a/arch/arm64/kernel/vdso/Makefile
> > +++ b/arch/arm64/kernel/vdso/Makefile
> > @@ -12,17 +12,12 @@ obj-vdso := gettimeofday.o note.o sigreturn.o
> >  targets := $(obj-vdso) vdso.so vdso.so.dbg
> >  obj-vdso := $(addprefix $(obj)/, $(obj-vdso))
> >
> > -ccflags-y := -shared -fno-common -fno-builtin
>
> Thanks for the patch; in general LGTM. Just some small questions:
>
> Looks like -shared and -fno-common are linker flags forwarded along,
> but -fno-builtin is meant for the compiler, right?

Definitely, -shared must be passed to the linker
to create a shared object.
It is listed in the linker option list:
https://gcc.gnu.org/onlinedocs/gcc-8.3.0/gcc/Link-Options.html#Link-Options



I think -fno-common is a compiler flag instead of a linker one
as far as I understand this:
https://gcc.gnu.org/onlinedocs/gcc-8.3.0/gcc/Code-Gen-Options.html#Code-Gen-Options


-fno-builtin is a compiler flag:
https://gcc.gnu.org/onlinedocs/gcc-8.3.0/gcc/C-Dialect-Options.html#C-Dialect-Options



> Do we want to keep
> that in ccflags?

All source files in arch/arm64/kernel/vdso/
are written in assembly (gettimeofday.S note.S sigreturn.S).

So, -fno-common -fno-builtin for ccflags-y
are not used.

That's why I dropped them.

If we want to keep them just in case,
it is fine with me too.


>  I'm not sure why it would have been added
> intentionally; maybe Will knows?  Maybe it's ok to drop.

They have been here since the initial VDSO support
by commit 9031fefde6f2a.

Will, do you remember why you added these flags?




> > -ccflags-y += -nostdlib -Wl,-soname=linux-vdso.so.1 \
> > -               $(call cc-ldoption, -Wl$(comma)--hash-style=sysv)
> > +ldflags-y := -shared -nostdlib -soname=linux-vdso.so.1 \
> > +               $(call ld-option, --hash-style=sysv) -n -T
>
> Forgive my ignorance, but the man page for ld makes it looks like the
> -T argument requires a subsequent argument `scriptfile`.  Is that what
> `$(real-prereqs)` was doing below? If so, is dropping it in this patch
> intentional?

I just re-used cmd_ld in scripts/Makefile.lib


It will be expanded like follows:

cmd_ld = $(LD) $(ld_flags) $(real-prereqs) -o $@

 ->

 $(LD) $(KBUILD_LDFLAGS) $(ldflags-y) $(LDFLAGS_$(@F)) $(real-prereqs) -o $@

 ->

$(LD) -shared -nostdlib -soname=linux-vdso.so.1 \
     --hash-style=sysv -n -T \
    $(obj)/vdso.lds $(obj-vdso) -o $(obj)/vdso.so.dbg



-- 
Best Regards
Masahiro Yamada