[tip:,x86/bugs] x86/retpoline: Ensure default return thunk isn't used at runtime
Commit Message
The following commit has been merged into the x86/bugs branch of tip:
Commit-ID: 4461438a8405e800f90e0e40409e5f3d07eed381
Gitweb: https://git.kernel.org/tip/4461438a8405e800f90e0e40409e5f3d07eed381
Author: Josh Poimboeuf <jpoimboe@kernel.org>
AuthorDate: Wed, 03 Jan 2024 19:36:26 +01:00
Committer: Borislav Petkov (AMD) <bp@alien8.de>
CommitterDate: Mon, 12 Feb 2024 11:42:15 +01:00
x86/retpoline: Ensure default return thunk isn't used at runtime
Make sure the default return thunk is not used after all return
instructions have been patched by the alternatives because the default
return thunk is insufficient when it comes to mitigating Retbleed or
SRSO.
Fix based on an earlier version by David Kaplan <david.kaplan@amd.com>.
[ bp: Fix the compilation error of warn_thunk_thunk being an invisible
symbol, hoist thunk macro into calling.h ]
Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
Co-developed-by: Borislav Petkov (AMD) <bp@alien8.de>
Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de>
Link: https://lore.kernel.org/r/20231010171020.462211-4-david.kaplan@amd.com
Link: https://lore.kernel.org/r/20240104132446.GEZZaxnrIgIyat0pqf@fat_crate.local
---
arch/x86/entry/calling.h | 60 +++++++++++++++++++++++++++-
arch/x86/entry/entry.S | 4 ++-
arch/x86/entry/thunk_32.S | 34 +++------------
arch/x86/entry/thunk_64.S | 33 +---------------
arch/x86/include/asm/nospec-branch.h | 2 +-
arch/x86/kernel/cpu/bugs.c | 5 ++-
arch/x86/lib/retpoline.S | 15 ++-----
7 files changed, 85 insertions(+), 68 deletions(-)
Comments
On Mon, Feb 12, 2024 at 02:13:39PM -0000, tip-bot2 for Josh Poimboeuf wrote:
> The following commit has been merged into the x86/bugs branch of tip:
>
> Commit-ID: 4461438a8405e800f90e0e40409e5f3d07eed381
> Gitweb: https://git.kernel.org/tip/4461438a8405e800f90e0e40409e5f3d07eed381
> Author: Josh Poimboeuf <jpoimboe@kernel.org>
> AuthorDate: Wed, 03 Jan 2024 19:36:26 +01:00
> Committer: Borislav Petkov (AMD) <bp@alien8.de>
> CommitterDate: Mon, 12 Feb 2024 11:42:15 +01:00
>
> x86/retpoline: Ensure default return thunk isn't used at runtime
>
> Make sure the default return thunk is not used after all return
> instructions have been patched by the alternatives because the default
> return thunk is insufficient when it comes to mitigating Retbleed or
> SRSO.
>
> Fix based on an earlier version by David Kaplan <david.kaplan@amd.com>.
>
> [ bp: Fix the compilation error of warn_thunk_thunk being an invisible
> symbol, hoist thunk macro into calling.h ]
>
> Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
> Co-developed-by: Borislav Petkov (AMD) <bp@alien8.de>
> Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de>
> Link: https://lore.kernel.org/r/20231010171020.462211-4-david.kaplan@amd.com
> Link: https://lore.kernel.org/r/20240104132446.GEZZaxnrIgIyat0pqf@fat_crate.local
This warning is now getting triggered for me in some of my builds,
specifically from Alpine Linux's configuration. A minimal reproducer on
top of defconfig:
$ echo 'CONFIG_X86_KERNEL_IBT=n
CONFIG_UNWINDER_ORC=n
CONFIG_UNWINDER_FRAME_POINTER=y' >arch/x86/configs/repro.config
$ make -skj"$(nproc)" ARCH=x86_64 CROSS_COMPILE=x86_64-linux- mrproper defconfig repro.config bzImage
$ qemu-system-x86_64 \
-display none \
-nodefaults \
-d unimp,guest_errors \
-append 'console=ttyS0 earlycon=uart8250,io,0x3f8' \
-kernel arch/x86/boot/bzImage \
-initrd rootfs.cpio \
-cpu host \
-enable-kvm \
-m 512m \
-smp 2 \
-serial mon:stdio
[ 0.000000] Linux version 6.7.0-01738-g4461438a8405-dirty (nathan@dev-arch.thelio-3990X) (x86_64-linux-gcc (GCC) 13.2.0, GNU ld (GNU Binutils) 2.41) #1 SMP PREEMPT_DYNAMIC Wed Feb 14 20:14:55 MST 2024
..
[ 0.337317] ------------[ cut here ]------------
[ 0.338282] Unpatched return thunk in use. This should not happen!
[ 0.339292] WARNING: CPU: 0 PID: 1 at arch/x86/kernel/cpu/bugs.c:2856 __warn_thunk+0x27/0x40
[ 0.340284] Modules linked in:
[ 0.341021] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 6.7.0-01738-g4461438a8405-dirty #1
[ 0.341281] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.16.3-0-ga6ed6b701f0a-prebuilt.qemu.org 04/01/2014
[ 0.342281] RIP: 0010:__warn_thunk+0x27/0x40
[ 0.343281] Code: 90 90 90 80 3d 22 20 c3 01 00 74 05 e9 32 a5 eb 00 55 c6 05 13 20 c3 01 01 48 89 e5 90 48 c7 c7 80 80 50 89 e8 6a c4 03 00 90 <0f> 0b 90 90 5d e9 0f a5 eb 00 cc cc cc cc cc cc cc cc cc cc cc cc
[ 0.344286] RSP: 0018:ffff8ba9c0013e10 EFLAGS: 00010286
[ 0.345281] RAX: 0000000000000000 RBX: ffffffff89afba70 RCX: 0000000000000000
[ 0.346281] RDX: 0000000000000000 RSI: 00000000ffffdfff RDI: 0000000000000001
[ 0.347282] RBP: ffff8ba9c0013e10 R08: 00000000ffffdfff R09: ffff8ba9c0013c88
[ 0.348282] R10: 0000000000000001 R11: ffffffff89856ae0 R12: 0000000000000000
[ 0.349282] R13: ffff88c101126ac0 R14: ffff8ba9c0013e78 R15: 0000000000000000
[ 0.350285] FS: 0000000000000000(0000) GS:ffff88c11f000000(0000) knlGS:0000000000000000
[ 0.351283] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 0.352282] CR2: ffff88c119601000 CR3: 0000000018e2c000 CR4: 0000000000350ef0
[ 0.353284] Call Trace:
[ 0.354281] <TASK>
[ 0.355281] ? show_regs+0x60/0x70
[ 0.356281] ? __warn+0x84/0x150
[ 0.357281] ? __warn_thunk+0x27/0x40
[ 0.358281] ? report_bug+0x16d/0x1a0
[ 0.359088] ? console_unlock+0x4f/0xe0
[ 0.359281] ? handle_bug+0x43/0x80
[ 0.360228] ? exc_invalid_op+0x18/0x70
[ 0.360281] ? asm_exc_invalid_op+0x1b/0x20
[ 0.361282] ? ia32_binfmt_init+0x40/0x40
[ 0.362283] ? __warn_thunk+0x27/0x40
[ 0.363283] warn_thunk_thunk+0x16/0x30
[ 0.364283] do_one_initcall+0x59/0x230
[ 0.365284] kernel_init_freeable+0x1a4/0x2e0
[ 0.366248] ? __pfx_kernel_init+0x10/0x10
[ 0.366282] kernel_init+0x15/0x1b0
[ 0.367200] ret_from_fork+0x38/0x60
[ 0.367280] ? __pfx_kernel_init+0x10/0x10
[ 0.368175] ret_from_fork_asm+0x1b/0x30
[ 0.368285] </TASK>
[ 0.369280] ---[ end trace 0000000000000000 ]---
..
If there is any more information I can provide or patches I can test, I
am more than happy to do so.
Cheers,
Nathan
# bad: [2c3b09aac00d7835023bbc4473ee06696be64fa8] Add linux-next specific files for 20240214
# good: [7e90b5c295ec1e47c8ad865429f046970c549a66] Merge tag 'trace-tools-v6.8-rc4' of git://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace
git bisect start '2c3b09aac00d7835023bbc4473ee06696be64fa8' '7e90b5c295ec1e47c8ad865429f046970c549a66'
# good: [a4f281576352365d7c83d9a2ff46c0430c8d6f1d] Merge branch 'main' of git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git
git bisect good a4f281576352365d7c83d9a2ff46c0430c8d6f1d
# good: [2b837601fcd12acc492699f9148ca20a41d76b5d] Merge branch 'for-next' of git://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git
git bisect good 2b837601fcd12acc492699f9148ca20a41d76b5d
# bad: [4b0fab17a40c71b1202109cca7ab4854722f6fee] Merge branch 'usb-next' of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git
git bisect bad 4b0fab17a40c71b1202109cca7ab4854722f6fee
# bad: [09e1b07412d3a47f343acd2ab2459af3034e028b] Merge branch 'master' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git
git bisect bad 09e1b07412d3a47f343acd2ab2459af3034e028b
# good: [2208f1364f1de82b19313f36e3e4758487183639] Merge branch 'for-next' of git://git.kernel.org/pub/scm/linux/kernel/git/krzk/linux-dt.git
git bisect good 2208f1364f1de82b19313f36e3e4758487183639
# good: [fbbf6ba42cd4bbe1675db713d230fccda1183a47] Merge branch into tip/master: 'timers/ptp'
git bisect good fbbf6ba42cd4bbe1675db713d230fccda1183a47
# good: [0da9a7e5c86b003a9b446b30c90eaf96b2e442c2] spi: get rid of some legacy macros
git bisect good 0da9a7e5c86b003a9b446b30c90eaf96b2e442c2
# good: [64ffc035640f3a74205ae57d21fb171a88748b60] Merge branch into tip/master: 'irq/urgent'
git bisect good 64ffc035640f3a74205ae57d21fb171a88748b60
# good: [d1ff85fdf0b8f63a6e042ae7559c630f9b1c50e2] spi: pl022: Use typedef for dma_filter_fn
git bisect good d1ff85fdf0b8f63a6e042ae7559c630f9b1c50e2
# bad: [743a9723b476831c7910e6e15a714a713ab5989f] Merge branch into tip/master: 'x86/bugs'
git bisect bad 743a9723b476831c7910e6e15a714a713ab5989f
# good: [ee4c1592b7e9a5bf89b962d7afd7e9b04c8d16ee] irqchip/gic-v3-its: Remove usage of the deprecated ida_simple_xx() API
git bisect good ee4c1592b7e9a5bf89b962d7afd7e9b04c8d16ee
# good: [850d0fd76557fa4ad2d389a7d380f8a40043f874] Merge branch into tip/master: 'x86/urgent'
git bisect good 850d0fd76557fa4ad2d389a7d380f8a40043f874
# bad: [4461438a8405e800f90e0e40409e5f3d07eed381] x86/retpoline: Ensure default return thunk isn't used at runtime
git bisect bad 4461438a8405e800f90e0e40409e5f3d07eed381
# first bad commit: [4461438a8405e800f90e0e40409e5f3d07eed381] x86/retpoline: Ensure default return thunk isn't used at runtime
On 15.02.24 г. 5:20 ч., Nathan Chancellor wrote:
> On Mon, Feb 12, 2024 at 02:13:39PM -0000, tip-bot2 for Josh Poimboeuf wrote:
>> The following commit has been merged into the x86/bugs branch of tip:
>>
>> Commit-ID: 4461438a8405e800f90e0e40409e5f3d07eed381
>> Gitweb: https://git.kernel.org/tip/4461438a8405e800f90e0e40409e5f3d07eed381
>> Author: Josh Poimboeuf <jpoimboe@kernel.org>
>> AuthorDate: Wed, 03 Jan 2024 19:36:26 +01:00
>> Committer: Borislav Petkov (AMD) <bp@alien8.de>
>> CommitterDate: Mon, 12 Feb 2024 11:42:15 +01:00
>>
>> x86/retpoline: Ensure default return thunk isn't used at runtime
>>
>> Make sure the default return thunk is not used after all return
>> instructions have been patched by the alternatives because the default
>> return thunk is insufficient when it comes to mitigating Retbleed or
>> SRSO.
>>
>> Fix based on an earlier version by David Kaplan <david.kaplan@amd.com>.
>>
>> [ bp: Fix the compilation error of warn_thunk_thunk being an invisible
>> symbol, hoist thunk macro into calling.h ]
>>
>> Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
>> Co-developed-by: Borislav Petkov (AMD) <bp@alien8.de>
>> Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de>
>> Link: https://lore.kernel.org/r/20231010171020.462211-4-david.kaplan@amd.com
>> Link: https://lore.kernel.org/r/20240104132446.GEZZaxnrIgIyat0pqf@fat_crate.local
>
> This warning is now getting triggered for me in some of my builds,
> specifically from Alpine Linux's configuration. A minimal reproducer on
> top of defconfig:
>
> $ echo 'CONFIG_X86_KERNEL_IBT=n
> CONFIG_UNWINDER_ORC=n
> CONFIG_UNWINDER_FRAME_POINTER=y' >arch/x86/configs/repro.config
>
I was able to reproduce this and it seems to go away if KERNEL_IBT=y.
When looking at the disassembly of do_one_initcall it seems the 2 return
sites are not patched at all, I see:
0xffffffff81001284 <+84>: call 0xffffffff81f2d000
<__x86_indirect_thunk_array+96>
0xffffffff810012e7 <+183>: jmp 0xffffffff81f2d760
<__x86_return_thunk>
The former should be rewritten to an indirect call as per
patch_retpoline and the latter should be rewritten altogether. I wonder
if objtool ignores the function for some reason ...
On Wed, Feb 14, 2024 at 08:20:49PM -0700, Nathan Chancellor wrote:
> On Mon, Feb 12, 2024 at 02:13:39PM -0000, tip-bot2 for Josh Poimboeuf wrote:
> > The following commit has been merged into the x86/bugs branch of tip:
> >
> > Commit-ID: 4461438a8405e800f90e0e40409e5f3d07eed381
> > Gitweb: https://git.kernel.org/tip/4461438a8405e800f90e0e40409e5f3d07eed381
> > Author: Josh Poimboeuf <jpoimboe@kernel.org>
> > AuthorDate: Wed, 03 Jan 2024 19:36:26 +01:00
> > Committer: Borislav Petkov (AMD) <bp@alien8.de>
> > CommitterDate: Mon, 12 Feb 2024 11:42:15 +01:00
> >
> > x86/retpoline: Ensure default return thunk isn't used at runtime
> >
> > Make sure the default return thunk is not used after all return
> > instructions have been patched by the alternatives because the default
> > return thunk is insufficient when it comes to mitigating Retbleed or
> > SRSO.
> >
> > Fix based on an earlier version by David Kaplan <david.kaplan@amd.com>.
> >
> > [ bp: Fix the compilation error of warn_thunk_thunk being an invisible
> > symbol, hoist thunk macro into calling.h ]
> >
> > Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
> > Co-developed-by: Borislav Petkov (AMD) <bp@alien8.de>
> > Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de>
> > Link: https://lore.kernel.org/r/20231010171020.462211-4-david.kaplan@amd.com
> > Link: https://lore.kernel.org/r/20240104132446.GEZZaxnrIgIyat0pqf@fat_crate.local
>
> This warning is now getting triggered for me in some of my builds,
> specifically from Alpine Linux's configuration. A minimal reproducer on
> top of defconfig:
>
> $ echo 'CONFIG_X86_KERNEL_IBT=n
> CONFIG_UNWINDER_ORC=n
> CONFIG_UNWINDER_FRAME_POINTER=y' >arch/x86/configs/repro.config
Yeah, the special vdso glue:
[ 325.985728] PCI: Using configuration type 1 for base access
[ 325.986637] PCI: Using configuration type 1 for extended access
[ 325.987797] initcall pci_arch_init+0x0/0x90 returned 0 after 3000 usecs
[ 325.988815] calling init_vdso_image_64+0x0/0x30 @ 1
[ 325.989804] ------------[ cut here ]------------
[ 325.990724] Unpatched return thunk in use. This should not happen!
[ 325.991735] WARNING: CPU: 0 PID: 1 at arch/x86/kernel/cpu/bugs.c:2856 __warn_thunk+0x27/0x40
[ 325.992793] Modules linked in:
[ 325.993440] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 6.8.0-rc4-00040-g4589f199eb68 #1
[ 325.993794] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
[ 325.994794] RIP: 0010:__warn_thunk+0x27/0x40
[ 325.995648] Code: 90 90 90 80 3d 62 38 c3 01 00 74 05 e9 52 07 ee 00 55 c6 05 53 38 c3 01 01 48 89 e5 90 48 c7 c7 08 54 71 82 e8 9a c9 03 00 90 <0f> 0b 90 90 5d e9 2f 07 ee 00 cc cc cc cc cc cc cc cc cc cc cc cc
[ 325.996793] RSP: 0018:ffffc90000013e10 EFLAGS: 00010286
[ 325.997793] RAX: 0000000000000000 RBX: ffffffff82cfdac0 RCX: 0000000000000000
[ 325.998795] RDX: 0000000000000000 RSI: 00000000fff7ffff RDI: 0000000000000001
[ 325.999793] RBP: ffffc90000013e10 R08: 00000000fff7ffff R09: ffffc90000013c90
[ 326.000794] R10: 0000000000000001 R11: ffff88807b7df000 R12: 0000000000000000
[ 326.001793] R13: ffff8880035f9900 R14: ffffc90000013e78 R15: 0000000000000000
[ 326.002795] FS: 0000000000000000(0000) GS:ffff888079200000(0000) knlGS:0000000000000000
[ 326.003795] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 326.004793] CR2: ffff888003201000 CR3: 0000000002a2c000 CR4: 00000000003506f0
[ 326.005794] Call Trace:
[ 326.006339] <TASK>
[ 326.006794] ? show_regs+0x5f/0x70
[ 326.007501] ? __warn+0x83/0x130
[ 326.007794] ? __warn_thunk+0x27/0x40
[ 326.008542] ? report_bug+0x171/0x1a0
[ 326.008793] ? srso_return_thunk+0x5/0x5f
[ 326.009603] ? console_unlock+0x4f/0xe0
[ 326.010381] ? handle_bug+0x43/0x80
[ 326.010795] ? exc_invalid_op+0x18/0x70
[ 326.011592] ? asm_exc_invalid_op+0x1b/0x20
[ 326.012440] ? ia32_binfmt_init+0x40/0x40
[ 326.012795] ? __warn_thunk+0x27/0x40
[ 326.013546] warn_thunk_thunk+0x16/0x30
[ 326.013795] do_one_initcall+0x59/0x230
[ 326.014574] kernel_init_freeable+0x19a/0x2d0
[ 326.014794] ? __pfx_kernel_init+0x10/0x10
[ 326.015629] kernel_init+0x15/0x1b0
[ 326.016326] ret_from_fork+0x38/0x60
[ 326.016794] ? __pfx_kernel_init+0x10/0x10
[ 326.017627] ret_from_fork_asm+0x1a/0x30
[ 326.018394] </TASK>
[ 326.018794] ---[ end trace 0000000000000000 ]---
[ 326.019705] initcall init_vdso_image_64+0x0/0x30 returned 0 after 29000 usecs
[ 326.020794] calling init_vdso_image_32+0x0/0x20 @ 1
During build we do:
# VDSO2C arch/x86/entry/vdso/vdso-image-64.c
arch/x86/entry/vdso/vdso2c arch/x86/entry/vdso/vdso64.so.dbg arch/x86/entry/vdso/vdso64.so arch/x86/entry/vdso/vdso-image-64.c
...
# CC arch/x86/entry/vdso/vdso-image-64.o
gcc -Wp,-MMD,arch/x86/entry/vdso/.vdso-image-64.o.d -nostdinc -I./arch/x86/include -I./arch/x86/include/generated -I./include -I./arch/x86/include/uapi -I./arch/x86/include/generated/uapi -I./include/uapi -I./include/generated/uapi -include ./include/linux/compiler-version.h -include ./include/linux/kconfig.h -include ./include/linux/compiler_types.h -D__KERNEL__ -fmacro-prefix-map=./= -Werror -std=gnu11 -fshort-wchar -funsigned-char -fno-common -fno-PIE -fno-strict-aliasing -mno-sse -mno-mmx -mno-sse2 -mno-3dnow -mno-avx -fcf-protection=none -m64 -falign-jumps=1 -falign-loops=1 -mno-80387 -mno-fp-ret-in-387 -mpreferred-stack-boundary=3 -mskip-rax-setup -mtune=generic -mno-red-zone -mcmodel=kernel -Wno-sign-compare -fno-asynchronous-unwind-tables -mindirect-branch=thunk-extern -mindirect-branch-register -mindirect-branch-cs-prefix -mfunction-return=thunk-extern -fno-jump-tables -fpatchable-function-entry=16,16 -fno-delete-null-pointer-checks -O2 -fno-allow-store-data-races -fstack-protector-strong -fno-omit-frame-pointer -fno-optimize-sibling-calls -ftrivial-auto-var-init=zero -fno-stack-clash-protection -falign-functions=16 -fstrict-flex-arrays=3 -fno-strict-overflow -fno-stack-check -fconserve-stack -Wall -Wundef -Werror=implicit-function-declaration -Werror=implicit-int -Werror=return-type -Werror=strict-prototypes -Wno-format-security -Wno-trigraphs -Wno-frame-address -Wno-address-of-packed-member -Wmissing-declarations -Wmissing-prototypes -Wframe-larger-than=2048 -Wno-main -Wno-unused-but-set-variable -Wno-unused-const-variable -Wno-dangling-pointer -Wvla -Wno-pointer-sign -Wcast-function-type -Wno-stringop-overflow -Wno-array-bounds -Wno-alloc-size-larger-than -Wimplicit-fallthrough=5 -Werror=date-time -Werror=incompatible-pointer-types -Werror=designated-init -Wenum-conversion -Wno-unused-but-set-variable -Wno-unused-const-variable -Wno-restrict -Wno-packed-not-aligned -Wno-format-overflow -Wno-format-truncation -Wno-stringop-truncation -Wno-missing-field-initializers -Wno-type-limits -Wno-shift-negative-value -Wno-maybe-uninitialized -Wno-sign-compare -DKBUILD_MODFILE='"arch/x86/entry/vdso/vdso-image-64"' -DKBUILD_BASENAME='"vdso_image_64"' -DKBUILD_MODNAME='"vdso_image_64"' -D__KBUILD_MODNAME=kmod_vdso_image_64 -c -o arch/x86/entry/vdso/vdso-image-64.o arch/x86/entry/vdso/vdso-image-64.c
...
and what is missing here is
# cmd_gen_objtooldep arch/x86/lib/vdso-image-64.o
{ echo ; echo 'arch/x86/lib/vdso-image-64.o: $(wildcard ./tools/objtool/objtool)' ; } >> arch/x86/lib/...
or so which needs to create the .return_sites but that thing gets
generated without it:
rm -f arch/x86/entry/vdso/built-in.a; printf "arch/x86/entry/vdso/%s " vma.o extable.o vdso32-setup.o vdso-image-64.o vdso-image-32.o | xargs ar cDPrST arch/x86/entry/vdso/built-in.a
I'd tend to look in Josh's direction as to say what would be the right
thing to do here and more specifically, where?
We need to run objtool on the vdso objects which are *kernel* code.
I.e., that initcall thing. The vdso-image-64.c gets generated by vdso2c
and lands in arch/x86/entry/vdso/vdso-image-64.c, that's why objtool
hasn't seen it yet.
I mean, it is one initcall in the vdso, probably not that important and
if its return hasn't been patched, it won't be the end of the world but
still...
In any case, the patch works as advertized! :-)
Thx.
On Thu, Feb 15, 2024 at 04:53:49PM +0100, Borislav Petkov wrote:
> I'd tend to look in Josh's direction as to say what would be the right
> thing to do here and more specifically, where?
>
> We need to run objtool on the vdso objects which are *kernel* code.
> I.e., that initcall thing. The vdso-image-64.c gets generated by vdso2c
> and lands in arch/x86/entry/vdso/vdso-image-64.c, that's why objtool
> hasn't seen it yet.
>
> I mean, it is one initcall in the vdso, probably not that important and
> if its return hasn't been patched, it won't be the end of the world but
> still...
>
> In any case, the patch works as advertized! :-)
Right, the good news is this isn't a regression and the warning is
working as designed.
This should tell the build to invoke objtool on that file:
diff --git a/arch/x86/entry/vdso/Makefile b/arch/x86/entry/vdso/Makefile
index b1b8dd1608f7..92d67379f570 100644
--- a/arch/x86/entry/vdso/Makefile
+++ b/arch/x86/entry/vdso/Makefile
@@ -36,6 +36,7 @@ UBSAN_SANITIZE_vma.o := y
KCSAN_SANITIZE_vma.o := y
OBJECT_FILES_NON_STANDARD_vma.o := n
OBJECT_FILES_NON_STANDARD_extable.o := n
+OBJECT_FILES_NON_STANDARD_vdso-image-64.o := n
# vDSO images to build
vdso_img-$(VDSO64-y) += 64
On Thu, Feb 15, 2024 at 09:42:35PM -0800, Josh Poimboeuf wrote:
> Right, the good news is this isn't a regression and the warning is
> working as designed.
>
> This should tell the build to invoke objtool on that file:
>
> diff --git a/arch/x86/entry/vdso/Makefile b/arch/x86/entry/vdso/Makefile
> index b1b8dd1608f7..92d67379f570 100644
> --- a/arch/x86/entry/vdso/Makefile
> +++ b/arch/x86/entry/vdso/Makefile
> @@ -36,6 +36,7 @@ UBSAN_SANITIZE_vma.o := y
> KCSAN_SANITIZE_vma.o := y
> OBJECT_FILES_NON_STANDARD_vma.o := n
> OBJECT_FILES_NON_STANDARD_extable.o := n
> +OBJECT_FILES_NON_STANDARD_vdso-image-64.o := n
Right, this should be:
diff --git a/arch/x86/entry/vdso/Makefile b/arch/x86/entry/vdso/Makefile
index c4df99aa1615..4a514cafd73e 100644
--- a/arch/x86/entry/vdso/Makefile
+++ b/arch/x86/entry/vdso/Makefile
@@ -36,6 +36,8 @@ UBSAN_SANITIZE_vma.o := y
KCSAN_SANITIZE_vma.o := y
OBJECT_FILES_NON_STANDARD_vma.o := n
OBJECT_FILES_NON_STANDARD_extable.o := n
+OBJECT_FILES_NON_STANDARD_vdso-image-32.o := n
+OBJECT_FILES_NON_STANDARD_vdso-image-64.o := n
# vDSO images to build
vdso_img-$(VDSO64-y) += 64
for completeness.
Lemme know if you want to write a formal patch or I should.
If you do, please make sure to include the exact way to reproduce
because we might need it in the future.
Thx.
@@ -426,3 +426,63 @@ For 32-bit we have the following conventions - kernel is built with
.endm
#endif /* CONFIG_SMP */
+
+#ifdef CONFIG_X86_64
+
+/* rdi: arg1 ... normal C conventions. rax is saved/restored. */
+.macro THUNK name, func
+SYM_FUNC_START(\name)
+ pushq %rbp
+ movq %rsp, %rbp
+
+ pushq %rdi
+ pushq %rsi
+ pushq %rdx
+ pushq %rcx
+ pushq %rax
+ pushq %r8
+ pushq %r9
+ pushq %r10
+ pushq %r11
+
+ call \func
+
+ popq %r11
+ popq %r10
+ popq %r9
+ popq %r8
+ popq %rax
+ popq %rcx
+ popq %rdx
+ popq %rsi
+ popq %rdi
+ popq %rbp
+ RET
+SYM_FUNC_END(\name)
+ _ASM_NOKPROBE(\name)
+.endm
+
+#else /* CONFIG_X86_32 */
+
+/* put return address in eax (arg1) */
+.macro THUNK name, func, put_ret_addr_in_eax=0
+SYM_CODE_START_NOALIGN(\name)
+ pushl %eax
+ pushl %ecx
+ pushl %edx
+
+ .if \put_ret_addr_in_eax
+ /* Place EIP in the arg1 */
+ movl 3*4(%esp), %eax
+ .endif
+
+ call \func
+ popl %edx
+ popl %ecx
+ popl %eax
+ RET
+ _ASM_NOKPROBE(\name)
+SYM_CODE_END(\name)
+ .endm
+
+#endif
@@ -7,6 +7,8 @@
#include <linux/linkage.h>
#include <asm/msr-index.h>
+#include "calling.h"
+
.pushsection .noinstr.text, "ax"
SYM_FUNC_START(entry_ibpb)
@@ -20,3 +22,5 @@ SYM_FUNC_END(entry_ibpb)
EXPORT_SYMBOL_GPL(entry_ibpb);
.popsection
+
+THUNK warn_thunk_thunk, __warn_thunk
@@ -4,33 +4,15 @@
* Copyright 2008 by Steven Rostedt, Red Hat, Inc
* (inspired by Andi Kleen's thunk_64.S)
*/
- #include <linux/export.h>
- #include <linux/linkage.h>
- #include <asm/asm.h>
- /* put return address in eax (arg1) */
- .macro THUNK name, func, put_ret_addr_in_eax=0
-SYM_CODE_START_NOALIGN(\name)
- pushl %eax
- pushl %ecx
- pushl %edx
+#include <linux/export.h>
+#include <linux/linkage.h>
+#include <asm/asm.h>
- .if \put_ret_addr_in_eax
- /* Place EIP in the arg1 */
- movl 3*4(%esp), %eax
- .endif
+#include "calling.h"
- call \func
- popl %edx
- popl %ecx
- popl %eax
- RET
- _ASM_NOKPROBE(\name)
-SYM_CODE_END(\name)
- .endm
-
- THUNK preempt_schedule_thunk, preempt_schedule
- THUNK preempt_schedule_notrace_thunk, preempt_schedule_notrace
- EXPORT_SYMBOL(preempt_schedule_thunk)
- EXPORT_SYMBOL(preempt_schedule_notrace_thunk)
+THUNK preempt_schedule_thunk, preempt_schedule
+THUNK preempt_schedule_notrace_thunk, preempt_schedule_notrace
+EXPORT_SYMBOL(preempt_schedule_thunk)
+EXPORT_SYMBOL(preempt_schedule_notrace_thunk)
@@ -9,39 +9,6 @@
#include "calling.h"
#include <asm/asm.h>
- /* rdi: arg1 ... normal C conventions. rax is saved/restored. */
- .macro THUNK name, func
-SYM_FUNC_START(\name)
- pushq %rbp
- movq %rsp, %rbp
-
- pushq %rdi
- pushq %rsi
- pushq %rdx
- pushq %rcx
- pushq %rax
- pushq %r8
- pushq %r9
- pushq %r10
- pushq %r11
-
- call \func
-
- popq %r11
- popq %r10
- popq %r9
- popq %r8
- popq %rax
- popq %rcx
- popq %rdx
- popq %rsi
- popq %rdi
- popq %rbp
- RET
-SYM_FUNC_END(\name)
- _ASM_NOKPROBE(\name)
- .endm
-
THUNK preempt_schedule_thunk, preempt_schedule
THUNK preempt_schedule_notrace_thunk, preempt_schedule_notrace
EXPORT_SYMBOL(preempt_schedule_thunk)
@@ -357,6 +357,8 @@ extern void entry_ibpb(void);
extern void (*x86_return_thunk)(void);
+extern void __warn_thunk(void);
+
#ifdef CONFIG_MITIGATION_CALL_DEPTH_TRACKING
extern void call_depth_return_thunk(void);
@@ -2850,3 +2850,8 @@ ssize_t cpu_show_gds(struct device *dev, struct device_attribute *attr, char *bu
return cpu_show_common(dev, attr, buf, X86_BUG_GDS);
}
#endif
+
+void __warn_thunk(void)
+{
+ WARN_ONCE(1, "Unpatched return thunk in use. This should not happen!\n");
+}
@@ -369,19 +369,16 @@ SYM_FUNC_END(call_depth_return_thunk)
* 'JMP __x86_return_thunk' sites are changed to something else by
* apply_returns().
*
- * This should be converted eventually to call a warning function which
- * should scream loudly when the default return thunk is called after
- * alternatives have been applied.
- *
- * That warning function cannot BUG() because the bug splat cannot be
- * displayed in all possible configurations, leading to users not really
- * knowing why the machine froze.
+ * The ALTERNATIVE below adds a really loud warning to catch the case
+ * where the insufficient default return thunk ends up getting used for
+ * whatever reason like miscompilation or failure of
+ * objtool/alternatives/etc to patch all the return sites.
*/
SYM_CODE_START(__x86_return_thunk)
UNWIND_HINT_FUNC
ANNOTATE_NOENDBR
- ANNOTATE_UNRET_SAFE
- ret
+ ALTERNATIVE __stringify(ANNOTATE_UNRET_SAFE; ret), \
+ "jmp warn_thunk_thunk", X86_FEATURE_ALWAYS
int3
SYM_CODE_END(__x86_return_thunk)
EXPORT_SYMBOL(__x86_return_thunk)