Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Skip to content
Commit 82aca127 authored by Bob Haarman's avatar Bob Haarman Committed by Alistair Delva
Browse files

FROMLIST: x86_64: fix jiffies ODR violation

`jiffies` and `jiffies_64` are meant to alias (two different symbols
that share the same address).  Most architectures make the symbols alias
to the same address via linker script assignment in their
arch/<arch>/kernel/vmlinux.lds.S:

jiffies = jiffies_64;

which is effectively a definition of jiffies.

jiffies and jiffies_64 are both forward declared for all arch's in:
include/linux/jiffies.h.

jiffies_64 is defined in kernel/time/timer.c for all arch's.

x86_64 was peculiar in that it wasn't doing the above linker script
assignment, but rather was:
1. defining jiffies in arch/x86/kernel/time.c instead via linker script.
2. overriding the symbol jiffies_64 from kernel/time/timer.c in
arch/x86/kernel/vmlinux.lds.s via `jiffies_64 = jiffies;`.

As Fangrui notes:
```
In LLD, symbol assignments in linker scripts override definitions in
object files. GNU ld appears to have the same behavior. It would
probably make sense for LLD to error "duplicate symbol" but GNU ld is
unlikely to adopt for compatibility reasons.
```

So we have an ODR violation (UB), which we seem to have gotten away
with thus far. Where it becomes harmful is when we:

1. Use -fno-semantic-interposition.

As Fangrui notes:
```
Clang after LLVM
commit 5b22bcc2b70d ("[X86][ELF] Prefer to lower MC_GlobalAddress
operands to .Lfoo$local")
defaults to -fno-semantic-interposition similar semantics which help
-fpic/-fPIC code avoid GOT/PLT when the referenced symbol is defined
within the same translation unit. Unlike GCC
-fno-semantic-interposition, Clang emits such relocations referencing
local symbols for non-pic code as well.
```

This causes references to jiffies to refer to `.Ljiffies$local` when
jiffies is defined in the same translation unit. Likewise, references
to jiffies_64 become references to `.Ljiffies_64$local` in translation
units that define jiffies_64.  Because these differ from the names
used in the linker script, they will not be rewritten to alias one
another.

Combined with ...

2. Full LTO effectively treats all source files as one translation
unit, causing these local references to be produced everywhere.  When
the linker processes the linker script, there are no longer any
references to `jiffies_64` anywhere to replace with `jiffies`.  And
thus `.Ljiffies$local` and `.Ljiffies_64$local` no longer alias
at all.

In the process of porting patches enabling Full LTO from arm64 to
x86_64, we observe spooky bugs where the kernel appeared to boot, but
init doesn't get scheduled.

Instead, we can avoid the ODR violation by matching other arch's by
defining jiffies only by linker script.  For -fno-semantic-interposition
+ Full LTO, there is no longer a global definition of jiffies for the
compiler to produce a local symbol which the linker script won't ensure
aliases to jiffies_64.

Link: https://github.com/ClangBuiltLinux/linux/issues/852


Fixes: 40747ffa ("asmlinkage: Make jiffies visible")
Cc: stable@vger.kernel.org
Reported-by: default avatarNathan Chancellor <natechancellor@gmail.com>
Reported-by: default avatarAlistair Delva <adelva@google.com>
Suggested-by: default avatarFangrui Song <maskray@google.com>
Debugged-by: default avatarNick Desaulniers <ndesaulniers@google.com>
Debugged-by: default avatarSami Tolvanen <samitolvanen@google.com>
Signed-off-by: default avatarBob Haarman <inglorion@google.com>

(am from https://lore.kernel.org/lkml/20200515180544.59824-1-inglorion@google.com/T/#u

)

Bug: 155426344
Signed-off-by: default avatarNick Desaulniers <ndesaulniers@google.com>
Change-Id: I6a5d3aeac388a0bc8254b83fa92e12376a04f922
parent 3763ace7
Loading
Loading
Loading
Loading
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment