diff mbox series

kbuild: Fix building with LLVM on NixOS

Message ID 20240208012057.2754421-2-yshuiv7@gmail.com (mailing list archive)
State New, archived
Headers show
Series kbuild: Fix building with LLVM on NixOS | expand

Commit Message

Yuxuan Shui Feb. 8, 2024, 1:20 a.m. UTC
NixOS is designed to have immutable packages, and explicit dependencies.
It allows multiple different versions of the same shared library to
co-exist in its file system.

Each application built with Nix, the NixOS package manager, will have
paths to its dependency shared libraries hardcoded into its executable,
this includes the dynamic linker. To achieve this, Nix adds a
--dynamic-linker linker flag when building any application.

This isn't a problem if the kernel is built with ld.bfd, because ld.bfd
ignores the --dynamic-linker flag when the resulting binary doesn't have
a DT_NEEDED entry. However, ld.lld respects --dynamic-linker
unconditionally, which breaks linking in several cases.

This commit adds an explicit --no-dynamic-linker flag which overrides
the flag added by Nix.

Signed-off-by: Yuxuan Shui <yshuiv7@gmail.com>
---
 Makefile                      | 3 +++
 arch/x86/boot/Makefile        | 2 +-
 arch/x86/realmode/rm/Makefile | 2 +-
 3 files changed, 5 insertions(+), 2 deletions(-)

Comments

Masahiro Yamada Feb. 12, 2024, 9:29 p.m. UTC | #1
+Cc: Fangrui Song <maskray@google.com>



On Thu, Feb 8, 2024 at 10:22 AM Yuxuan Shui <yshuiv7@gmail.com> wrote:
>
> NixOS is designed to have immutable packages, and explicit dependencies.
> It allows multiple different versions of the same shared library to
> co-exist in its file system.
>
> Each application built with Nix, the NixOS package manager, will have
> paths to its dependency shared libraries hardcoded into its executable,
> this includes the dynamic linker. To achieve this, Nix adds a
> --dynamic-linker linker flag when building any application.
>
> This isn't a problem if the kernel is built with ld.bfd, because ld.bfd
> ignores the --dynamic-linker flag when the resulting binary doesn't have
> a DT_NEEDED entry. However, ld.lld respects --dynamic-linker
> unconditionally, which breaks linking in several cases.
>
> This commit adds an explicit --no-dynamic-linker flag which overrides
> the flag added by Nix.



I expect some Acks from LLVM folks (especially, from Frangrui)
if this is the right thing to do.







> Signed-off-by: Yuxuan Shui <yshuiv7@gmail.com>
> ---
>  Makefile                      | 3 +++
>  arch/x86/boot/Makefile        | 2 +-
>  arch/x86/realmode/rm/Makefile | 2 +-
>  3 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index a171eafce2a3b..10ed19caecb1b 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -531,6 +531,9 @@ RUSTFLAGS_KERNEL =
>  AFLAGS_KERNEL  =
>  LDFLAGS_vmlinux =
>
> +LDFLAGS_MODULE += --no-dynamic-linker
> +LDFLAGS_vmlinux += --no-dynamic-linker
> +
>  # Use USERINCLUDE when you must reference the UAPI directories only.
>  USERINCLUDE    := \
>                 -I$(srctree)/arch/$(SRCARCH)/include/uapi \
> diff --git a/arch/x86/boot/Makefile b/arch/x86/boot/Makefile
> index 3cece19b74732..390a4604166eb 100644
> --- a/arch/x86/boot/Makefile
> +++ b/arch/x86/boot/Makefile
> @@ -102,7 +102,7 @@ $(obj)/zoffset.h: $(obj)/compressed/vmlinux FORCE
>  AFLAGS_header.o += -I$(objtree)/$(obj)
>  $(obj)/header.o: $(obj)/zoffset.h
>
> -LDFLAGS_setup.elf      := -m elf_i386 -z noexecstack -T
> +LDFLAGS_setup.elf      := --no-dynamic-linker -m elf_i386 -z noexecstack -T
>  $(obj)/setup.elf: $(src)/setup.ld $(SETUP_OBJS) FORCE
>         $(call if_changed,ld)
>
> diff --git a/arch/x86/realmode/rm/Makefile b/arch/x86/realmode/rm/Makefile
> index f614009d3e4e2..4b42006d9ce02 100644
> --- a/arch/x86/realmode/rm/Makefile
> +++ b/arch/x86/realmode/rm/Makefile
> @@ -50,7 +50,7 @@ $(obj)/pasyms.h: $(REALMODE_OBJS) FORCE
>  targets += realmode.lds
>  $(obj)/realmode.lds: $(obj)/pasyms.h
>
> -LDFLAGS_realmode.elf := -m elf_i386 --emit-relocs -T
> +LDFLAGS_realmode.elf := --no-dynamic-linker -m elf_i386 --emit-relocs -T
>  CPPFLAGS_realmode.lds += -P -C -I$(objtree)/$(obj)
>
>  targets += realmode.elf
> --
> 2.43.0
>
Fangrui Song Feb. 12, 2024, 9:59 p.m. UTC | #2
On Mon, Feb 12, 2024 at 1:30 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> +Cc: Fangrui Song <maskray@google.com>
>
>
>
> On Thu, Feb 8, 2024 at 10:22 AM Yuxuan Shui <yshuiv7@gmail.com> wrote:
> >
> > NixOS is designed to have immutable packages, and explicit dependencies.
> > It allows multiple different versions of the same shared library to
> > co-exist in its file system.
> >
> > Each application built with Nix, the NixOS package manager, will have
> > paths to its dependency shared libraries hardcoded into its executable,
> > this includes the dynamic linker. To achieve this, Nix adds a
> > --dynamic-linker linker flag when building any application.
> >
> > This isn't a problem if the kernel is built with ld.bfd, because ld.bfd
> > ignores the --dynamic-linker flag when the resulting binary doesn't have
> > a DT_NEEDED entry. However, ld.lld respects --dynamic-linker
> > unconditionally, which breaks linking in several cases.
> >
> > This commit adds an explicit --no-dynamic-linker flag which overrides
> > the flag added by Nix.
>
>
>
> I expect some Acks from LLVM folks (especially, from Frangrui)
> if this is the right thing to do.

GNU ld seems to ignore --dynamic-linker for a position-dependent
executable (ET_EXEC) when there is no DT_NEEDED entry.
ld.lld respects --dynamic-linker in this case. Before this kernel
report, I do not know any user inconvenienced by this difference.
(mold respects --dynamic-linker as well.)
This could be helpful to test an executable with PT_INTERP but no DT_NEEDED.

I think this patch does not fix non-x86 builds.
It feels to me that NixOS should provide a linker wrapper that does
not force --dynamic-linker=.
While it's extremely uncommon (and generally not recommended), certain
programs invoke the linker directly (instead of using a compiler
driver).
Such programs would run into a problem when they make a
position-dependent executable with no dependency as well.
I don't feel that NixOS forcing --dynamic-linker= is enough
justification to change linkers.

> > Signed-off-by: Yuxuan Shui <yshuiv7@gmail.com>
> > ---
> >  Makefile                      | 3 +++
> >  arch/x86/boot/Makefile        | 2 +-
> >  arch/x86/realmode/rm/Makefile | 2 +-
> >  3 files changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/Makefile b/Makefile
> > index a171eafce2a3b..10ed19caecb1b 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -531,6 +531,9 @@ RUSTFLAGS_KERNEL =
> >  AFLAGS_KERNEL  =
> >  LDFLAGS_vmlinux =
> >
> > +LDFLAGS_MODULE += --no-dynamic-linker
> > +LDFLAGS_vmlinux += --no-dynamic-linker
> > +
> >  # Use USERINCLUDE when you must reference the UAPI directories only.
> >  USERINCLUDE    := \
> >                 -I$(srctree)/arch/$(SRCARCH)/include/uapi \
> > diff --git a/arch/x86/boot/Makefile b/arch/x86/boot/Makefile
> > index 3cece19b74732..390a4604166eb 100644
> > --- a/arch/x86/boot/Makefile
> > +++ b/arch/x86/boot/Makefile
> > @@ -102,7 +102,7 @@ $(obj)/zoffset.h: $(obj)/compressed/vmlinux FORCE
> >  AFLAGS_header.o += -I$(objtree)/$(obj)
> >  $(obj)/header.o: $(obj)/zoffset.h
> >
> > -LDFLAGS_setup.elf      := -m elf_i386 -z noexecstack -T
> > +LDFLAGS_setup.elf      := --no-dynamic-linker -m elf_i386 -z noexecstack -T
> >  $(obj)/setup.elf: $(src)/setup.ld $(SETUP_OBJS) FORCE
> >         $(call if_changed,ld)
> >
> > diff --git a/arch/x86/realmode/rm/Makefile b/arch/x86/realmode/rm/Makefile
> > index f614009d3e4e2..4b42006d9ce02 100644
> > --- a/arch/x86/realmode/rm/Makefile
> > +++ b/arch/x86/realmode/rm/Makefile
> > @@ -50,7 +50,7 @@ $(obj)/pasyms.h: $(REALMODE_OBJS) FORCE
> >  targets += realmode.lds
> >  $(obj)/realmode.lds: $(obj)/pasyms.h
> >
> > -LDFLAGS_realmode.elf := -m elf_i386 --emit-relocs -T
> > +LDFLAGS_realmode.elf := --no-dynamic-linker -m elf_i386 --emit-relocs -T
> >  CPPFLAGS_realmode.lds += -P -C -I$(objtree)/$(obj)
> >
> >  targets += realmode.elf
> > --
> > 2.43.0
> >
>
>
> --
> Best Regards
> Masahiro Yamada
>
Yuxuan Shui Feb. 12, 2024, 10:41 p.m. UTC | #3
Hi,

On Mon, Feb 12, 2024 at 9:59 PM Fangrui Song <maskray@google.com> wrote:
>
> On Mon, Feb 12, 2024 at 1:30 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
> >
> > +Cc: Fangrui Song <maskray@google.com>
> >
> >
> >
> > On Thu, Feb 8, 2024 at 10:22 AM Yuxuan Shui <yshuiv7@gmail.com> wrote:
> > >
> > > NixOS is designed to have immutable packages, and explicit dependencies.
> > > It allows multiple different versions of the same shared library to
> > > co-exist in its file system.
> > >
> > > Each application built with Nix, the NixOS package manager, will have
> > > paths to its dependency shared libraries hardcoded into its executable,
> > > this includes the dynamic linker. To achieve this, Nix adds a
> > > --dynamic-linker linker flag when building any application.
> > >
> > > This isn't a problem if the kernel is built with ld.bfd, because ld.bfd
> > > ignores the --dynamic-linker flag when the resulting binary doesn't have
> > > a DT_NEEDED entry. However, ld.lld respects --dynamic-linker
> > > unconditionally, which breaks linking in several cases.
> > >
> > > This commit adds an explicit --no-dynamic-linker flag which overrides
> > > the flag added by Nix.
> >
> >
> >
> > I expect some Acks from LLVM folks (especially, from Frangrui)
> > if this is the right thing to do.
>
> GNU ld seems to ignore --dynamic-linker for a position-dependent
> executable (ET_EXEC) when there is no DT_NEEDED entry.
> ld.lld respects --dynamic-linker in this case. Before this kernel
> report, I do not know any user inconvenienced by this difference.
> (mold respects --dynamic-linker as well.)
> This could be helpful to test an executable with PT_INTERP but no DT_NEEDED.
>
> I think this patch does not fix non-x86 builds.

This is true. I am not familiar with non-x86, and I don't have a test
environment for them.
That being said, if you or someone can point to me what other linker
flag variables I need
to change, I will try to fix them as well.

> It feels to me that NixOS should provide a linker wrapper that does
> not force --dynamic-linker=.

I actually tried that. The problem is, NixOS tries to enforce the same
build environment
for downstream packages that depend on the kernel (think kernel
modules, perf, etc.) This
is reasonable in most cases. But if I use a linker wrapper that does
_not_ have --dynamic-linker
for the kernel, then all downstream packages will inherit the same
linker wrapper too. This
breaks, for example, zfs. Because its configure script tries to build
some executable and
run them, which fails because it's using a linker wrapper without
--dynamic-linker.

> While it's extremely uncommon (and generally not recommended), certain
> programs invoke the linker directly (instead of using a compiler
> driver).
> Such programs would run into a problem when they make a
> position-dependent executable with no dependency as well.

I don't think I quite get what you are trying to say here.

> I don't feel that NixOS forcing --dynamic-linker= is enough
> justification to change linkers.

You already rejected the option of changing lld to match ld.bfd's behavior.
If you reject this kernel patch also, then what's left can only be some horrible
hacks on NixOS' side; or this means NixOS will never get a clang built kernel.
Which would be quite unfortunate.

>
> > > Signed-off-by: Yuxuan Shui <yshuiv7@gmail.com>
> > > ---
> > >  Makefile                      | 3 +++
> > >  arch/x86/boot/Makefile        | 2 +-
> > >  arch/x86/realmode/rm/Makefile | 2 +-
> > >  3 files changed, 5 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/Makefile b/Makefile
> > > index a171eafce2a3b..10ed19caecb1b 100644
> > > --- a/Makefile
> > > +++ b/Makefile
> > > @@ -531,6 +531,9 @@ RUSTFLAGS_KERNEL =
> > >  AFLAGS_KERNEL  =
> > >  LDFLAGS_vmlinux =
> > >
> > > +LDFLAGS_MODULE += --no-dynamic-linker
> > > +LDFLAGS_vmlinux += --no-dynamic-linker
> > > +
> > >  # Use USERINCLUDE when you must reference the UAPI directories only.
> > >  USERINCLUDE    := \
> > >                 -I$(srctree)/arch/$(SRCARCH)/include/uapi \
> > > diff --git a/arch/x86/boot/Makefile b/arch/x86/boot/Makefile
> > > index 3cece19b74732..390a4604166eb 100644
> > > --- a/arch/x86/boot/Makefile
> > > +++ b/arch/x86/boot/Makefile
> > > @@ -102,7 +102,7 @@ $(obj)/zoffset.h: $(obj)/compressed/vmlinux FORCE
> > >  AFLAGS_header.o += -I$(objtree)/$(obj)
> > >  $(obj)/header.o: $(obj)/zoffset.h
> > >
> > > -LDFLAGS_setup.elf      := -m elf_i386 -z noexecstack -T
> > > +LDFLAGS_setup.elf      := --no-dynamic-linker -m elf_i386 -z noexecstack -T
> > >  $(obj)/setup.elf: $(src)/setup.ld $(SETUP_OBJS) FORCE
> > >         $(call if_changed,ld)
> > >
> > > diff --git a/arch/x86/realmode/rm/Makefile b/arch/x86/realmode/rm/Makefile
> > > index f614009d3e4e2..4b42006d9ce02 100644
> > > --- a/arch/x86/realmode/rm/Makefile
> > > +++ b/arch/x86/realmode/rm/Makefile
> > > @@ -50,7 +50,7 @@ $(obj)/pasyms.h: $(REALMODE_OBJS) FORCE
> > >  targets += realmode.lds
> > >  $(obj)/realmode.lds: $(obj)/pasyms.h
> > >
> > > -LDFLAGS_realmode.elf := -m elf_i386 --emit-relocs -T
> > > +LDFLAGS_realmode.elf := --no-dynamic-linker -m elf_i386 --emit-relocs -T
> > >  CPPFLAGS_realmode.lds += -P -C -I$(objtree)/$(obj)
> > >
> > >  targets += realmode.elf
> > > --
> > > 2.43.0
> > >
> >
> >
> > --
> > Best Regards
> > Masahiro Yamada
> >
>
>
> --
> 宋方睿
Tristan ross Sept. 25, 2024, 2:39 a.m. UTC | #4
LLVM maintainer for Nix here, I do agree that our wrapper is causing issues here. However, we're looking to fix this by mimicking GNU ld's behavior for checking --dynamic-linker. We're also going to look into getting LLVM lld to support the same behavior in case others have this issue. We might not be the only ones who could be affected so adding this behavior into LLVM definitely will be useful. I tested aarch64 building of the kernel and it works after we fixed our handling of hardening options. However, aside from aarch64 and x86_64, I am not sure what other things could be failing. I hope this clarifies things.

Thank you,
Tristan Ross

(Sorry if I sent two emails, I forgot to reply all last time. First time using the LKML)


 ---- On Mon, 12 Feb 2024 13:59:13 -0800  Fangrui Song  wrote --- 
 > On Mon, Feb 12, 2024 at 1:30 PM Masahiro Yamada masahiroy@kernel.org> wrote:
 > >
 > > +Cc: Fangrui Song maskray@google.com>
 > >
 > >
 > >
 > > On Thu, Feb 8, 2024 at 10:22 AM Yuxuan Shui yshuiv7@gmail.com> wrote:
 > > >
 > > > NixOS is designed to have immutable packages, and explicit dependencies.
 > > > It allows multiple different versions of the same shared library to
 > > > co-exist in its file system.
 > > >
 > > > Each application built with Nix, the NixOS package manager, will have
 > > > paths to its dependency shared libraries hardcoded into its executable,
 > > > this includes the dynamic linker. To achieve this, Nix adds a
 > > > --dynamic-linker linker flag when building any application.
 > > >
 > > > This isn't a problem if the kernel is built with ld.bfd, because ld.bfd
 > > > ignores the --dynamic-linker flag when the resulting binary doesn't have
 > > > a DT_NEEDED entry. However, ld.lld respects --dynamic-linker
 > > > unconditionally, which breaks linking in several cases.
 > > >
 > > > This commit adds an explicit --no-dynamic-linker flag which overrides
 > > > the flag added by Nix.
 > >
 > >
 > >
 > > I expect some Acks from LLVM folks (especially, from Frangrui)
 > > if this is the right thing to do.
 > 
 > GNU ld seems to ignore --dynamic-linker for a position-dependent
 > executable (ET_EXEC) when there is no DT_NEEDED entry.
 > ld.lld respects --dynamic-linker in this case. Before this kernel
 > report, I do not know any user inconvenienced by this difference.
 > (mold respects --dynamic-linker as well.)
 > This could be helpful to test an executable with PT_INTERP but no DT_NEEDED.
 > 
 > I think this patch does not fix non-x86 builds.
 > It feels to me that NixOS should provide a linker wrapper that does
 > not force --dynamic-linker=.
 > While it's extremely uncommon (and generally not recommended), certain
 > programs invoke the linker directly (instead of using a compiler
 > driver).
 > Such programs would run into a problem when they make a
 > position-dependent executable with no dependency as well.
 > I don't feel that NixOS forcing --dynamic-linker= is enough
 > justification to change linkers.
 > 
 > > > Signed-off-by: Yuxuan Shui yshuiv7@gmail.com>
 > > > ---
 > > >  Makefile                      | 3 +++
 > > >  arch/x86/boot/Makefile        | 2 +-
 > > >  arch/x86/realmode/rm/Makefile | 2 +-
 > > >  3 files changed, 5 insertions(+), 2 deletions(-)
 > > >
 > > > diff --git a/Makefile b/Makefile
 > > > index a171eafce2a3b..10ed19caecb1b 100644
 > > > --- a/Makefile
 > > > +++ b/Makefile
 > > > @@ -531,6 +531,9 @@ RUSTFLAGS_KERNEL =
 > > >  AFLAGS_KERNEL  =
 > > >  LDFLAGS_vmlinux =
 > > >
 > > > +LDFLAGS_MODULE += --no-dynamic-linker
 > > > +LDFLAGS_vmlinux += --no-dynamic-linker
 > > > +
 > > >  # Use USERINCLUDE when you must reference the UAPI directories only.
 > > >  USERINCLUDE    := \
 > > >                 -I$(srctree)/arch/$(SRCARCH)/include/uapi \
 > > > diff --git a/arch/x86/boot/Makefile b/arch/x86/boot/Makefile
 > > > index 3cece19b74732..390a4604166eb 100644
 > > > --- a/arch/x86/boot/Makefile
 > > > +++ b/arch/x86/boot/Makefile
 > > > @@ -102,7 +102,7 @@ $(obj)/zoffset.h: $(obj)/compressed/vmlinux FORCE
 > > >  AFLAGS_header.o += -I$(objtree)/$(obj)
 > > >  $(obj)/header.o: $(obj)/zoffset.h
 > > >
 > > > -LDFLAGS_setup.elf      := -m elf_i386 -z noexecstack -T
 > > > +LDFLAGS_setup.elf      := --no-dynamic-linker -m elf_i386 -z noexecstack -T
 > > >  $(obj)/setup.elf: $(src)/setup.ld $(SETUP_OBJS) FORCE
 > > >         $(call if_changed,ld)
 > > >
 > > > diff --git a/arch/x86/realmode/rm/Makefile b/arch/x86/realmode/rm/Makefile
 > > > index f614009d3e4e2..4b42006d9ce02 100644
 > > > --- a/arch/x86/realmode/rm/Makefile
 > > > +++ b/arch/x86/realmode/rm/Makefile
 > > > @@ -50,7 +50,7 @@ $(obj)/pasyms.h: $(REALMODE_OBJS) FORCE
 > > >  targets += realmode.lds
 > > >  $(obj)/realmode.lds: $(obj)/pasyms.h
 > > >
 > > > -LDFLAGS_realmode.elf := -m elf_i386 --emit-relocs -T
 > > > +LDFLAGS_realmode.elf := --no-dynamic-linker -m elf_i386 --emit-relocs -T
 > > >  CPPFLAGS_realmode.lds += -P -C -I$(objtree)/$(obj)
 > > >
 > > >  targets += realmode.elf
 > > > --
 > > > 2.43.0
 > > >
 > >
 > >
 > > --
 > > Best Regards
 > > Masahiro Yamada
 > >
 > 
 > 
 > -- 
 > 宋方睿
 > 
 > 
 > 
Tristan ross
CEO & Founder of Midstall Software
diff mbox series

Patch

diff --git a/Makefile b/Makefile
index a171eafce2a3b..10ed19caecb1b 100644
--- a/Makefile
+++ b/Makefile
@@ -531,6 +531,9 @@  RUSTFLAGS_KERNEL =
 AFLAGS_KERNEL	=
 LDFLAGS_vmlinux =
 
+LDFLAGS_MODULE += --no-dynamic-linker
+LDFLAGS_vmlinux += --no-dynamic-linker
+
 # Use USERINCLUDE when you must reference the UAPI directories only.
 USERINCLUDE    := \
 		-I$(srctree)/arch/$(SRCARCH)/include/uapi \
diff --git a/arch/x86/boot/Makefile b/arch/x86/boot/Makefile
index 3cece19b74732..390a4604166eb 100644
--- a/arch/x86/boot/Makefile
+++ b/arch/x86/boot/Makefile
@@ -102,7 +102,7 @@  $(obj)/zoffset.h: $(obj)/compressed/vmlinux FORCE
 AFLAGS_header.o += -I$(objtree)/$(obj)
 $(obj)/header.o: $(obj)/zoffset.h
 
-LDFLAGS_setup.elf	:= -m elf_i386 -z noexecstack -T
+LDFLAGS_setup.elf	:= --no-dynamic-linker -m elf_i386 -z noexecstack -T
 $(obj)/setup.elf: $(src)/setup.ld $(SETUP_OBJS) FORCE
 	$(call if_changed,ld)
 
diff --git a/arch/x86/realmode/rm/Makefile b/arch/x86/realmode/rm/Makefile
index f614009d3e4e2..4b42006d9ce02 100644
--- a/arch/x86/realmode/rm/Makefile
+++ b/arch/x86/realmode/rm/Makefile
@@ -50,7 +50,7 @@  $(obj)/pasyms.h: $(REALMODE_OBJS) FORCE
 targets += realmode.lds
 $(obj)/realmode.lds: $(obj)/pasyms.h
 
-LDFLAGS_realmode.elf := -m elf_i386 --emit-relocs -T
+LDFLAGS_realmode.elf := --no-dynamic-linker -m elf_i386 --emit-relocs -T
 CPPFLAGS_realmode.lds += -P -C -I$(objtree)/$(obj)
 
 targets += realmode.elf