From: wade_farnsworth@mentor.com (Wade Farnsworth)
To: linux-arm-kernel@lists.infradead.org
Subject: ARM tracehook support
Date: Tue, 21 Feb 2012 11:08:46 -0700 [thread overview]
Message-ID: <4F43DDAE.2080800@mentor.com> (raw)
In-Reply-To: <20120221172750.GA1774@n2100.arm.linux.org.uk>
Russell King - ARM Linux wrote:
> On Mon, Feb 20, 2012 at 11:08:57AM -0700, Wade Farnsworth wrote:
>> Will Deacon wrote:
>>> On Wed, Nov 30, 2011 at 02:46:20PM +0000, Steven Walter wrote:
>>>> +static inline void syscall_get_arguments(struct task_struct *task,
>>>> + struct pt_regs *regs,
>>>> + unsigned int i, unsigned int n,
>>>> + unsigned long *args)
>>>> +{
>>>> + BUG_ON(i + n> 6);
>>>
>>> So I guess 6 is the maximum number of registers that are used for
>>> syscall passing. That sounds about right to me, but I wondered how
>>> you worked it out (and whether or not it should be defined
>>> somewhere?).
>>
>> I believe the 6 argument constraint is a specific to
>> syscall_get_arguments(). Notice the comment in
>> include/asm-generic/syscall.h:
>
> Well, there's two things here. Is a BUG_ON() really suitable here?
> What controls how many arguments are fetched? Userspace?
Yeah, it could be userspace, or a kernel tracer such as ftrace.
> If so, that's
> a nice way to oops the kernel.
I agree that a BUG_ON is probably not ideal, though I note that the
other arch's tend to BUG as well. Since there's no way to return an
error from this function, what if we just used a pr_warning() and
backfill the bogus elements of args[] with zero?
>
> Secondly, there is a 7 argument syscall - sys_syscall, which we use on
> OABI to deal with calling a syscall by number. That really does show
> up as a unique syscall there, so if you want to parse the last argument
> to such a syscall you need to be able to read up to and including ARM
> register 7.
I think we can change it to allow up to 7 arguments in this function
easily enough. I think changing the tracers to support an additional
argument should be done separatels from this patchset, however.
>
>> /*
>> [...]
>> *
>> * It's only valid to call this when @task is stopped for tracing on
>> * entry to a system call, due to %TIF_SYSCALL_TRACE or %TIF_SYSCALL_AUDIT.
>> * It's invalid to call this with @i + @n> 6; we only support system calls
>> * taking up to 6 arguments.
>> */
>>
>> Additionally, if you'll look at the other architectures' implementations
>> you'll see similar code.
>>
>>> In fact, how are these things supposed to deal with 64-bit arguments
>>> that straddle two registers? I think we always pack arguments such
>>> that we don't get holes in the register layout, but it might be worth
>>> checking (EABI requires 64-bit arguments to be passed in even
>>> registers).
>>
>> Hmm, I do believe that 32-bit powerpc has similar alignment issues
>> (64-bit args must be passed in odd/even pairs), but I don't see any
>> special handling of this in that architecture's
>> syscall_get/set_arguments(). So I'm wondering if the handling of this
>> is or should be handled elsewhere. I'll keep digging on this.
>
> I did point that issue out when it first came up, and I think Roland had
> an answer for it, though I forget what it was.
Yes, see my other mail for a link to Roland's answer.
>
> The big stumbling block to this is OABI, and as I continue to be wholely
> OABI based here, it's extremely important that nothing in OABI land gets
> broken. I don't see there's any chance of me ever getting off OABI given
> the range of platforms I have, and the restriction that EABI was designed
> to be impossible on ARMv4 architectures.
Right, I think most if not all of the issues w.r.t. OABI that were
present in Steven's original patches have been ironed out.
I'll post the patches once I've addressed the current crop of concerns.
Thanks, Russell and Will, for the comments.
-Wade
prev parent reply other threads:[~2012-02-21 18:08 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-02-20 15:37 ARM tracehook support Wade Farnsworth
2012-02-20 15:47 ` Will Deacon
2012-02-20 18:08 ` Wade Farnsworth
2012-02-21 17:06 ` Wade Farnsworth
2012-02-21 17:24 ` Will Deacon
2012-02-21 17:30 ` Russell King - ARM Linux
2012-02-21 17:27 ` Russell King - ARM Linux
2012-02-21 18:08 ` Wade Farnsworth [this message]
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=4F43DDAE.2080800@mentor.com \
--to=wade_farnsworth@mentor.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).