Hi Josh, Thanks for the review! On Tue, Mar 18, 2025 at 11:45 AM Josh Poimboeuf <jpoimboe@xxxxxxxxxx> wrote: > > On Fri, Mar 07, 2025 at 05:27:41PM -0800, Song Liu wrote: > > With proper exception boundary detection, it is possible to implment > > arch_stack_walk_reliable without sframe. > > > > Note that, arch_stack_walk_reliable does not guarantee getting reliable > > stack in all scenarios. Instead, it can reliably detect when the stack > > trace is not reliable, which is enough to provide reliable livepatching. > > > > This version has been inspired by Weinan Liu's patch [1]. > > > > [1] https://lore.kernel.org/live-patching/20250127213310.2496133-7-wnliu@xxxxxxxxxx/ > > Signed-off-by: Song Liu <song@xxxxxxxxxx> > > This looks incomplete. The reliable unwinder needs to be extra > paranoid. There are several already-checked-for errors in the unwinder > that don't actually set the unreliable bit. > > There are likely other failure modes it should also be checking for. > For example I don't see where it confirms that the unwind completed to > the end of the stack (which is typically at a certain offset). If I understand the comment correctly, this should be handled by the meta data type FRAME_META_TYPE_FINAL. > > See for example all the error conditions in the x86 version of > arch_stack_walk_reliable() and in arch/x86/kernel/unwind_frame.c. I guess I missed this part: diff --git i/arch/arm64/kernel/stacktrace.c w/arch/arm64/kernel/stacktrace.c index 69d0567a0c38..3bb8e3ea4c4b 100644 --- i/arch/arm64/kernel/stacktrace.c +++ w/arch/arm64/kernel/stacktrace.c @@ -268,6 +268,8 @@ kunwind_next(struct kunwind_state *state) case KUNWIND_SOURCE_TASK: case KUNWIND_SOURCE_REGS_PC: err = kunwind_next_frame_record(state); + if (err && err != -ENOENT) + state->common.unreliable = true; break; default: err = -EINVAL; With this part, we should cover all these cases? Did I miss something else? Thanks, Song
