linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: wcohen@redhat.com (William Cohen)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v6 0/6] arm64: Add kernel probes (kprobes) support
Date: Tue, 05 May 2015 17:02:41 -0400	[thread overview]
Message-ID: <55492FF1.6040907@redhat.com> (raw)
In-Reply-To: <20150505154830.GI1550@arm.com>

On 05/05/2015 11:48 AM, Will Deacon wrote:
> On Tue, May 05, 2015 at 06:14:51AM +0100, David Long wrote:
>> On 05/01/15 21:44, William Cohen wrote:
>>> Dave Long and I did some additional experimentation to better
>>> understand what is condition causes the kernel to sometimes spew:
>>>
>>> Unexpected kernel single-step exception at EL1
>>>
>>> The functioncallcount.stp test instruments the entry and return of
>>> every function in the mm files, including kfree.  In most cases the
>>> arm64 trampoline_probe_handler just determines which return probe
>>> instance matches the current conditions, runs the associated handler,
>>> and recycles the return probe instance for another use by placing it
>>> on a hlist.  However, it is possible that a return probe instance has
>>> been set up on function entry and the return probe is unregistered
>>> before the return probe instance fires.  In this case kfree is called
>>> by the trampoline handler to remove the return probe instances related
>>> to the unregistered kretprobe.  This case where the the kprobed kfree
>>> is called within the arm64 trampoline_probe_handler function trigger
>>> the problem.
>>>
>>> The kprobe breakpoint for the kfree call from within the
>>> trampoline_probe_handler is encountered and started, but things go
>>> wrong when attempting the single step on the instruction.
>>>
>>> It took a while to trigger this problem with the sytemtap testsuite.
>>> Dave Long came up with steps that reproduce this more quickly with a
>>> probed function that is always called within the trampoline handler.
>>> Trying the same on x86_64 doesn't trigger the problem.  It appears
>>> that the x86_64 code can handle a single step from within the
>>> trampoline_handler.
>>>
>>
>> I'm assuming there are no plans for supporting software breakpoint debug 
>> exceptions during processing of single-step exceptions, any time soon on 
>> arm64.  Given that the only solution that I can come with for this is 
>> instead of making this orphaned kretprobe instance list exist only 
>> temporarily (in the scope of the kretprobe trampoline handler), make it 
>> always exist and kfree any items found on it as part of a periodic 
>> cleanup running outside of the handler context.  I think these changes 
>> would still all be in archiecture-specific code.  This doesn't feel to 
>> me like a bad solution.  Does anyone think there is a simpler way out of 
>> this?
> 
> Just to clarify, is the problem here the software breakpoint exception,
> or trying to step the faulting instruction whilst we were already handling
> a step?
> 
> I think I'd be inclined to keep the code run in debug context to a minimum.
> We already can't block there, and the more code we add the more black spots
> we end up with in the kernel itself. The alternative would be to make your
> kprobes code re-entrant, but that sounds like a nightmare.
> 
> You say this works on x86. How do they handle it? Is the nested probe
> on kfree ignored or handled?
> 
> Will
> 

Hi Dave and Will,

The attached patch attempts to eliminate the need for the breakpoint in the trampoline.  It is modeled after the x86_64 code and just saves the register state, calls the trampoline handler, and then fixes the return address.  The code compiles, but I have NOT verified that it works. It looks feasible to do things this way.  In addition to avoiding the possible issue with a kretprobe on kfree it would also make the kretprobes faster because it would avoid the breakpoint exception and the associated kprobe handling in the trampoline.

-Will
-------------- next part --------------
A non-text attachment was scrubbed...
Name: avoid_bkpt_trampoline.diff
Type: text/x-patch
Size: 4465 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150505/1ebbc086/attachment-0001.bin>

  parent reply	other threads:[~2015-05-05 21:02 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-04-20 20:19 [PATCH v6 0/6] arm64: Add kernel probes (kprobes) support David Long
2015-04-20 20:19 ` [PATCH v6 1/6] arm64: Add HAVE_REGS_AND_STACK_ACCESS_API feature David Long
2015-05-20 13:39   ` Catalin Marinas
2015-05-21  3:29     ` David Long
2015-05-21 17:55       ` Catalin Marinas
2015-05-22 17:05         ` David Long
2015-04-20 20:19 ` [PATCH v6 2/6] arm64: Add more test functions to insn.c David Long
2015-04-20 20:19 ` [PATCH v6 3/6] arm64: Kprobes with single stepping support David Long
2015-05-20 16:39   ` Catalin Marinas
2015-05-21  4:44     ` David Long
2015-05-22 11:00       ` Catalin Marinas
2015-05-22 15:49         ` William Cohen
2015-05-22 16:54           ` Catalin Marinas
2015-05-22 16:57             ` David Long
2015-04-20 20:19 ` [PATCH v6 4/6] arm64: kprobes instruction simulation support David Long
2015-04-20 20:19 ` [PATCH v6 5/6] arm64: Add kernel return probes support (kretprobes) David Long
2015-04-20 20:19 ` [PATCH v6 6/6] kprobes: Add arm64 case in kprobe example module David Long
2015-04-21 11:42 ` [PATCH v6 0/6] arm64: Add kernel probes (kprobes) support Masami Hiramatsu
2015-04-21 14:07   ` William Cohen
2015-04-24 21:14   ` William Cohen
2015-04-28  2:58     ` William Cohen
2015-04-29 10:23       ` Will Deacon
2015-05-02  1:44         ` William Cohen
2015-05-05  5:14           ` David Long
2015-05-05 15:48             ` Will Deacon
2015-05-05 16:18               ` William Cohen
2015-05-05 21:02               ` William Cohen [this message]
2015-05-06  3:14                 ` William Cohen
2015-05-12  5:54               ` David Long
2015-05-12 12:48                 ` William Cohen
2015-05-13  9:22                   ` Masami Hiramatsu
2015-05-13 15:41                     ` William Cohen
2015-05-13 19:58                       ` David Long
2015-05-13 20:35                         ` William Cohen
2015-05-14  0:01                       ` Masami Hiramatsu
2015-05-14  3:48                         ` Ananth N Mavinakayanahalli
2015-04-29  4:33   ` David Long

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=55492FF1.6040907@redhat.com \
    --to=wcohen@redhat.com \
    --cc=linux-arm-kernel@lists.infradead.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 NNTP newsgroup(s).