2019-12-09 23:04:09

by Brendan Higgins

[permalink] [raw]
Subject: [PATCH v1] uml: remove support for CONFIG_STATIC_LINK

CONFIG_STATIC_LINK appears to have been broken since before v4.20. It
doesn't play nice with CONFIG_UML_NET_VECTOR=y:

/usr/bin/ld: arch/um/drivers/vector_user.o: in function
`user_init_socket_fds': vector_user.c:(.text+0x430): warning: Using
'getaddrinfo' in statically linked applications requires at runtime the
shared libraries from the glibc version used for linking

And it seems to break the ptrace check:

Checking that ptrace can change system call numbers...check_ptrace :
child exited with exitcode 6, while expecting 0; status 0x67f
[1] 126822 abort ./linux mem=256M

(Apparently, a patch was recently discussed that fixes this - around
v5.5-rc1[1] - but the fact that this was broken for over a year
remains.)

According to Anton, PCAP throws even more warnings, and the resulting
binary isn't really even static anyway, so there is really no point in
keeping this config around[2].

Signed-off-by: Brendan Higgins <[email protected]>
Link[1]: https://www.spinics.net/lists/stable/msg346933.html
Link[2]: https://lkml.org/lkml/2019/12/6/46
---
arch/um/Kconfig | 23 +----
arch/um/Makefile | 3 +-
arch/um/kernel/dyn.lds.S | 170 ----------------------------------
arch/um/kernel/uml.lds.S | 115 -----------------------
arch/um/kernel/vmlinux.lds.S | 175 ++++++++++++++++++++++++++++++++++-
5 files changed, 172 insertions(+), 314 deletions(-)
delete mode 100644 arch/um/kernel/dyn.lds.S
delete mode 100644 arch/um/kernel/uml.lds.S

diff --git a/arch/um/Kconfig b/arch/um/Kconfig
index 2a6d04fcb3e91..00927fb7ce67a 100644
--- a/arch/um/Kconfig
+++ b/arch/um/Kconfig
@@ -19,6 +19,7 @@ config UML
select GENERIC_CLOCKEVENTS
select HAVE_GCC_PLUGINS
select TTY # Needed for line.c
+ select MODULE_REL_CRCS if MODVERSIONS

config MMU
bool
@@ -61,28 +62,6 @@ config NR_CPUS

source "arch/$(HEADER_ARCH)/um/Kconfig"

-config STATIC_LINK
- bool "Force a static link"
- default n
- help
- This option gives you the ability to force a static link of UML.
- Normally, UML is linked as a shared binary. This is inconvenient for
- use in a chroot jail. So, if you intend to run UML inside a chroot,
- you probably want to say Y here.
- Additionally, this option enables using higher memory spaces (up to
- 2.75G) for UML.
-
-config LD_SCRIPT_STATIC
- bool
- default y
- depends on STATIC_LINK
-
-config LD_SCRIPT_DYN
- bool
- default y
- depends on !LD_SCRIPT_STATIC
- select MODULE_REL_CRCS if MODVERSIONS
-
config HOSTFS
tristate "Host filesystem"
help
diff --git a/arch/um/Makefile b/arch/um/Makefile
index d2daa206872da..ec8af28daf051 100644
--- a/arch/um/Makefile
+++ b/arch/um/Makefile
@@ -117,8 +117,7 @@ archheaders:
archprepare:
$(Q)$(MAKE) $(build)=$(HOST_DIR)/um include/generated/user_constants.h

-LINK-$(CONFIG_LD_SCRIPT_STATIC) += -static
-LINK-$(CONFIG_LD_SCRIPT_DYN) += -Wl,-rpath,/lib $(call cc-option, -no-pie)
+LINK-y += -Wl,-rpath,/lib $(call cc-option, -no-pie)

CFLAGS_NO_HARDENING := $(call cc-option, -fno-PIC,) $(call cc-option, -fno-pic,) \
$(call cc-option, -fno-stack-protector,) \
diff --git a/arch/um/kernel/dyn.lds.S b/arch/um/kernel/dyn.lds.S
deleted file mode 100644
index c69d69ee96beb..0000000000000
--- a/arch/um/kernel/dyn.lds.S
+++ /dev/null
@@ -1,170 +0,0 @@
-#include <asm/vmlinux.lds.h>
-#include <asm/page.h>
-
-OUTPUT_FORMAT(ELF_FORMAT)
-OUTPUT_ARCH(ELF_ARCH)
-ENTRY(_start)
-jiffies = jiffies_64;
-
-SECTIONS
-{
- PROVIDE (__executable_start = START);
- . = START + SIZEOF_HEADERS;
- .interp : { *(.interp) }
- __binary_start = .;
- . = ALIGN(4096); /* Init code and data */
- _text = .;
- INIT_TEXT_SECTION(PAGE_SIZE)
-
- . = ALIGN(PAGE_SIZE);
-
- /* Read-only sections, merged into text segment: */
- .hash : { *(.hash) }
- .gnu.hash : { *(.gnu.hash) }
- .dynsym : { *(.dynsym) }
- .dynstr : { *(.dynstr) }
- .gnu.version : { *(.gnu.version) }
- .gnu.version_d : { *(.gnu.version_d) }
- .gnu.version_r : { *(.gnu.version_r) }
- .rel.init : { *(.rel.init) }
- .rela.init : { *(.rela.init) }
- .rel.text : { *(.rel.text .rel.text.* .rel.gnu.linkonce.t.*) }
- .rela.text : { *(.rela.text .rela.text.* .rela.gnu.linkonce.t.*) }
- .rel.fini : { *(.rel.fini) }
- .rela.fini : { *(.rela.fini) }
- .rel.rodata : { *(.rel.rodata .rel.rodata.* .rel.gnu.linkonce.r.*) }
- .rela.rodata : { *(.rela.rodata .rela.rodata.* .rela.gnu.linkonce.r.*) }
- .rel.data : { *(.rel.data .rel.data.* .rel.gnu.linkonce.d.*) }
- .rela.data : { *(.rela.data .rela.data.* .rela.gnu.linkonce.d.*) }
- .rel.tdata : { *(.rel.tdata .rel.tdata.* .rel.gnu.linkonce.td.*) }
- .rela.tdata : { *(.rela.tdata .rela.tdata.* .rela.gnu.linkonce.td.*) }
- .rel.tbss : { *(.rel.tbss .rel.tbss.* .rel.gnu.linkonce.tb.*) }
- .rela.tbss : { *(.rela.tbss .rela.tbss.* .rela.gnu.linkonce.tb.*) }
- .rel.ctors : { *(.rel.ctors) }
- .rela.ctors : { *(.rela.ctors) }
- .rel.dtors : { *(.rel.dtors) }
- .rela.dtors : { *(.rela.dtors) }
- .rel.got : { *(.rel.got) }
- .rela.got : { *(.rela.got) }
- .rel.bss : { *(.rel.bss .rel.bss.* .rel.gnu.linkonce.b.*) }
- .rela.bss : { *(.rela.bss .rela.bss.* .rela.gnu.linkonce.b.*) }
- .rel.plt : {
- *(.rel.plt)
- PROVIDE_HIDDEN(__rel_iplt_start = .);
- *(.rel.iplt)
- PROVIDE_HIDDEN(__rel_iplt_end = .);
- }
- .rela.plt : {
- *(.rela.plt)
- PROVIDE_HIDDEN(__rela_iplt_start = .);
- *(.rela.iplt)
- PROVIDE_HIDDEN(__rela_iplt_end = .);
- }
- .init : {
- KEEP (*(.init))
- } =0x90909090
- .plt : { *(.plt) }
- .text : {
- _stext = .;
- TEXT_TEXT
- SCHED_TEXT
- CPUIDLE_TEXT
- LOCK_TEXT
- IRQENTRY_TEXT
- SOFTIRQENTRY_TEXT
- *(.fixup)
- *(.stub .text.* .gnu.linkonce.t.*)
- /* .gnu.warning sections are handled specially by elf32.em. */
- *(.gnu.warning)
-
- . = ALIGN(PAGE_SIZE);
- } =0x90909090
- . = ALIGN(PAGE_SIZE);
- .syscall_stub : {
- __syscall_stub_start = .;
- *(.__syscall_stub*)
- __syscall_stub_end = .;
- }
- .fini : {
- KEEP (*(.fini))
- } =0x90909090
-
- .kstrtab : { *(.kstrtab) }
-
- #include <asm/common.lds.S>
-
- __init_begin = .;
- init.data : { INIT_DATA }
- __init_end = .;
-
- /* Ensure the __preinit_array_start label is properly aligned. We
- could instead move the label definition inside the section, but
- the linker would then create the section even if it turns out to
- be empty, which isn't pretty. */
- . = ALIGN(32 / 8);
- .preinit_array : { *(.preinit_array) }
- .fini_array : { *(.fini_array) }
- .data : {
- INIT_TASK_DATA(KERNEL_STACK_SIZE)
- . = ALIGN(KERNEL_STACK_SIZE);
- *(.data..init_irqstack)
- DATA_DATA
- *(.data.* .gnu.linkonce.d.*)
- SORT(CONSTRUCTORS)
- }
- .data1 : { *(.data1) }
- .tdata : { *(.tdata .tdata.* .gnu.linkonce.td.*) }
- .tbss : { *(.tbss .tbss.* .gnu.linkonce.tb.*) *(.tcommon) }
- .eh_frame : { KEEP (*(.eh_frame)) }
- .gcc_except_table : { *(.gcc_except_table) }
- .dynamic : { *(.dynamic) }
- .ctors : {
- /* gcc uses crtbegin.o to find the start of
- the constructors, so we make sure it is
- first. Because this is a wildcard, it
- doesn't matter if the user does not
- actually link against crtbegin.o; the
- linker won't look for a file to match a
- wildcard. The wildcard also means that it
- doesn't matter which directory crtbegin.o
- is in. */
- KEEP (*crtbegin.o(.ctors))
- /* We don't want to include the .ctor section from
- from the crtend.o file until after the sorted ctors.
- The .ctor section from the crtend file contains the
- end of ctors marker and it must be last */
- KEEP (*(EXCLUDE_FILE (*crtend.o ) .ctors))
- KEEP (*(SORT(.ctors.*)))
- KEEP (*(.ctors))
- }
- .dtors : {
- KEEP (*crtbegin.o(.dtors))
- KEEP (*(EXCLUDE_FILE (*crtend.o ) .dtors))
- KEEP (*(SORT(.dtors.*)))
- KEEP (*(.dtors))
- }
- .jcr : { KEEP (*(.jcr)) }
- .got : { *(.got.plt) *(.got) }
- _edata = .;
- PROVIDE (edata = .);
- .bss : {
- __bss_start = .;
- *(.dynbss)
- *(.bss .bss.* .gnu.linkonce.b.*)
- *(COMMON)
- /* Align here to ensure that the .bss section occupies space up to
- _end. Align after .bss to ensure correct alignment even if the
- .bss section disappears because there are no input sections. */
- . = ALIGN(32 / 8);
- . = ALIGN(32 / 8);
- }
- __bss_stop = .;
- _end = .;
- PROVIDE (end = .);
-
- STABS_DEBUG
-
- DWARF_DEBUG
-
- DISCARDS
-}
diff --git a/arch/um/kernel/uml.lds.S b/arch/um/kernel/uml.lds.S
deleted file mode 100644
index 9f21443be2c9e..0000000000000
--- a/arch/um/kernel/uml.lds.S
+++ /dev/null
@@ -1,115 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-#include <asm/vmlinux.lds.h>
-#include <asm/page.h>
-
-OUTPUT_FORMAT(ELF_FORMAT)
-OUTPUT_ARCH(ELF_ARCH)
-ENTRY(_start)
-jiffies = jiffies_64;
-
-SECTIONS
-{
- /* This must contain the right address - not quite the default ELF one.*/
- PROVIDE (__executable_start = START);
- /* Static binaries stick stuff here, like the sigreturn trampoline,
- * invisibly to objdump. So, just make __binary_start equal to the very
- * beginning of the executable, and if there are unmapped pages after this,
- * they are forever unusable.
- */
- __binary_start = START;
-
- . = START + SIZEOF_HEADERS;
-
- _text = .;
- INIT_TEXT_SECTION(0)
- . = ALIGN(PAGE_SIZE);
-
- .text :
- {
- _stext = .;
- TEXT_TEXT
- SCHED_TEXT
- CPUIDLE_TEXT
- LOCK_TEXT
- IRQENTRY_TEXT
- SOFTIRQENTRY_TEXT
- *(.fixup)
- /* .gnu.warning sections are handled specially by elf32.em. */
- *(.gnu.warning)
- *(.gnu.linkonce.t*)
- }
-
- . = ALIGN(PAGE_SIZE);
- .syscall_stub : {
- __syscall_stub_start = .;
- *(.__syscall_stub*)
- __syscall_stub_end = .;
- }
-
- /*
- * These are needed even in a static link, even if they wind up being empty.
- * Newer glibc needs these __rel{,a}_iplt_{start,end} symbols.
- */
- .rel.plt : {
- *(.rel.plt)
- PROVIDE_HIDDEN(__rel_iplt_start = .);
- *(.rel.iplt)
- PROVIDE_HIDDEN(__rel_iplt_end = .);
- }
- .rela.plt : {
- *(.rela.plt)
- PROVIDE_HIDDEN(__rela_iplt_start = .);
- *(.rela.iplt)
- PROVIDE_HIDDEN(__rela_iplt_end = .);
- }
-
- #include <asm/common.lds.S>
-
- __init_begin = .;
- init.data : { INIT_DATA }
- __init_end = .;
-
- .data :
- {
- INIT_TASK_DATA(KERNEL_STACK_SIZE)
- . = ALIGN(KERNEL_STACK_SIZE);
- *(.data..init_irqstack)
- DATA_DATA
- *(.gnu.linkonce.d*)
- CONSTRUCTORS
- }
- .data1 : { *(.data1) }
- .ctors :
- {
- *(.ctors)
- }
- .dtors :
- {
- *(.dtors)
- }
-
- .got : { *(.got.plt) *(.got) }
- .dynamic : { *(.dynamic) }
- .tdata : { *(.tdata .tdata.* .gnu.linkonce.td.*) }
- .tbss : { *(.tbss .tbss.* .gnu.linkonce.tb.*) *(.tcommon) }
- /* We want the small data sections together, so single-instruction offsets
- can access them all, and initialized data all before uninitialized, so
- we can shorten the on-disk segment size. */
- .sdata : { *(.sdata) }
- _edata = .;
- PROVIDE (edata = .);
- . = ALIGN(PAGE_SIZE);
- __bss_start = .;
- PROVIDE(_bss_start = .);
- SBSS(0)
- BSS(0)
- __bss_stop = .;
- _end = .;
- PROVIDE (end = .);
-
- STABS_DEBUG
-
- DWARF_DEBUG
-
- DISCARDS
-}
diff --git a/arch/um/kernel/vmlinux.lds.S b/arch/um/kernel/vmlinux.lds.S
index 16e49bfa2b426..f4b6114e54d62 100644
--- a/arch/um/kernel/vmlinux.lds.S
+++ b/arch/um/kernel/vmlinux.lds.S
@@ -1,8 +1,173 @@

KERNEL_STACK_SIZE = 4096 * (1 << CONFIG_KERNEL_STACK_ORDER);

-#ifdef CONFIG_LD_SCRIPT_STATIC
-#include "uml.lds.S"
-#else
-#include "dyn.lds.S"
-#endif
+#include <asm/vmlinux.lds.h>
+#include <asm/page.h>
+
+OUTPUT_FORMAT(ELF_FORMAT)
+OUTPUT_ARCH(ELF_ARCH)
+ENTRY(_start)
+jiffies = jiffies_64;
+
+SECTIONS
+{
+ PROVIDE (__executable_start = START);
+ . = START + SIZEOF_HEADERS;
+ .interp : { *(.interp) }
+ __binary_start = .;
+ . = ALIGN(4096); /* Init code and data */
+ _text = .;
+ INIT_TEXT_SECTION(PAGE_SIZE)
+
+ . = ALIGN(PAGE_SIZE);
+
+ /* Read-only sections, merged into text segment: */
+ .hash : { *(.hash) }
+ .gnu.hash : { *(.gnu.hash) }
+ .dynsym : { *(.dynsym) }
+ .dynstr : { *(.dynstr) }
+ .gnu.version : { *(.gnu.version) }
+ .gnu.version_d : { *(.gnu.version_d) }
+ .gnu.version_r : { *(.gnu.version_r) }
+ .rel.init : { *(.rel.init) }
+ .rela.init : { *(.rela.init) }
+ .rel.text : { *(.rel.text .rel.text.* .rel.gnu.linkonce.t.*) }
+ .rela.text : { *(.rela.text .rela.text.* .rela.gnu.linkonce.t.*) }
+ .rel.fini : { *(.rel.fini) }
+ .rela.fini : { *(.rela.fini) }
+ .rel.rodata : { *(.rel.rodata .rel.rodata.* .rel.gnu.linkonce.r.*) }
+ .rela.rodata : { *(.rela.rodata .rela.rodata.* .rela.gnu.linkonce.r.*) }
+ .rel.data : { *(.rel.data .rel.data.* .rel.gnu.linkonce.d.*) }
+ .rela.data : { *(.rela.data .rela.data.* .rela.gnu.linkonce.d.*) }
+ .rel.tdata : { *(.rel.tdata .rel.tdata.* .rel.gnu.linkonce.td.*) }
+ .rela.tdata : { *(.rela.tdata .rela.tdata.* .rela.gnu.linkonce.td.*) }
+ .rel.tbss : { *(.rel.tbss .rel.tbss.* .rel.gnu.linkonce.tb.*) }
+ .rela.tbss : { *(.rela.tbss .rela.tbss.* .rela.gnu.linkonce.tb.*) }
+ .rel.ctors : { *(.rel.ctors) }
+ .rela.ctors : { *(.rela.ctors) }
+ .rel.dtors : { *(.rel.dtors) }
+ .rela.dtors : { *(.rela.dtors) }
+ .rel.got : { *(.rel.got) }
+ .rela.got : { *(.rela.got) }
+ .rel.bss : { *(.rel.bss .rel.bss.* .rel.gnu.linkonce.b.*) }
+ .rela.bss : { *(.rela.bss .rela.bss.* .rela.gnu.linkonce.b.*) }
+ .rel.plt : {
+ *(.rel.plt)
+ PROVIDE_HIDDEN(__rel_iplt_start = .);
+ *(.rel.iplt)
+ PROVIDE_HIDDEN(__rel_iplt_end = .);
+ }
+ .rela.plt : {
+ *(.rela.plt)
+ PROVIDE_HIDDEN(__rela_iplt_start = .);
+ *(.rela.iplt)
+ PROVIDE_HIDDEN(__rela_iplt_end = .);
+ }
+ .init : {
+ KEEP (*(.init))
+ } =0x90909090
+ .plt : { *(.plt) }
+ .text : {
+ _stext = .;
+ TEXT_TEXT
+ SCHED_TEXT
+ CPUIDLE_TEXT
+ LOCK_TEXT
+ IRQENTRY_TEXT
+ SOFTIRQENTRY_TEXT
+ *(.fixup)
+ *(.stub .text.* .gnu.linkonce.t.*)
+ /* .gnu.warning sections are handled specially by elf32.em. */
+ *(.gnu.warning)
+
+ . = ALIGN(PAGE_SIZE);
+ } =0x90909090
+ . = ALIGN(PAGE_SIZE);
+ .syscall_stub : {
+ __syscall_stub_start = .;
+ *(.__syscall_stub*)
+ __syscall_stub_end = .;
+ }
+ .fini : {
+ KEEP (*(.fini))
+ } =0x90909090
+
+ .kstrtab : { *(.kstrtab) }
+
+ #include <asm/common.lds.S>
+
+ __init_begin = .;
+ init.data : { INIT_DATA }
+ __init_end = .;
+
+ /* Ensure the __preinit_array_start label is properly aligned. We
+ could instead move the label definition inside the section, but
+ the linker would then create the section even if it turns out to
+ be empty, which isn't pretty. */
+ . = ALIGN(32 / 8);
+ .preinit_array : { *(.preinit_array) }
+ .fini_array : { *(.fini_array) }
+ .data : {
+ INIT_TASK_DATA(KERNEL_STACK_SIZE)
+ . = ALIGN(KERNEL_STACK_SIZE);
+ *(.data..init_irqstack)
+ DATA_DATA
+ *(.data.* .gnu.linkonce.d.*)
+ SORT(CONSTRUCTORS)
+ }
+ .data1 : { *(.data1) }
+ .tdata : { *(.tdata .tdata.* .gnu.linkonce.td.*) }
+ .tbss : { *(.tbss .tbss.* .gnu.linkonce.tb.*) *(.tcommon) }
+ .eh_frame : { KEEP (*(.eh_frame)) }
+ .gcc_except_table : { *(.gcc_except_table) }
+ .dynamic : { *(.dynamic) }
+ .ctors : {
+ /* gcc uses crtbegin.o to find the start of
+ the constructors, so we make sure it is
+ first. Because this is a wildcard, it
+ doesn't matter if the user does not
+ actually link against crtbegin.o; the
+ linker won't look for a file to match a
+ wildcard. The wildcard also means that it
+ doesn't matter which directory crtbegin.o
+ is in. */
+ KEEP (*crtbegin.o(.ctors))
+ /* We don't want to include the .ctor section from
+ from the crtend.o file until after the sorted ctors.
+ The .ctor section from the crtend file contains the
+ end of ctors marker and it must be last */
+ KEEP (*(EXCLUDE_FILE (*crtend.o ) .ctors))
+ KEEP (*(SORT(.ctors.*)))
+ KEEP (*(.ctors))
+ }
+ .dtors : {
+ KEEP (*crtbegin.o(.dtors))
+ KEEP (*(EXCLUDE_FILE (*crtend.o ) .dtors))
+ KEEP (*(SORT(.dtors.*)))
+ KEEP (*(.dtors))
+ }
+ .jcr : { KEEP (*(.jcr)) }
+ .got : { *(.got.plt) *(.got) }
+ _edata = .;
+ PROVIDE (edata = .);
+ .bss : {
+ __bss_start = .;
+ *(.dynbss)
+ *(.bss .bss.* .gnu.linkonce.b.*)
+ *(COMMON)
+ /* Align here to ensure that the .bss section occupies space up to
+ _end. Align after .bss to ensure correct alignment even if the
+ .bss section disappears because there are no input sections. */
+ . = ALIGN(32 / 8);
+ . = ALIGN(32 / 8);
+ }
+ __bss_stop = .;
+ _end = .;
+ PROVIDE (end = .);
+
+ STABS_DEBUG
+
+ DWARF_DEBUG
+
+ DISCARDS
+}
--
2.24.0.393.g34dc348eaf-goog


2019-12-09 23:16:37

by Richard Weinberger

[permalink] [raw]
Subject: Re: [PATCH v1] uml: remove support for CONFIG_STATIC_LINK

----- Ursprüngliche Mail -----
> Von: "Brendan Higgins" <[email protected]>
> An: "Jeff Dike" <[email protected]>, "richard" <[email protected]>, "anton ivanov" <[email protected]>
> CC: "Johannes Berg" <[email protected]>, "linux-um" <[email protected]>, "linux-kernel"
> <[email protected]>, [email protected], "Brendan Higgins" <[email protected]>
> Gesendet: Dienstag, 10. Dezember 2019 00:02:48
> Betreff: [PATCH v1] uml: remove support for CONFIG_STATIC_LINK

> CONFIG_STATIC_LINK appears to have been broken since before v4.20. It
> doesn't play nice with CONFIG_UML_NET_VECTOR=y:
>
> /usr/bin/ld: arch/um/drivers/vector_user.o: in function
> `user_init_socket_fds': vector_user.c:(.text+0x430): warning: Using
> 'getaddrinfo' in statically linked applications requires at runtime the
> shared libraries from the glibc version used for linking

This is nothing serious.

> And it seems to break the ptrace check:
>
> Checking that ptrace can change system call numbers...check_ptrace :
> child exited with exitcode 6, while expecting 0; status 0x67f
> [1] 126822 abort ./linux mem=256M

Didn't we fix that already?

> (Apparently, a patch was recently discussed that fixes this - around
> v5.5-rc1[1] - but the fact that this was broken for over a year
> remains.)
>
> According to Anton, PCAP throws even more warnings, and the resulting
> binary isn't really even static anyway, so there is really no point in
> keeping this config around[2].

What?
Anton, please explain. Why is it not static when build with CONFIG_STATIC_LINK?

Thanks,
//richard

2019-12-10 07:21:41

by Anton Ivanov

[permalink] [raw]
Subject: Re: [PATCH v1] uml: remove support for CONFIG_STATIC_LINK

On 09/12/2019 23:15, Richard Weinberger wrote:
> ----- Ursprüngliche Mail -----
>> Von: "Brendan Higgins" <[email protected]>
>> An: "Jeff Dike" <[email protected]>, "richard" <[email protected]>, "anton ivanov" <[email protected]>
>> CC: "Johannes Berg" <[email protected]>, "linux-um" <[email protected]>, "linux-kernel"
>> <[email protected]>, [email protected], "Brendan Higgins" <[email protected]>
>> Gesendet: Dienstag, 10. Dezember 2019 00:02:48
>> Betreff: [PATCH v1] uml: remove support for CONFIG_STATIC_LINK
>
>> CONFIG_STATIC_LINK appears to have been broken since before v4.20. It
>> doesn't play nice with CONFIG_UML_NET_VECTOR=y:
>>
>> /usr/bin/ld: arch/um/drivers/vector_user.o: in function
>> `user_init_socket_fds': vector_user.c:(.text+0x430): warning: Using
>> 'getaddrinfo' in statically linked applications requires at runtime the
>> shared libraries from the glibc version used for linking
>
> This is nothing serious.
>
>> And it seems to break the ptrace check:
>>
>> Checking that ptrace can change system call numbers...check_ptrace :
>> child exited with exitcode 6, while expecting 0; status 0x67f
>> [1] 126822 abort ./linux mem=256M
>
> Didn't we fix that already?

Yes we did - I commented on this.

>
>> (Apparently, a patch was recently discussed that fixes this - around
>> v5.5-rc1[1] - but the fact that this was broken for over a year
>> remains.)
>>
>> According to Anton, PCAP throws even more warnings, and the resulting
>> binary isn't really even static anyway, so there is really no point in
>> keeping this config around[2].
>
> What?
> Anton, please explain. Why is it not static when build with CONFIG_STATIC_LINK?


LIBC itself tries to dynamic load stuff internally.

It is beyond our control and it claims that it will work only on EXACTLY
the same version of libc library as the one used for static link.

So you get a not-exactly static binary which is not properly moveable
between systems.

This is specifically in the name resolution, etc parts of libc which all
of: pcap, vector, vde, etc rely on.

Another alternative is to turn off static specifically for those.

Further to this - any properly written piece of networking code which
uses the newer functions for name/service resolution will have the same
problem. You can be static only if you do everything "manually" the old way.

>
> Thanks,
> //richard
>
> _______________________________________________
> linux-um mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-um
>


--
Anton R. Ivanov
Cambridgegreys Limited. Registered in England. Company Number 10273661
https://www.cambridgegreys.com/

2019-12-10 07:36:51

by Anton Ivanov

[permalink] [raw]
Subject: Re: [PATCH v1] uml: remove support for CONFIG_STATIC_LINK

On 10/12/2019 07:20, Anton Ivanov wrote:
> On 09/12/2019 23:15, Richard Weinberger wrote:
>> ----- Ursprüngliche Mail -----
>>> Von: "Brendan Higgins" <[email protected]>
>>> An: "Jeff Dike" <[email protected]>, "richard" <[email protected]>,
>>> "anton ivanov" <[email protected]>
>>> CC: "Johannes Berg" <[email protected]>, "linux-um"
>>> <[email protected]>, "linux-kernel"
>>> <[email protected]>, [email protected], "Brendan
>>> Higgins" <[email protected]>
>>> Gesendet: Dienstag, 10. Dezember 2019 00:02:48
>>> Betreff: [PATCH v1] uml: remove support for CONFIG_STATIC_LINK
>>
>>> CONFIG_STATIC_LINK appears to have been broken since before v4.20. It
>>> doesn't play nice with CONFIG_UML_NET_VECTOR=y:
>>>
>>> /usr/bin/ld: arch/um/drivers/vector_user.o: in function
>>> `user_init_socket_fds': vector_user.c:(.text+0x430): warning: Using
>>> 'getaddrinfo' in statically linked applications requires at runtime the
>>> shared libraries from the glibc version used for linking
>>
>> This is nothing serious.
>>
>>> And it seems to break the ptrace check:
>>>
>>> Checking that ptrace can change system call numbers...check_ptrace :
>>> child exited with exitcode 6, while expecting 0; status 0x67f
>>> [1]    126822 abort      ./linux mem=256M
>>
>> Didn't we fix that already?
>
> Yes we did - I commented on this.
>
>>
>>> (Apparently, a patch was recently discussed that fixes this - around
>>> v5.5-rc1[1] - but the fact that this was broken for over a year
>>> remains.)
>>>
>>> According to Anton, PCAP throws even more warnings, and the resulting
>>> binary isn't really even static anyway, so there is really no point in
>>> keeping this config around[2].
>>
>> What?
>> Anton, please explain. Why is it not static when build with
>> CONFIG_STATIC_LINK?
>
>
> LIBC itself tries to dynamic load stuff internally.
>
> It is beyond our control and it claims that it will work only on EXACTLY
> the same version of libc library as the one used for static link.
>
> So you get a not-exactly static binary which is not properly moveable
> between systems.
>
> This is specifically in the name resolution, etc parts of libc which all
> of: pcap, vector, vde, etc rely on.
>
> Another alternative is to turn off static specifically for those.
>
> Further to this - any properly written piece of networking code which
> uses the newer functions for name/service resolution will have the same
> problem. You can be static only if you do everything "manually" the old
> way.

The offending piece of code is the glibc implementation of getaddrinfo().

If you use it and link static the resulting binary is not really static.


>
>>
>> Thanks,
>> //richard
>>
>> _______________________________________________
>> linux-um mailing list
>> [email protected]
>> http://lists.infradead.org/mailman/listinfo/linux-um
>>
>
>


--
Anton R. Ivanov
Cambridgegreys Limited. Registered in England. Company Number 10273661
https://www.cambridgegreys.com/

2019-12-10 08:30:17

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH v1] uml: remove support for CONFIG_STATIC_LINK

On Tue, 2019-12-10 at 07:34 +0000, Anton Ivanov wrote:

> > Further to this - any properly written piece of networking code which
> > uses the newer functions for name/service resolution will have the same
> > problem. You can be static only if you do everything "manually" the old
> > way.
>
> The offending piece of code is the glibc implementation of getaddrinfo().
>
> If you use it and link static the resulting binary is not really static.

However, this (getaddrinfo) really only applies if you use the vector
network driver, if you e.g. use only virtio then this particular problem
isn't present.

Note sure if we implicitly call getaddrinfo from libpcap, but again,
that's just a single driver.

IOW, we could just make CONFIG_STATIC_LINK depend on !VECTOR && !PCAP?

johannes

2019-12-10 08:35:37

by Richard Weinberger

[permalink] [raw]
Subject: Re: [PATCH v1] uml: remove support for CONFIG_STATIC_LINK

----- Ursprüngliche Mail -----
> Von: "anton ivanov" <[email protected]>
>> LIBC itself tries to dynamic load stuff internally.
>>
>> It is beyond our control and it claims that it will work only on EXACTLY
>> the same version of libc library as the one used for static link.
>>
>> So you get a not-exactly static binary which is not properly moveable
>> between systems.
>>
>> This is specifically in the name resolution, etc parts of libc which all
>> of: pcap, vector, vde, etc rely on.
>>
>> Another alternative is to turn off static specifically for those.
>>
>> Further to this - any properly written piece of networking code which
>> uses the newer functions for name/service resolution will have the same
>> problem. You can be static only if you do everything "manually" the old
>> way.
>
> The offending piece of code is the glibc implementation of getaddrinfo().
>
> If you use it and link static the resulting binary is not really static.

glibc tries to load NSS and NIS stuff, yes. But what is the problem?

The goal of CONFIG_STATIC_LINK is that you can run UML without dependencies,
this used to work since ever. Lately it broke, but hey, let's fix it.
I have tons of old statically linked UML systems on my disk which I can
just run because they don't depend on specific libs.

Thanks,
//richard

2019-12-10 08:56:44

by Anton Ivanov

[permalink] [raw]
Subject: Re: [PATCH v1] uml: remove support for CONFIG_STATIC_LINK



On 10/12/2019 08:28, Johannes Berg wrote:
> On Tue, 2019-12-10 at 07:34 +0000, Anton Ivanov wrote:
>
>>> Further to this - any properly written piece of networking code which
>>> uses the newer functions for name/service resolution will have the same
>>> problem. You can be static only if you do everything "manually" the old
>>> way.
>>
>> The offending piece of code is the glibc implementation of getaddrinfo().
>>
>> If you use it and link static the resulting binary is not really static.
>
> However, this (getaddrinfo) really only applies if you use the vector
> network driver, if you e.g. use only virtio then this particular problem
> isn't present.
>
> Note sure if we implicitly call getaddrinfo from libpcap, but again,
> that's just a single driver.
>
> IOW, we could just make CONFIG_STATIC_LINK depend on !VECTOR && !PCAP?

+1

We also need to add VDE (wonder if anyone still uses that).

We will need to add XDP when I finish it. If memory servces me right,
libelf or libbpf has the same lovely features as NSS.

This is not just NSS - it is creeping in with a lot of new libraries.
Sometimes the libc guys fix that. For example, librt was like that when
I started working on epoll and vector IO.

Sometimes (as in the NSS case) they don't. So the static build
containing those will be broken and we are better off making it conflict
for those options.

>
> johannes
>
>
> _______________________________________________
> linux-um mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-um
>

--
Anton R. Ivanov
Cambridgegreys Limited. Registered in England. Company Number 10273661
https://www.cambridgegreys.com/

2019-12-10 19:42:26

by Brendan Higgins

[permalink] [raw]
Subject: Re: [PATCH v1] uml: remove support for CONFIG_STATIC_LINK

On Tue, Dec 10, 2019 at 12:54 AM Anton Ivanov
<[email protected]> wrote:
>
>
>
> On 10/12/2019 08:28, Johannes Berg wrote:
> > On Tue, 2019-12-10 at 07:34 +0000, Anton Ivanov wrote:
> >
> >>> Further to this - any properly written piece of networking code which
> >>> uses the newer functions for name/service resolution will have the same
> >>> problem. You can be static only if you do everything "manually" the old
> >>> way.
> >>
> >> The offending piece of code is the glibc implementation of getaddrinfo().
> >>
> >> If you use it and link static the resulting binary is not really static.
> >
> > However, this (getaddrinfo) really only applies if you use the vector
> > network driver, if you e.g. use only virtio then this particular problem
> > isn't present.
> >
> > Note sure if we implicitly call getaddrinfo from libpcap, but again,
> > that's just a single driver.
> >
> > IOW, we could just make CONFIG_STATIC_LINK depend on !VECTOR && !PCAP?
>
> +1
>
> We also need to add VDE (wonder if anyone still uses that).
>
> We will need to add XDP when I finish it. If memory servces me right,
> libelf or libbpf has the same lovely features as NSS.
>
> This is not just NSS - it is creeping in with a lot of new libraries.
> Sometimes the libc guys fix that. For example, librt was like that when
> I started working on epoll and vector IO.
>
> Sometimes (as in the NSS case) they don't. So the static build
> containing those will be broken and we are better off making it conflict
> for those options.

No strong opinions from me. I am more than happy to submit a new patch
that adds the negative dependencies.

2019-12-12 07:17:02

by Anton Ivanov

[permalink] [raw]
Subject: Re: [PATCH v1] uml: remove support for CONFIG_STATIC_LINK

On 12/12/2019 05:23, James McMechan wrote:
> Fixed up version without html...
> Um what is broken, uml seems to be working with the old stack and static linking.
>
> I saw a comment that the vector io stuff breaks uml on static link,
> but I was just running one without vector io, I had the pcap library linked in instead.
> It was up for about 3 days when I rebooted from mconsole 'cad'
> It would seem simplest to just mark vector io as !STATIC in the config...
> I was running 5.4.2 with a pretty close to defconfig
>
> I had to twiddle the uml pcap a little to deconflict the headers, and make sure pcap was not linking to dbus.
> I have not looked into vector io too closely I seemed to remember it was using host getaddrinfo the only thing in uml doing so I think it was pulling in the glibc, so-so dynamically link networking even when told not to... and that was breaking the startup process by changing memory maps behind the programs back or some such and then segfaulting the process much like the "we will implement async io in userspace behind the processes back" did back in the day.
>
> I would expect it to be possible to remove the getaddrinfo or use kernel functions.

If you just try to do 1:1 replacement with the legacy functions you stop
being v6 compliant. You after that need to go through quite a lot of
hoops and extra code to get that back.

You cannot use kernel functions - this is on the host userspace side of UML.

> It does not seem to have much in the way of comments, and I don't understand what it is trying to do.
> Why would a network protocol need to lookup "protocol/ip address/port num" sets in userspace? it should just be passing everything somewhere...
> Is it trying to do dynamic routing of the packets to different transports?

It is using shared code for (at present) tap (two forms - one optimized
for throughput, one using a tap/socket pair optimized for packets per
second), raw sockets, gre, l2tpv3 (both raw and over udp) and one
flavour unix domain sockets.

Both gre and l2tpv3 support v4 and v6.

While it is possible to replace getaddrinfo with a sequence of
gethostbyname, etc it will:

1. Grow the size of the code by at least 100 lines if not more. If I
take care of all corner cases which getaddrinfo() does - more.

2. Defeat one of the purposes of the exercise - to make it easy to add
new socket transports.

3. There is no guarantee that the glibc guys will not make gethostbyname
and friends fallback to getaddrinfo internally one day (in fact I am
surprised they have not done it yet).

>
> Slight confused but still running,
>
> Jim McMechan
>


--
Anton R. Ivanov
Cambridgegreys Limited. Registered in England. Company Number 10273661
https://www.cambridgegreys.com/