Bug#983379: [PATCH] um: mark all kernel symbols as local
On Mon, 2021-03-08 at 15:59 +0530, Ritesh Raj Sarraf wrote: > Hi, > > Just a follow-up question on this fix. > > Is it something that is a candidate for linux-stable ? I guess that makes sense. Once it's in mainline you can also request that yourself :) johannes
Bug#983379: [PATCH] um: mark all kernel symbols as local
Hi,
Just a follow-up question on this fix.
Is it something that is a candidate for linux-stable ?
Thanks,
Ritesh
On Sat, 2021-03-06 at 16:21 +0530, Ritesh Raj Sarraf wrote:
> > > Marking all the symbols as local seems correct, and does seem
> > > to address the issue, so do that. Also do it for static link,
> > > nsswitch libraries could still be loaded there.
> > >
> > > [1] https://bugs.debian.org/983379
> > >
> > > Reported-by: Ritesh Raj Sarraf
> > > Signed-off-by: Johannes Berg
> > > ---
> > > arch/um/kernel/dyn.lds.S | 6 ++
> > > arch/um/kernel/uml.lds.S | 6 ++
> > > 2 files changed, 12 insertions(+)
> > >
> > > diff --git a/arch/um/kernel/dyn.lds.S b/arch/um/kernel/dyn.lds.S
> > > index dacbfabf66d8..2f2a8ce92f1e 100644
> > > --- a/arch/um/kernel/dyn.lds.S
> > > +++ b/arch/um/kernel/dyn.lds.S
> > > @@ -6,6 +6,12 @@ OUTPUT_ARCH(ELF_ARCH)
> > > ENTRY(_start)
> > > jiffies = jiffies_64;
> > >
> > > +VERSION {
> > > + {
> > > + local: *;
> > > + };
> > > +}
> > > +
> > > SECTIONS
> > > {
> > > PROVIDE (__executable_start = START);
> > > diff --git a/arch/um/kernel/uml.lds.S b/arch/um/kernel/uml.lds.S
> > > index 45d957d7004c..7a8e2b123e29 100644
> > > --- a/arch/um/kernel/uml.lds.S
> > > +++ b/arch/um/kernel/uml.lds.S
> > > @@ -7,6 +7,12 @@ OUTPUT_ARCH(ELF_ARCH)
> > > ENTRY(_start)
> > > jiffies = jiffies_64;
> > >
> > > +VERSION {
> > > + {
> > > + local: *;
> > > + };
> > > +}
> > > +
> > > SECTIONS
> > > {
> > > /* This must contain the right address - not quite the
> default
> > > ELF one.*/
> > >
>
> Tested on all 3 machines where the issue was seen before.
>
>
> >
> > Acked-By: Anton Ivanov
>
> Tested-By: Ritesh Raj Sarraf
--
Ritesh Raj Sarraf | http://people.debian.org/~rrs
Debian - The Universal Operating System
signature.asc
Description: This is a digitally signed message part
Bug#983379: [PATCH] um: mark all kernel symbols as local
On Fri, 2021-03-05 at 20:54 +, Anton Ivanov wrote:
> On 05/03/2021 20:43, Johannes Berg wrote:
> > From: Johannes Berg
> >
> > Ritesh reported a bug [1] against UML, noting that it crashed on
> > startup. The backtrace shows the following (heavily redacted):
> >
> > (gdb) bt
> > ...
> > #26 0x60015b5d in sem_init () at ipc/sem.c:268
> > #27 0x7f89906d92f7 in ?? () from /lib/x86_64-linux-
> > gnu/libcom_err.so.2
> > #28 0x7f8990ab8fb2 in call_init (...) at dl-init.c:72
> > ...
> > #40 0x7f89909bf3a6 in nss_load_library (...) at
> > nsswitch.c:359
> > ...
> > #44 0x7f8990895e35 in _nss_compat_getgrnam_r (...) at
> > nss_compat/compat-grp.c:486
> > #45 0x7f8990968b85 in __getgrnam_r [...]
> > #46 0x7f89909d6b77 in grantpt [...]
> > #47 0x7f8990a9394e in __GI_openpty [...]
> > #48 0x604a1f65 in openpty_cb (...) at arch/um/os-
> > Linux/sigio.c:407
> > #49 0x604a58d0 in start_idle_thread (...) at arch/um/os-
> > Linux/skas/process.c:598
> > #50 0x60004a3d in start_uml () at
> > arch/um/kernel/skas/process.c:45
> > #51 0x600047b2 in linux_main (...) at
> > arch/um/kernel/um_arch.c:334
> > #52 0x6000574f in main (...) at arch/um/os-
> > Linux/main.c:144
> >
> > indicating that the UML function openpty_cb() calls openpty(),
> > which internally calls __getgrnam_r(), which causes the nsswitch
> > machinery to get started.
> >
> > This loads, through lots of indirection that I snipped, the
> > libcom_err.so.2 library, which (in an unknown function, "??")
> > calls sem_init().
> >
> > Now, of course it wants to get libpthread's sem_init(), since
> > it's linked against libpthread. However, the dynamic linker
> > looks up that symbol against the binary first, and gets the
> > kernel's sem_init().
> >
> > Hajime Tazaki noted that "objcopy -L" can localize a symbol,
> > so the dynamic linker wouldn't do the lookup this way. I tried,
> > but for some reason that didn't seem to work.
> >
> > Doing the same thing in the linker script instead does seem to
> > work, though I cannot entirely explain - it *also* works if I
> > just add "VERSION { { global: *; }; }" instead, indicating that
> > something else is happening that I don't really understand. It
> > may be that explicitly doing that marks them with some kind of
> > empty version, and that's different from the default.
> >
> > Explicitly marking them with a version breaks kallsyms, so that
> > doesn't seem to be possible.
> >
> > Marking all the symbols as local seems correct, and does seem
> > to address the issue, so do that. Also do it for static link,
> > nsswitch libraries could still be loaded there.
> >
> > [1] https://bugs.debian.org/983379
> >
> > Reported-by: Ritesh Raj Sarraf
> > Signed-off-by: Johannes Berg
> > ---
> > arch/um/kernel/dyn.lds.S | 6 ++
> > arch/um/kernel/uml.lds.S | 6 ++
> > 2 files changed, 12 insertions(+)
> >
> > diff --git a/arch/um/kernel/dyn.lds.S b/arch/um/kernel/dyn.lds.S
> > index dacbfabf66d8..2f2a8ce92f1e 100644
> > --- a/arch/um/kernel/dyn.lds.S
> > +++ b/arch/um/kernel/dyn.lds.S
> > @@ -6,6 +6,12 @@ OUTPUT_ARCH(ELF_ARCH)
> > ENTRY(_start)
> > jiffies = jiffies_64;
> >
> > +VERSION {
> > + {
> > + local: *;
> > + };
> > +}
> > +
> > SECTIONS
> > {
> > PROVIDE (__executable_start = START);
> > diff --git a/arch/um/kernel/uml.lds.S b/arch/um/kernel/uml.lds.S
> > index 45d957d7004c..7a8e2b123e29 100644
> > --- a/arch/um/kernel/uml.lds.S
> > +++ b/arch/um/kernel/uml.lds.S
> > @@ -7,6 +7,12 @@ OUTPUT_ARCH(ELF_ARCH)
> > ENTRY(_start)
> > jiffies = jiffies_64;
> >
> > +VERSION {
> > + {
> > + local: *;
> > + };
> > +}
> > +
> > SECTIONS
> > {
> > /* This must contain the right address - not quite the default
> > ELF one.*/
> >
Tested on all 3 machines where the issue was seen before.
>
> Acked-By: Anton Ivanov
Tested-By: Ritesh Raj Sarraf
--
Ritesh Raj Sarraf | http://people.debian.org/~rrs
Debian - The Universal Operating System
signature.asc
Description: This is a digitally signed message part
Bug#983379: [PATCH] um: mark all kernel symbols as local
On 05/03/2021 20:43, Johannes Berg wrote:
From: Johannes Berg
Ritesh reported a bug [1] against UML, noting that it crashed on
startup. The backtrace shows the following (heavily redacted):
(gdb) bt
...
#26 0x60015b5d in sem_init () at ipc/sem.c:268
#27 0x7f89906d92f7 in ?? () from /lib/x86_64-linux-gnu/libcom_err.so.2
#28 0x7f8990ab8fb2 in call_init (...) at dl-init.c:72
...
#40 0x7f89909bf3a6 in nss_load_library (...) at nsswitch.c:359
...
#44 0x7f8990895e35 in _nss_compat_getgrnam_r (...) at
nss_compat/compat-grp.c:486
#45 0x7f8990968b85 in __getgrnam_r [...]
#46 0x7f89909d6b77 in grantpt [...]
#47 0x7f8990a9394e in __GI_openpty [...]
#48 0x604a1f65 in openpty_cb (...) at arch/um/os-Linux/sigio.c:407
#49 0x604a58d0 in start_idle_thread (...) at
arch/um/os-Linux/skas/process.c:598
#50 0x60004a3d in start_uml () at arch/um/kernel/skas/process.c:45
#51 0x600047b2 in linux_main (...) at arch/um/kernel/um_arch.c:334
#52 0x6000574f in main (...) at arch/um/os-Linux/main.c:144
indicating that the UML function openpty_cb() calls openpty(),
which internally calls __getgrnam_r(), which causes the nsswitch
machinery to get started.
This loads, through lots of indirection that I snipped, the
libcom_err.so.2 library, which (in an unknown function, "??")
calls sem_init().
Now, of course it wants to get libpthread's sem_init(), since
it's linked against libpthread. However, the dynamic linker
looks up that symbol against the binary first, and gets the
kernel's sem_init().
Hajime Tazaki noted that "objcopy -L" can localize a symbol,
so the dynamic linker wouldn't do the lookup this way. I tried,
but for some reason that didn't seem to work.
Doing the same thing in the linker script instead does seem to
work, though I cannot entirely explain - it *also* works if I
just add "VERSION { { global: *; }; }" instead, indicating that
something else is happening that I don't really understand. It
may be that explicitly doing that marks them with some kind of
empty version, and that's different from the default.
Explicitly marking them with a version breaks kallsyms, so that
doesn't seem to be possible.
Marking all the symbols as local seems correct, and does seem
to address the issue, so do that. Also do it for static link,
nsswitch libraries could still be loaded there.
[1] https://bugs.debian.org/983379
Reported-by: Ritesh Raj Sarraf
Signed-off-by: Johannes Berg
---
arch/um/kernel/dyn.lds.S | 6 ++
arch/um/kernel/uml.lds.S | 6 ++
2 files changed, 12 insertions(+)
diff --git a/arch/um/kernel/dyn.lds.S b/arch/um/kernel/dyn.lds.S
index dacbfabf66d8..2f2a8ce92f1e 100644
--- a/arch/um/kernel/dyn.lds.S
+++ b/arch/um/kernel/dyn.lds.S
@@ -6,6 +6,12 @@ OUTPUT_ARCH(ELF_ARCH)
ENTRY(_start)
jiffies = jiffies_64;
+VERSION {
+ {
+local: *;
+ };
+}
+
SECTIONS
{
PROVIDE (__executable_start = START);
diff --git a/arch/um/kernel/uml.lds.S b/arch/um/kernel/uml.lds.S
index 45d957d7004c..7a8e2b123e29 100644
--- a/arch/um/kernel/uml.lds.S
+++ b/arch/um/kernel/uml.lds.S
@@ -7,6 +7,12 @@ OUTPUT_ARCH(ELF_ARCH)
ENTRY(_start)
jiffies = jiffies_64;
+VERSION {
+ {
+local: *;
+ };
+}
+
SECTIONS
{
/* This must contain the right address - not quite the default ELF one.*/
Acked-By: Anton Ivanov
--
Anton R. Ivanov
Cambridgegreys Limited. Registered in England. Company Number 10273661
https://www.cambridgegreys.com/
Bug#983379: [PATCH] um: mark all kernel symbols as local
From: Johannes Berg
Ritesh reported a bug [1] against UML, noting that it crashed on
startup. The backtrace shows the following (heavily redacted):
(gdb) bt
...
#26 0x60015b5d in sem_init () at ipc/sem.c:268
#27 0x7f89906d92f7 in ?? () from /lib/x86_64-linux-gnu/libcom_err.so.2
#28 0x7f8990ab8fb2 in call_init (...) at dl-init.c:72
...
#40 0x7f89909bf3a6 in nss_load_library (...) at nsswitch.c:359
...
#44 0x7f8990895e35 in _nss_compat_getgrnam_r (...) at
nss_compat/compat-grp.c:486
#45 0x7f8990968b85 in __getgrnam_r [...]
#46 0x7f89909d6b77 in grantpt [...]
#47 0x7f8990a9394e in __GI_openpty [...]
#48 0x604a1f65 in openpty_cb (...) at arch/um/os-Linux/sigio.c:407
#49 0x604a58d0 in start_idle_thread (...) at
arch/um/os-Linux/skas/process.c:598
#50 0x60004a3d in start_uml () at arch/um/kernel/skas/process.c:45
#51 0x600047b2 in linux_main (...) at arch/um/kernel/um_arch.c:334
#52 0x6000574f in main (...) at arch/um/os-Linux/main.c:144
indicating that the UML function openpty_cb() calls openpty(),
which internally calls __getgrnam_r(), which causes the nsswitch
machinery to get started.
This loads, through lots of indirection that I snipped, the
libcom_err.so.2 library, which (in an unknown function, "??")
calls sem_init().
Now, of course it wants to get libpthread's sem_init(), since
it's linked against libpthread. However, the dynamic linker
looks up that symbol against the binary first, and gets the
kernel's sem_init().
Hajime Tazaki noted that "objcopy -L" can localize a symbol,
so the dynamic linker wouldn't do the lookup this way. I tried,
but for some reason that didn't seem to work.
Doing the same thing in the linker script instead does seem to
work, though I cannot entirely explain - it *also* works if I
just add "VERSION { { global: *; }; }" instead, indicating that
something else is happening that I don't really understand. It
may be that explicitly doing that marks them with some kind of
empty version, and that's different from the default.
Explicitly marking them with a version breaks kallsyms, so that
doesn't seem to be possible.
Marking all the symbols as local seems correct, and does seem
to address the issue, so do that. Also do it for static link,
nsswitch libraries could still be loaded there.
[1] https://bugs.debian.org/983379
Reported-by: Ritesh Raj Sarraf
Signed-off-by: Johannes Berg
---
arch/um/kernel/dyn.lds.S | 6 ++
arch/um/kernel/uml.lds.S | 6 ++
2 files changed, 12 insertions(+)
diff --git a/arch/um/kernel/dyn.lds.S b/arch/um/kernel/dyn.lds.S
index dacbfabf66d8..2f2a8ce92f1e 100644
--- a/arch/um/kernel/dyn.lds.S
+++ b/arch/um/kernel/dyn.lds.S
@@ -6,6 +6,12 @@ OUTPUT_ARCH(ELF_ARCH)
ENTRY(_start)
jiffies = jiffies_64;
+VERSION {
+ {
+local: *;
+ };
+}
+
SECTIONS
{
PROVIDE (__executable_start = START);
diff --git a/arch/um/kernel/uml.lds.S b/arch/um/kernel/uml.lds.S
index 45d957d7004c..7a8e2b123e29 100644
--- a/arch/um/kernel/uml.lds.S
+++ b/arch/um/kernel/uml.lds.S
@@ -7,6 +7,12 @@ OUTPUT_ARCH(ELF_ARCH)
ENTRY(_start)
jiffies = jiffies_64;
+VERSION {
+ {
+local: *;
+ };
+}
+
SECTIONS
{
/* This must contain the right address - not quite the default ELF one.*/
--
2.26.2
