From: "Madhavan T. Venkataraman" <madvenka@linux.microsoft.com> To: Josh Poimboeuf <jpoimboe@redhat.com> Cc: broonie@kernel.org, mark.rutland@arm.com, jthierry@redhat.com, catalin.marinas@arm.com, will@kernel.org, jmorris@namei.org, pasha.tatashin@soleen.com, linux-arm-kernel@lists.infradead.org, live-patching@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [RFC PATCH v3 1/4] arm64: Introduce stack trace reliability checks in the unwinder Date: Tue, 4 May 2021 19:21:37 -0500 [thread overview] Message-ID: <2fc9a77a-ddf5-812a-0681-ece94b433d71@linux.microsoft.com> (raw) In-Reply-To: <20210505000728.yxg3xbwa3emcu2wi@treble> On 5/4/21 7:07 PM, Josh Poimboeuf wrote: > On Tue, May 04, 2021 at 06:13:39PM -0500, Madhavan T. Venkataraman wrote: >> >> >> On 5/4/21 4:52 PM, Josh Poimboeuf wrote: >>> On Mon, May 03, 2021 at 12:36:12PM -0500, madvenka@linux.microsoft.com wrote: >>>> @@ -44,6 +44,8 @@ int notrace unwind_frame(struct task_struct *tsk, struct stackframe *frame) >>>> unsigned long fp = frame->fp; >>>> struct stack_info info; >>>> >>>> + frame->reliable = true; >>>> + >>> >>> Why set 'reliable' to true on every invocation of unwind_frame()? >>> Shouldn't it be remembered across frames? >>> >> >> This is mainly for debug purposes in case a caller wants to print the whole stack and also >> print which functions are unreliable. For livepatch, it does not make any difference. It will >> quit as soon as it encounters an unreliable frame. > > Hm, ok. So 'frame->reliable' refers to the current frame, not the > entire stack. > Yes. >>> Also, it looks like there are several error scenarios where it returns >>> -EINVAL but doesn't set 'reliable' to false. >>> >> >> I wanted to make a distinction between an error situation (like stack corruption where unwinding >> has to stop) and an unreliable situation (where unwinding can still proceed). E.g., when a >> stack trace is taken for informational purposes or debug purposes, the unwinding will try to >> proceed until either the stack trace ends or an error happens. > > Ok, but I don't understand how that relates to my comment. > > Why wouldn't a stack corruption like !on_accessible_stack() set > 'frame->reliable' to false? > I do see your point. If an error has been hit, then the stack trace is essentially unreliable regardless of anything else. So, I accept your comment. I will mark the stack trace as unreliable if any kind of error is encountered. Thanks! Madhavan
WARNING: multiple messages have this Message-ID (diff)
From: "Madhavan T. Venkataraman" <madvenka@linux.microsoft.com> To: Josh Poimboeuf <jpoimboe@redhat.com> Cc: broonie@kernel.org, mark.rutland@arm.com, jthierry@redhat.com, catalin.marinas@arm.com, will@kernel.org, jmorris@namei.org, pasha.tatashin@soleen.com, linux-arm-kernel@lists.infradead.org, live-patching@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [RFC PATCH v3 1/4] arm64: Introduce stack trace reliability checks in the unwinder Date: Tue, 4 May 2021 19:21:37 -0500 [thread overview] Message-ID: <2fc9a77a-ddf5-812a-0681-ece94b433d71@linux.microsoft.com> (raw) In-Reply-To: <20210505000728.yxg3xbwa3emcu2wi@treble> On 5/4/21 7:07 PM, Josh Poimboeuf wrote: > On Tue, May 04, 2021 at 06:13:39PM -0500, Madhavan T. Venkataraman wrote: >> >> >> On 5/4/21 4:52 PM, Josh Poimboeuf wrote: >>> On Mon, May 03, 2021 at 12:36:12PM -0500, madvenka@linux.microsoft.com wrote: >>>> @@ -44,6 +44,8 @@ int notrace unwind_frame(struct task_struct *tsk, struct stackframe *frame) >>>> unsigned long fp = frame->fp; >>>> struct stack_info info; >>>> >>>> + frame->reliable = true; >>>> + >>> >>> Why set 'reliable' to true on every invocation of unwind_frame()? >>> Shouldn't it be remembered across frames? >>> >> >> This is mainly for debug purposes in case a caller wants to print the whole stack and also >> print which functions are unreliable. For livepatch, it does not make any difference. It will >> quit as soon as it encounters an unreliable frame. > > Hm, ok. So 'frame->reliable' refers to the current frame, not the > entire stack. > Yes. >>> Also, it looks like there are several error scenarios where it returns >>> -EINVAL but doesn't set 'reliable' to false. >>> >> >> I wanted to make a distinction between an error situation (like stack corruption where unwinding >> has to stop) and an unreliable situation (where unwinding can still proceed). E.g., when a >> stack trace is taken for informational purposes or debug purposes, the unwinding will try to >> proceed until either the stack trace ends or an error happens. > > Ok, but I don't understand how that relates to my comment. > > Why wouldn't a stack corruption like !on_accessible_stack() set > 'frame->reliable' to false? > I do see your point. If an error has been hit, then the stack trace is essentially unreliable regardless of anything else. So, I accept your comment. I will mark the stack trace as unreliable if any kind of error is encountered. Thanks! Madhavan _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2021-05-05 0:21 UTC|newest] Thread overview: 64+ messages / expand[flat|nested] mbox.gz Atom feed top [not found] <65cf4dfbc439b010b50a0c46ec500432acde86d6> 2021-05-03 17:36 ` [RFC PATCH v3 0/4] arm64: Stack trace reliability checks in the unwinder madvenka 2021-05-03 17:36 ` madvenka 2021-05-03 17:36 ` [RFC PATCH v3 1/4] arm64: Introduce stack " madvenka 2021-05-03 17:36 ` madvenka 2021-05-04 15:50 ` Mark Brown 2021-05-04 15:50 ` Mark Brown 2021-05-04 19:14 ` Madhavan T. Venkataraman 2021-05-04 19:14 ` Madhavan T. Venkataraman 2021-05-04 21:52 ` Josh Poimboeuf 2021-05-04 21:52 ` Josh Poimboeuf 2021-05-04 23:13 ` Madhavan T. Venkataraman 2021-05-04 23:13 ` Madhavan T. Venkataraman 2021-05-05 0:07 ` Josh Poimboeuf 2021-05-05 0:07 ` Josh Poimboeuf 2021-05-05 0:21 ` Madhavan T. Venkataraman [this message] 2021-05-05 0:21 ` Madhavan T. Venkataraman 2021-05-03 17:36 ` [RFC PATCH v3 2/4] arm64: Check the return PC against unreliable code sections madvenka 2021-05-03 17:36 ` madvenka 2021-05-04 16:05 ` Mark Brown 2021-05-04 16:05 ` Mark Brown 2021-05-04 19:03 ` Madhavan T. Venkataraman 2021-05-04 19:03 ` Madhavan T. Venkataraman 2021-05-04 19:32 ` Madhavan T. Venkataraman 2021-05-04 19:32 ` Madhavan T. Venkataraman 2021-05-05 16:46 ` Mark Brown 2021-05-05 16:46 ` Mark Brown 2021-05-05 18:48 ` Madhavan T. Venkataraman 2021-05-05 18:48 ` Madhavan T. Venkataraman 2021-05-05 18:50 ` Madhavan T. Venkataraman 2021-05-05 18:50 ` Madhavan T. Venkataraman 2021-05-06 13:45 ` Mark Brown 2021-05-06 13:45 ` Mark Brown 2021-05-06 15:21 ` Madhavan T. Venkataraman 2021-05-06 15:21 ` Madhavan T. Venkataraman 2021-05-05 16:34 ` Mark Brown 2021-05-05 16:34 ` Mark Brown 2021-05-05 17:51 ` Madhavan T. Venkataraman 2021-05-05 17:51 ` Madhavan T. Venkataraman 2021-05-05 19:30 ` Ard Biesheuvel 2021-05-05 19:30 ` Ard Biesheuvel 2021-05-05 20:00 ` Madhavan T. Venkataraman 2021-05-05 20:00 ` Madhavan T. Venkataraman 2021-05-03 17:36 ` [RFC PATCH v3 3/4] arm64: Handle miscellaneous functions in .text and .init.text madvenka 2021-05-03 17:36 ` madvenka 2021-05-06 14:12 ` Mark Brown 2021-05-06 14:12 ` Mark Brown 2021-05-06 15:30 ` Madhavan T. Venkataraman 2021-05-06 15:30 ` Madhavan T. Venkataraman 2021-05-06 15:32 ` Madhavan T. Venkataraman 2021-05-06 15:32 ` Madhavan T. Venkataraman 2021-05-06 15:44 ` Mark Brown 2021-05-06 15:44 ` Mark Brown 2021-05-06 15:56 ` Madhavan T. Venkataraman 2021-05-06 15:56 ` Madhavan T. Venkataraman 2021-05-06 15:37 ` Mark Brown 2021-05-06 15:37 ` Mark Brown 2021-05-06 15:57 ` Madhavan T. Venkataraman 2021-05-06 15:57 ` Madhavan T. Venkataraman 2021-05-03 17:36 ` [RFC PATCH v3 4/4] arm64: Handle funtion graph tracer better in the unwinder madvenka 2021-05-03 17:36 ` madvenka 2021-05-06 14:43 ` Mark Brown 2021-05-06 14:43 ` Mark Brown 2021-05-06 15:20 ` Madhavan T. Venkataraman 2021-05-06 15:20 ` Madhavan T. Venkataraman
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=2fc9a77a-ddf5-812a-0681-ece94b433d71@linux.microsoft.com \ --to=madvenka@linux.microsoft.com \ --cc=broonie@kernel.org \ --cc=catalin.marinas@arm.com \ --cc=jmorris@namei.org \ --cc=jpoimboe@redhat.com \ --cc=jthierry@redhat.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=live-patching@vger.kernel.org \ --cc=mark.rutland@arm.com \ --cc=pasha.tatashin@soleen.com \ --cc=will@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.