[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <mb61pwme55kuw.fsf@kernel.org>
Date: Tue, 04 Feb 2025 23:52:07 +0000
From: Puranjay Mohan <puranjay@...nel.org>
To: Weinan Liu <wnliu@...gle.com>, Josh Poimboeuf <jpoimboe@...nel.org>,
Steven Rostedt <rostedt@...dmis.org>, Indu Bhagat
<indu.bhagat@...cle.com>, Peter Zijlstra <peterz@...radead.org>
Cc: Mark Rutland <mark.rutland@....com>, roman.gushchin@...ux.dev, "Will
Deacon" <will@...nel.org>, Ian Rogers <irogers@...gle.com>,
linux-toolchains@...r.kernel.org, linux-kernel@...r.kernel.org,
live-patching@...r.kernel.org, joe.lawrence@...hat.com,
linux-arm-kernel@...ts.infradead.org, Weinan Liu <wnliu@...gle.com>
Subject: Re: [PATCH 0/8] unwind, arm64: add sframe unwinder for kernel
Puranjay Mohan <puranjay@...nel.org> writes:
> Weinan Liu <wnliu@...gle.com> writes:
>
>> This patchset implements a generic kernel sframe-based [1] unwinder.
>> The main goal is to support reliable stacktraces on arm64.
>>
>> On x86 orc unwinder provides reliable stacktraces. But arm64 misses the
>> required support from objtool: it cannot generate orc unwind tables for
>> arm64.
>>
>> Currently, there's already a sframe unwinder proposed for userspace: [2].
>> Since the sframe unwind table algorithm is similar, these two proposal
>> could integrate common functionality in the future.
>>
>> There are some incomplete features or challenges:
>> - The unwinder doesn't yet work with kernel modules. The `start_addr` of
>> FRE from kernel modules doesn't appear correct, preventing us from
>> unwinding functions from kernel modules.
>> - Currently, only GCC supports sframe.
>>
>> Ref:
>> [1]: https://sourceware.org/binutils/docs/sframe-spec.html
>> [2]: https://lore.kernel.org/lkml/cover.1730150953.git.jpoimboe@kernel.org/
>>
>
> Hi Weinan,
> Thanks for working on this.
>
> I tested this set on my setup and faced some issues, here are the
> details:
>
> Here is my setup [on AWS c6gd.16xlarge instance]:
> -------------------------------------------------
>
> [root@...172-31-32-86 linux-upstream]# uname -a
> Linux ip-172-31-32-86.ec2.internal 6.14.0-rc1+ #1 SMP Tue Feb 4 14:15:55 UTC 2025 aarch64 aarch64 aarch64 GNU/Linux
>
> [root@...172-31-32-86 linux-upstream]# git log --oneline
> e9a702365 (HEAD -> master) arm64: Enable livepatch for ARM64
> 5dedc956e arm64: Define TIF_PATCH_PENDING for livepatch
> ba563b31a unwind: arm64: add reliable stacktrace support for arm64
> d807d392d unwind: arm64: Add sframe unwinder on arm64
> 7872f050b unwind: Implement generic sframe unwinder library
> 03d2ad003 unwind: add sframe v2 header
> 5e95cc051 arm64: entry: add unwind info for various kernel entries
> faff6cbc3 unwind: build kernel with sframe info
> 0de63bb7d (origin/master, origin/HEAD) Merge tag 'pull-fix' of git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs
> 902e09c8a fix braino in "9p: fix ->rename_sem exclusion"
> f286757b6 Merge tag 'timers-urgent-2025-02-03' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip
> a360f3ffd (grafted) Merge tag 'irq-urgent-2025-02-03' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip
> bb2784d9a (grafted) jiffies: Cast to unsigned long in secs_to_jiffies() conversion
> 30d61efe1 (grafted) 9p: fix ->rename_sem exclusion
>
> [root@...172-31-32-86 linux-upstream]# grep SFRAME .config
> CONFIG_AS_HAS_SFRAME_SUPPORT=y
> CONFIG_SFRAME_UNWIND_TABLE=y
> CONFIG_SFRAME_UNWINDER=y
> [root@...172-31-32-86 linux-upstream]# grep LIVEPATCH .config
> CONFIG_HAVE_LIVEPATCH=y
> CONFIG_LIVEPATCH=y
> CONFIG_SAMPLE_LIVEPATCH=m
>
> [root@...172-31-32-86 linux-upstream]# as --version
> GNU assembler version 2.41-50.al2023.0.2
> Copyright (C) 2023 Free Software Foundation, Inc.
> This program is free software; you may redistribute it under the terms of
> the GNU General Public License version 3 or later.
> This program has absolutely no warranty.
> This assembler was configured for a target of `aarch64-amazon-linux'.
>
> [root@...172-31-32-86 linux-upstream]# gcc --version
> gcc (GCC) 11.4.1 20230605 (Red Hat 11.4.1-2)
> Copyright (C) 2021 Free Software Foundation, Inc.
> This is free software; see the source for copying conditions. There is NO
> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
>
> Loading the livepatch-sameple module:
> -------------------------------------
>
> [root@...172-31-32-86 linux-upstream]# kpatch load /lib/modules/6.14.0-rc1+/kernel/samples/livepatch/livepatch-sample.ko
> loading patch module: /lib/modules/6.14.0-rc1+/kernel/samples/livepatch/livepatch-sample.ko
> waiting (up to 15 seconds) for patch transition to complete...
> patch transition has stalled!
> <4>kpatch: Livepatch process signaling is performed automatically on your system.
> <4>kpatch: Skipping manual process signaling.
> waiting (up to 60 seconds) for patch transition to complete...
>
> Stalled processes:
> 340 kdevtmpfs
> stack:
> [<0>] devtmpfs_work_loop+0x2cc/0x2d8
> [<0>] devtmpfsd+0x4c/0x58
> [<0>] kthread+0xf0/0x100
> [<0>] ret_from_fork+0x10/0x20
> module livepatch_sample did not complete its transition, unloading...
> disabling patch module: livepatch_sample
> waiting (up to 15 seconds) for patch transition to complete...
> transition complete (3 seconds)
> unloading patch module: livepatch_sample
> <4>kpatch: error: failed to load module livepatch_sample (transition stalled)
After some debugging this is what I found:
devtmpfsd() calls devtmpfs_work_loop() which is marked '__noreturn' and has an
infinite loop. The compiler puts the `bl` to devtmpfs_work_loop() as the the
last instruction in devtmpfsd() and therefore on entry to devtmpfs_work_loop(),
LR points to an instruction beyond devtmpfsd() and this consfuses the unwinder.
ffff800080d9a070 <devtmpfsd>:
ffff800080d9a070: d503201f nop
ffff800080d9a074: d503201f nop
ffff800080d9a078: d503233f paciasp
ffff800080d9a07c: a9be7bfd stp x29, x30, [sp, #-32]!
ffff800080d9a080: 910003fd mov x29, sp
ffff800080d9a084: f9000bf3 str x19, [sp, #16]
ffff800080d9a088: 943378e8 bl ffff800081a78428 <devtmpfs_setup>
ffff800080d9a08c: 90006ca1 adrp x1, ffff800081b2e000 <unique_processor_ids+0x3758>
ffff800080d9a090: 2a0003f3 mov w19, w0
ffff800080d9a094: 912de021 add x1, x1, #0xb78
ffff800080d9a098: 91002020 add x0, x1, #0x8
ffff800080d9a09c: 97cd2a43 bl ffff8000800e49a8 <complete>
ffff800080d9a0a0: 340000d3 cbz w19, ffff800080d9a0b8 <devtmpfsd+0x48>
ffff800080d9a0a4: 2a1303e0 mov w0, w19
ffff800080d9a0a8: f9400bf3 ldr x19, [sp, #16]
ffff800080d9a0ac: a8c27bfd ldp x29, x30, [sp], #32
ffff800080d9a0b0: d50323bf autiasp
ffff800080d9a0b4: d65f03c0 ret
ffff800080d9a0b8: 97f06526 bl ffff8000809b3550 <devtmpfs_work_loop>
ffff800080d9a0bc: 00000000 udf #0
ffff800080d9a0c0: d503201f nop
ffff800080d9a0c4: d503201f nop
find_fde() got pc=0xffff800080d9a0bc which is not in [sfde_func_start_address, sfde_func_size)
output for readelf --sframe for devtmpfsd()
func idx [51825]: pc = 0xffff800080d9a070, size = 76 bytes
STARTPC CFA FP RA
ffff800080d9a070 sp+0 u u
ffff800080d9a07c sp+0 u u[s]
ffff800080d9a080 sp+32 c-32 c-24[s]
ffff800080d9a0b0 sp+0 u u[s]
ffff800080d9a0b4 sp+0 u u
ffff800080d9a0b8 sp+32 c-32 c-24[s]
The unwinder and all the related infra is assuming that the return address
will be part of a valid function which is not the case here.
I am not sure which component needs to be fixed here, but the following
patch(which is a hack) fixes the issue by considering the return address as
part of the function descriptor entry.
-- 8< --
diff --git a/kernel/sframe_lookup.c b/kernel/sframe_lookup.c
index 846f1da95..28bec5064 100644
--- a/kernel/sframe_lookup.c
+++ b/kernel/sframe_lookup.c
@@ -82,7 +82,7 @@ static struct sframe_fde *find_fde(const struct sframe_table *tbl, unsigned long
if (f >= tbl->sfhdr_p->num_fdes || f < 0)
return NULL;
fdep = tbl->fde_p + f;
- if (ip < fdep->start_addr || ip >= fdep->start_addr + fdep->size)
+ if (ip < fdep->start_addr || ip > fdep->start_addr + fdep->size)
return NULL;
return fdep;
@@ -106,7 +106,7 @@ static int find_fre(const struct sframe_table *tbl, unsigned long pc,
else
ip_off = (int32_t)(pc - (unsigned long)tbl->sfhdr_p) - fdep->start_addr;
- if (ip_off < 0 || ip_off >= fdep->size)
+ if (ip_off < 0 || ip_off > fdep->size)
return -EINVAL;
/*
-- >8 --
Thanks,
Puranjay
Download attachment "signature.asc" of type "application/pgp-signature" (256 bytes)
Powered by blists - more mailing lists