[PATCH v2 0/2] mips: vdso: fix 'jalr t9' crash in vdso code

[PATCH v2 0/2] mips: vdso: fix 'jalr t9' crash in vdso code

[Victor Kamensky]

Hi Paul and all,

Here is the second version of patch set dealing with 'jalr t9'
crash in vdso code. Root cause and investigation details could be
found in first version cover letter for this patch series [1].

Changes in v2:
   - added -mrelax-pic-calls -mexplicit-relocs unconditionally
     (dropped 'call cc-option') since minimal supported gcc version
     already has them
   - fixed few misspellings in commit messages
   - added explanation in commit messages about handling static
     functions PIC calls through mips local GOT and absence of dynamic
     relocations in this case

Thanks,
Victor

[1] https://lore.kernel.org/linux-mips/20200203233133.38613-1-kamensky@cisco.com

Victor Kamensky (2):
  mips: vdso: fix 'jalr t9' crash in vdso code
  mips: vdso: add build time check that no 'jalr t9' calls left

 arch/mips/vdso/Makefile | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

-- 
2.24.1

[PATCH v2 1/2] mips: vdso: fix 'jalr t9' crash in vdso code

[Victor Kamensky]

Observed that when kernel is built with Yocto mips64-poky-linux-gcc,
and mips64-poky-linux-gnun32-gcc toolchain, resulting vdso contains
'jalr t9' instructions in its code and since in vdso case nobody
sets GOT table code crashes when instruction reached. On other hand
observed that when kernel is built mips-poky-linux-gcc toolchain, the
same 'jalr t9' instruction are replaced with PC relative function
calls using 'bal' instructions.

The difference boils down to -mrelax-pic-calls and -mexplicit-relocs
gcc options that gets different default values depending on gcc
target triplets and corresponding binutils. -mrelax-pic-calls got
enabled by default only in mips-poky-linux-gcc case. MIPS binutils
ld relies on R_MIPS_JALR relocation to convert 'jalr t9' into 'bal'
and such relocation is generated only if -mrelax-pic-calls option
is on.

Please note 'jalr t9' conversion to 'bal' can happen only to static
functions. These static PIC calls use mips local GOT entries that
are supposed to be filled with start of DSO value by run-time linker
(missing in VDSO case) and they do not have dynamic relocations.
Global mips GOT entries must have dynamic relocations and they should
be prevented by cmd_vdso_check Makefile rule.

Solution call out -mrelax-pic-calls and -mexplicit-relocs options
explicitly while compiling MIPS vdso code. That would get correct
and consistent between different toolchains behaviour.

Reported-by: Bruce Ashfield <bruce.ashfield@gmail.com>
Signed-off-by: Victor Kamensky <kamensky@cisco.com>
---
 arch/mips/vdso/Makefile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/mips/vdso/Makefile b/arch/mips/vdso/Makefile
index aa89a41dc5dd..848baeaef1f8 100644
--- a/arch/mips/vdso/Makefile
+++ b/arch/mips/vdso/Makefile
@@ -33,6 +33,7 @@ endif
 cflags-vdso := $(ccflags-vdso) \
 	$(filter -W%,$(filter-out -Wa$(comma)%,$(KBUILD_CFLAGS))) \
 	-O3 -g -fPIC -fno-strict-aliasing -fno-common -fno-builtin -G 0 \
+	-mrelax-pic-calls -mexplicit-relocs \
 	-fno-stack-protector -fno-jump-tables -DDISABLE_BRANCH_PROFILING \
 	$(call cc-option, -fno-asynchronous-unwind-tables) \
 	$(call cc-option, -fno-stack-protector)
-- 
2.24.1

[PATCH v2 2/2] mips: vdso: add build time check that no 'jalr t9' calls left

[Victor Kamensky]

vdso shared object cannot have GOT based PIC 'jalr t9' calls
because nobody set GOT table in vdso. Contributing into vdso
.o files are compiled in PIC mode and as result for internal
static functions calls compiler will generate 'jalr t9'
instructions. Those are supposed to be converted into PC
relative 'bal' calls by linker when relocation are processed.

Mips global GOT entries do have dynamic relocations and they
will be caught by cmd_vdso_check Makefile rule. Static PIC
calls go through mips local GOT entries that do not have
dynamic relocations. For those 'jalr t9' calls could be present
but without dynamic relocations and they need to be converted
to 'bal' calls by linker.

Add additional build time check to make sure that no 'jalr t9'
slip through because of some toolchain misconfiguration that
prevents 'jalr t9' to 'bal' conversion.

Signed-off-by: Victor Kamensky <kamensky@cisco.com>
---
 arch/mips/vdso/Makefile | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/arch/mips/vdso/Makefile b/arch/mips/vdso/Makefile
index 848baeaef1f8..8d713e0ae33a 100644
--- a/arch/mips/vdso/Makefile
+++ b/arch/mips/vdso/Makefile
@@ -82,12 +82,18 @@ GCOV_PROFILE := n
 UBSAN_SANITIZE := n
 KCOV_INSTRUMENT := n
 
+# Check that we don't have PIC 'jalr t9' calls left
+quiet_cmd_vdso_mips_check = VDSOCHK $@
+      cmd_vdso_mips_check = if $(OBJDUMP) --disassemble $@ | egrep -h "jalr.*t9" > /dev/null; \
+		       then (echo >&2 "$@: PIC 'jalr t9' calls are not supported"; \
+			     rm -f $@; /bin/false); fi
+
 #
 # Shared build commands.
 #
 
 quiet_cmd_vdsold_and_vdso_check = LD      $@
-      cmd_vdsold_and_vdso_check = $(cmd_vdsold); $(cmd_vdso_check)
+      cmd_vdsold_and_vdso_check = $(cmd_vdsold); $(cmd_vdso_check); $(cmd_vdso_mips_check)
 
 quiet_cmd_vdsold = VDSO    $@
       cmd_vdsold = $(CC) $(c_flags) $(VDSO_LDFLAGS) \
-- 
2.24.1

Re: [PATCH v2 0/2] mips: vdso: fix 'jalr t9' crash in vdso code

[Paul Burton]

Hello,

Victor Kamensky wrote:
> Hi Paul and all,
> 
> Here is the second version of patch set dealing with 'jalr t9'
> crash in vdso code. Root cause and investigation details could be
> found in first version cover letter for this patch series [1].
> 
> Changes in v2:
>    - added -mrelax-pic-calls -mexplicit-relocs unconditionally
>      (dropped 'call cc-option') since minimal supported gcc version
>      already has them
>    - fixed few misspellings in commit messages
>    - added explanation in commit messages about handling static
>      functions PIC calls through mips local GOT and absence of dynamic
>      relocations in this case
> 
> Thanks,
> Victor
> 
> [1] https://lore.kernel.org/linux-mips/20200203233133.38613-1-kamensky@cisco.com
> 
> Victor Kamensky (2):
>   mips: vdso: fix 'jalr t9' crash in vdso code
>   mips: vdso: add build time check that no 'jalr t9' calls left
> 
>  arch/mips/vdso/Makefile | 9 ++++++++-

Series applied to mips-fixes.

> mips: vdso: fix 'jalr t9' crash in vdso code
>   commit d3f703c4359f
>   https://git.kernel.org/mips/c/d3f703c4359f
>   
>   Reported-by: Bruce Ashfield <bruce.ashfield@gmail.com>
>   Signed-off-by: Victor Kamensky <kamensky@cisco.com>
>   Signed-off-by: Paul Burton <paulburton@kernel.org>
> 
> mips: vdso: add build time check that no 'jalr t9' calls left
>   commit 976c23af3ee5
>   https://git.kernel.org/mips/c/976c23af3ee5
>   
>   Signed-off-by: Victor Kamensky <kamensky@cisco.com>
>   Signed-off-by: Paul Burton <paulburton@kernel.org>

Thanks,
    Paul

[ This message was auto-generated; if you believe anything is incorrect
  then please email paulburton@kernel.org to report it. ]