From: dave.long@linaro.org (David Long)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v4 15/16] ARM: add uprobes support
Date: Tue, 21 Jan 2014 11:51:22 -0500 [thread overview]
Message-ID: <52DEA58A.9050406@linaro.org> (raw)
In-Reply-To: <1387564464.3404.106.camel@linaro1.home>
On 12/20/13 13:34, Jon Medhurst (Tixy) wrote:
> On Sun, 2013-12-15 at 23:08 -0500, David Long wrote:
>> From: "David A. Long" <dave.long@linaro.org>
>>
>> Using Rabin Vincent's ARM uprobes patches as a base, enable uprobes
>> support on ARM.
>>
>> Caveats:
>>
>> - Thumb is not supported
>> - XOL abort/trap handling is not implemented
>
> I shall repeat my comment from version one of the patch...
>
> What are the consequences of this, e.g. is it possible for a probe to
> get stuck in an infinite loop of faulting? I hope there are no integrity
> issues for the kernel itself.
>
> Would be good if someone familiar with uprobes working could answer
> that.
Testing shows it does indeed get stuck continuously trapping. The
process is killable. Fortunately all the infrastructure is already
there for fixing this. I've patched the code to detect the trap and
allow it to be processed, using the powerpc uprobes code as a model.
The changes required are fairly small and entirely in the
architecture-specific code.
As mentioned before, thumb support is a follow-on project.
> I've a few other comments...
[snip]
>
>
>> +const union decode_item uprobes_probes_actions[] = {
>> + [PROBES_EMULATE_NONE] {.handler = probes_simulate_nop},
>
> There is a missing '=' in the line above. Interesting that GCC doesn't
> complain (I tried compiling this patch and it didn't).
>
That is indeed odd. I have fixed it (my code, not the compiler).
[snip]
>> +bool arch_uprobe_skip_sstep(struct arch_uprobe *auprobe, struct pt_regs *regs)
>> +{
>> + void *addr;
>
> 'addr' is not used so this line can be deleted
>
>> + probes_opcode_t opcode;
>> +
>> + if (!auprobe->simulate)
>> + return false;
>> +
>> + addr = (void *) regs->ARM_pc;
>
> and so can this line ^^^
Fixed.
>
>> + opcode = __mem_to_opcode_arm(*(unsigned int *) auprobe->insn);
>> +
>> + auprobe->asi.insn_singlestep(opcode, &auprobe->asi, regs);
>> +
>> + return true;
>> +}
>> +
>
Fixed.
> [rest of patch snipped]
>
-dl
WARNING: multiple messages have this Message-ID (diff)
From: David Long <dave.long@linaro.org>
To: "Jon Medhurst (Tixy)" <tixy@linaro.org>
Cc: linux-arm-kernel@lists.infradead.org,
Russell King <linux@arm.linux.org.uk>,
Rabin Vincent <rabin@rab.in>, Oleg Nesterov <oleg@redhat.com>,
Srikar Dronamraju <srikar@linux.vnet.ibm.com>,
Ingo Molnar <mingo@redhat.com>,
Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>,
Ananth N Mavinakayanahalli <ananth@in.ibm.com>,
Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com>,
davem@davemloft.net, Peter Zijlstra <a.p.zijlstra@chello.nl>,
Paul Mackerras <paulus@samba.org>,
Arnaldo Carvalho de Melo <acme@ghostprotocols.net>,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH v4 15/16] ARM: add uprobes support
Date: Tue, 21 Jan 2014 11:51:22 -0500 [thread overview]
Message-ID: <52DEA58A.9050406@linaro.org> (raw)
In-Reply-To: <1387564464.3404.106.camel@linaro1.home>
On 12/20/13 13:34, Jon Medhurst (Tixy) wrote:
> On Sun, 2013-12-15 at 23:08 -0500, David Long wrote:
>> From: "David A. Long" <dave.long@linaro.org>
>>
>> Using Rabin Vincent's ARM uprobes patches as a base, enable uprobes
>> support on ARM.
>>
>> Caveats:
>>
>> - Thumb is not supported
>> - XOL abort/trap handling is not implemented
>
> I shall repeat my comment from version one of the patch...
>
> What are the consequences of this, e.g. is it possible for a probe to
> get stuck in an infinite loop of faulting? I hope there are no integrity
> issues for the kernel itself.
>
> Would be good if someone familiar with uprobes working could answer
> that.
Testing shows it does indeed get stuck continuously trapping. The
process is killable. Fortunately all the infrastructure is already
there for fixing this. I've patched the code to detect the trap and
allow it to be processed, using the powerpc uprobes code as a model.
The changes required are fairly small and entirely in the
architecture-specific code.
As mentioned before, thumb support is a follow-on project.
> I've a few other comments...
[snip]
>
>
>> +const union decode_item uprobes_probes_actions[] = {
>> + [PROBES_EMULATE_NONE] {.handler = probes_simulate_nop},
>
> There is a missing '=' in the line above. Interesting that GCC doesn't
> complain (I tried compiling this patch and it didn't).
>
That is indeed odd. I have fixed it (my code, not the compiler).
[snip]
>> +bool arch_uprobe_skip_sstep(struct arch_uprobe *auprobe, struct pt_regs *regs)
>> +{
>> + void *addr;
>
> 'addr' is not used so this line can be deleted
>
>> + probes_opcode_t opcode;
>> +
>> + if (!auprobe->simulate)
>> + return false;
>> +
>> + addr = (void *) regs->ARM_pc;
>
> and so can this line ^^^
Fixed.
>
>> + opcode = __mem_to_opcode_arm(*(unsigned int *) auprobe->insn);
>> +
>> + auprobe->asi.insn_singlestep(opcode, &auprobe->asi, regs);
>> +
>> + return true;
>> +}
>> +
>
Fixed.
> [rest of patch snipped]
>
-dl
next prev parent reply other threads:[~2014-01-21 16:51 UTC|newest]
Thread overview: 68+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-12-16 4:08 [PATCH v4 00/16] uprobes: Add uprobes support for ARM David Long
2013-12-16 4:08 ` David Long
2013-12-16 4:08 ` [PATCH v4 01/16] uprobes: allow ignoring of probe hits David Long
2013-12-16 4:08 ` David Long
2013-12-16 4:08 ` [PATCH v4 02/16] ARM: move shared uprobe/kprobe definitions into new include file David Long
2013-12-16 4:08 ` David Long
2013-12-20 12:46 ` Jon Medhurst (Tixy)
2013-12-20 12:46 ` Jon Medhurst (Tixy)
2014-01-15 16:43 ` David Long
2014-01-15 16:43 ` David Long
2013-12-16 4:08 ` [PATCH v4 03/16] ARM: Move generic arm instruction parsing code to new files for sharing between features David Long
2013-12-16 4:08 ` David Long
2013-12-16 4:08 ` [PATCH v4 04/16] ARM: move generic thumb instruction parsing code to new files for use by other feature David Long
2013-12-16 4:08 ` David Long
2013-12-20 12:46 ` Jon Medhurst (Tixy)
2013-12-20 12:46 ` Jon Medhurst (Tixy)
2014-01-15 16:41 ` David Long
2014-01-15 16:41 ` David Long
2013-12-16 4:08 ` [PATCH v4 05/16] ARM: use a function table for determining instruction interpreter actions David Long
2013-12-16 4:08 ` David Long
2013-12-20 12:45 ` Jon Medhurst (Tixy)
2013-12-20 12:45 ` Jon Medhurst (Tixy)
2014-01-15 16:25 ` David Long
2014-01-15 16:25 ` David Long
2013-12-16 4:08 ` [PATCH v4 06/16] ARM: Disable jprobes test when built into thumb-mode kernel David Long
2013-12-16 4:08 ` David Long
2013-12-16 4:08 ` [PATCH v4 07/16] ARM: Remove use of struct kprobe from generic probes code David Long
2013-12-16 4:08 ` David Long
2013-12-20 13:55 ` Jon Medhurst (Tixy)
2013-12-20 13:55 ` Jon Medhurst (Tixy)
2014-01-15 16:44 ` David Long
2014-01-15 16:44 ` David Long
2013-12-16 4:08 ` [PATCH v4 08/16] ARM: Use new opcode type in ARM kprobes/uprobes code David Long
2013-12-16 4:08 ` David Long
2013-12-16 4:08 ` [PATCH v4 09/16] ARM: Make the kprobes condition_check symbol names more generic David Long
2013-12-16 4:08 ` David Long
2013-12-16 4:08 ` [PATCH v4 10/16] ARM: Change more ARM kprobes symbol names to something more David Long
2013-12-16 4:08 ` David Long
2013-12-16 4:08 ` [PATCH v4 11/16] ARM: Rename the shared kprobes/uprobe return value enum David Long
2013-12-16 4:08 ` David Long
2013-12-16 4:08 ` [PATCH v4 12/16] ARM: Change the remaining shared kprobes/uprobes symbols to something generic David Long
2013-12-16 4:08 ` David Long
2013-12-16 4:08 ` [PATCH v4 13/16] ARM: Add an emulate flag to the kprobes/uprobes instruction decode functions David Long
2013-12-16 4:08 ` David Long
2013-12-20 14:58 ` Jon Medhurst (Tixy)
2013-12-20 14:58 ` Jon Medhurst (Tixy)
2014-01-15 19:31 ` David Long
2014-01-15 19:31 ` David Long
2014-01-16 9:18 ` Jon Medhurst (Tixy)
2014-01-16 9:18 ` Jon Medhurst (Tixy)
2014-01-16 18:12 ` David Long
2014-01-16 18:12 ` David Long
2013-12-16 4:08 ` [PATCH v4 14/16] ARM: Make arch_specific_insn a define for new arch_probes_insn structure David Long
2013-12-16 4:08 ` David Long
2013-12-16 4:08 ` [PATCH v4 15/16] ARM: add uprobes support David Long
2013-12-16 4:08 ` David Long
2013-12-20 18:34 ` Jon Medhurst (Tixy)
2013-12-20 18:34 ` Jon Medhurst (Tixy)
2013-12-20 19:00 ` Rabin Vincent
2013-12-20 19:00 ` Rabin Vincent
2013-12-20 19:47 ` Jon Medhurst (Tixy)
2013-12-20 19:47 ` Jon Medhurst (Tixy)
2013-12-23 15:32 ` Oleg Nesterov
2013-12-23 15:32 ` Oleg Nesterov
2014-01-21 16:51 ` David Long [this message]
2014-01-21 16:51 ` David Long
2013-12-16 4:08 ` [PATCH v4 16/16] ARM: Remove uprobes dependency on kprobes David Long
2013-12-16 4:08 ` 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=52DEA58A.9050406@linaro.org \
--to=dave.long@linaro.org \
--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 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.