From: "Madhavan T. Venkataraman" <madvenka@linux.microsoft.com>
To: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>,
mark.rutland@arm.com, broonie@kernel.org, jthierry@redhat.com,
catalin.marinas@arm.com, will@kernel.org,
linux-arm-kernel@lists.infradead.org,
live-patching@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [RFC PATCH v1 0/4] arm64: Implement stack trace reliability checks
Date: Mon, 5 Apr 2021 12:12:08 -0500 [thread overview]
Message-ID: <380d0437-e205-5eab-3664-f17aa9adc3eb@linux.microsoft.com> (raw)
In-Reply-To: <7dda9af3-1ecf-5e6f-1e46-8870a2a5e550@linux.microsoft.com>
On 4/5/21 9:56 AM, Madhavan T. Venkataraman wrote:
>
>
> On 4/5/21 8:24 AM, Masami Hiramatsu wrote:
>> Hi Madhaven,
>>
>> On Sat, 3 Apr 2021 22:29:12 -0500
>> "Madhavan T. Venkataraman" <madvenka@linux.microsoft.com> wrote:
>>
>>
>>>>> Check for kretprobe
>>>>> ===================
>>>>>
>>>>> For functions with a kretprobe set up, probe code executes on entry
>>>>> to the function and replaces the return address in the stack frame with a
>>>>> kretprobe trampoline. Whenever the function returns, control is
>>>>> transferred to the trampoline. The trampoline eventually returns to the
>>>>> original return address.
>>>>>
>>>>> A stack trace taken while executing in the function (or in functions that
>>>>> get called from the function) will not show the original return address.
>>>>> Similarly, a stack trace taken while executing in the trampoline itself
>>>>> (and functions that get called from the trampoline) will not show the
>>>>> original return address. This means that the caller of the probed function
>>>>> will not show. This makes the stack trace unreliable.
>>>>>
>>>>> Add the kretprobe trampoline to special_functions[].
>>>>>
>>>>> FYI, each task contains a task->kretprobe_instances list that can
>>>>> theoretically be consulted to find the orginal return address. But I am
>>>>> not entirely sure how to safely traverse that list for stack traces
>>>>> not on the current process. So, I have taken the easy way out.
>>>>
>>>> For kretprobes, unwinding from the trampoline or kretprobe handler
>>>> shouldn't be a reliability concern for live patching, for similar
>>>> reasons as above.
>>>>
>>>
>>> Please see previous answer.
>>>
>>>> Otherwise, when unwinding from a blocked task which has
>>>> 'kretprobe_trampoline' on the stack, the unwinder needs a way to get the
>>>> original return address. Masami has been working on an interface to
>>>> make that possible for x86. I assume something similar could be done
>>>> for arm64.
>>>>
>>>
>>> OK. Until that is available, this case needs to be addressed.
>>
>> Actually, I've done that on arm64 :) See below patch.
>> (and I also have a similar code for arm32, what I'm considering is how
>> to unify x86/arm/arm64 kretprobe_find_ret_addr(), since those are very
>> similar.)
>>
>> This is applicable on my x86 series v5
>>
>> https://lore.kernel.org/bpf/161676170650.330141.6214727134265514123.stgit@devnote2/
>>
>> Thank you,
>>
>>
>
> I took a brief look at your changes. Looks reasonable.
>
> However, for now, I am going to include the kretprobe_trampoline in the special_functions[]
> array until your changes are merged. At that point, it is just a matter of deleting
> kretprobe_trampoline from the special_functions[] array. That is all.
>
> I hope that is fine with everyone.
>
Actually, there may still be a problem to solve.
If arch_stack_walk_reliable() is ever called from within kretprobe_trampoline() for debugging or
other purposes after the instance is deleted from the task instance list, it would not be able
to retrieve the original return address.
The stack trace would be unreliable in that case, would it not?
Madhavan
next prev parent reply other threads:[~2021-04-05 17:12 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <77bd5edeea72d44533c769b1e8c0fea7a9d7eb3a>
2021-03-30 19:09 ` [RFC PATCH v1 0/4] arm64: Implement stack trace reliability checks madvenka
2021-03-30 19:09 ` [RFC PATCH v1 1/4] arm64: Implement infrastructure for " madvenka
2021-04-01 15:27 ` Mark Brown
2021-04-01 17:44 ` Madhavan T. Venkataraman
2021-03-30 19:09 ` [RFC PATCH v1 2/4] arm64: Mark a stack trace unreliable if an EL1 exception frame is detected madvenka
2021-04-01 17:21 ` Mark Brown
2021-03-30 19:09 ` [RFC PATCH v1 3/4] arm64: Detect FTRACE cases that make the stack trace unreliable madvenka
2021-04-01 14:27 ` Mark Brown
2021-04-01 17:43 ` Madhavan T. Venkataraman
2021-04-01 18:28 ` Mark Brown
2021-04-01 18:40 ` Madhavan T. Venkataraman
2021-04-01 18:53 ` Madhavan T. Venkataraman
2021-04-01 19:47 ` Madhavan T. Venkataraman
2021-04-06 11:02 ` Mark Brown
2021-04-01 17:48 ` Madhavan T. Venkataraman
2021-03-30 19:09 ` [RFC PATCH v1 4/4] arm64: Mark stack trace as unreliable if kretprobed functions are present madvenka
2021-04-01 17:23 ` Mark Brown
2021-04-03 17:01 ` [RFC PATCH v1 0/4] arm64: Implement stack trace reliability checks Josh Poimboeuf
2021-04-04 3:29 ` Madhavan T. Venkataraman
2021-04-05 13:24 ` Masami Hiramatsu
2021-04-05 13:46 ` Madhavan T. Venkataraman
2021-04-05 14:56 ` Madhavan T. Venkataraman
2021-04-05 17:12 ` Madhavan T. Venkataraman [this message]
2021-04-05 23:39 ` Masami Hiramatsu
2021-04-05 23:40 ` Masami Hiramatsu
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=380d0437-e205-5eab-3664-f17aa9adc3eb@linux.microsoft.com \
--to=madvenka@linux.microsoft.com \
--cc=broonie@kernel.org \
--cc=catalin.marinas@arm.com \
--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=mhiramat@kernel.org \
--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: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).