From: takahiro.akashi@linaro.org (AKASHI Takahiro)
To: linux-arm-kernel@lists.infradead.org
Subject: [RFC] ptrace: add generic SET_SYSCALL request
Date: Wed, 12 Nov 2014 19:46:01 +0900 [thread overview]
Message-ID: <54633A69.6090202@linaro.org> (raw)
In-Reply-To: <20141107140405.GA30156@redhat.com>
Will,
On 11/07/2014 11:04 PM, Oleg Nesterov wrote:
> On 11/07, AKASHI Takahiro wrote:
>>
>> --- a/arch/arm/kernel/ptrace.c
>> +++ b/arch/arm/kernel/ptrace.c
>> @@ -853,11 +853,6 @@ long arch_ptrace(struct task_struct *child, long request,
>> datap);
>> break;
>>
>> - case PTRACE_SET_SYSCALL:
>> - task_thread_info(child)->syscall = data;
>> - ret = 0;
>> - break;
>> -
>> #ifdef CONFIG_CRUNCH
>> case PTRACE_GETCRUNCHREGS:
>> ret = ptrace_getcrunchregs(child, datap);
>> diff --git a/kernel/ptrace.c b/kernel/ptrace.c
>> index 54e7522..d7048fa 100644
>> --- a/kernel/ptrace.c
>> +++ b/kernel/ptrace.c
>> @@ -1001,6 +1001,12 @@ int ptrace_request(struct task_struct *child, long request,
>> break;
>> }
>> #endif
>> +
>> +#ifdef PTRACE_SET_SYSCALL
>> + case PTRACE_SET_SYSCALL:
>> + ret = syscall_set_nr(child, task_pt_regs(child), data);
>> + break;
>> +#endif
>
> I too do not understand why it makes sense to move PTRACE_SET_SYSCALL into
> the common kernel/ptrace.c.
I think I explained why we need a new (atomic) interface of changing a system
call number while tracing with ptrace. But I don't have a strong preference,
either ptrace(SET_SYSCALL) or ptrace(SETREGSET, NT_SYSTEM_CALL).
> To me the fact that PTRACE_SET_SYSCALL can be undefined and syscall_set_nr()
> is very much arch-dependant (but most probably trivial) means that this code
> should live in arch_ptrace().
Thinking of Oleg's comment above, it doesn't make sense neither to define generic
NT_SYSTEM_CALL (user_regset) in uapi/linux/elf.h and implement it in ptrace_regset()
in kernel/ptrace.c with arch-defined syscall_(g)set_nr().
Since we should have the same interface on arm and arm64, we'd better implement
ptrace(PTRACE_SET_SYSCALL) locally on arm64 for now (as I originally submitted).
-Takahiro AKASHI
> In any case, I think it doesn't make sense to pass task_pt_regs(child), this
> helper can do this itself if it needs struct pt_regs.
>
> Oleg.
>
WARNING: multiple messages have this Message-ID (diff)
From: AKASHI Takahiro <takahiro.akashi@linaro.org>
To: Oleg Nesterov <oleg@redhat.com>
Cc: roland@hack.frob.com, linux@arm.linux.org.uk,
will.deacon@arm.com, dsaxena@linaro.org, keescook@chromium.org,
linux-arm-kernel@lists.infradead.org,
linaro-kernel@lists.linaro.org, linux-kernel@vger.kernel.org
Subject: Re: [RFC] ptrace: add generic SET_SYSCALL request
Date: Wed, 12 Nov 2014 19:46:01 +0900 [thread overview]
Message-ID: <54633A69.6090202@linaro.org> (raw)
In-Reply-To: <20141107140405.GA30156@redhat.com>
Will,
On 11/07/2014 11:04 PM, Oleg Nesterov wrote:
> On 11/07, AKASHI Takahiro wrote:
>>
>> --- a/arch/arm/kernel/ptrace.c
>> +++ b/arch/arm/kernel/ptrace.c
>> @@ -853,11 +853,6 @@ long arch_ptrace(struct task_struct *child, long request,
>> datap);
>> break;
>>
>> - case PTRACE_SET_SYSCALL:
>> - task_thread_info(child)->syscall = data;
>> - ret = 0;
>> - break;
>> -
>> #ifdef CONFIG_CRUNCH
>> case PTRACE_GETCRUNCHREGS:
>> ret = ptrace_getcrunchregs(child, datap);
>> diff --git a/kernel/ptrace.c b/kernel/ptrace.c
>> index 54e7522..d7048fa 100644
>> --- a/kernel/ptrace.c
>> +++ b/kernel/ptrace.c
>> @@ -1001,6 +1001,12 @@ int ptrace_request(struct task_struct *child, long request,
>> break;
>> }
>> #endif
>> +
>> +#ifdef PTRACE_SET_SYSCALL
>> + case PTRACE_SET_SYSCALL:
>> + ret = syscall_set_nr(child, task_pt_regs(child), data);
>> + break;
>> +#endif
>
> I too do not understand why it makes sense to move PTRACE_SET_SYSCALL into
> the common kernel/ptrace.c.
I think I explained why we need a new (atomic) interface of changing a system
call number while tracing with ptrace. But I don't have a strong preference,
either ptrace(SET_SYSCALL) or ptrace(SETREGSET, NT_SYSTEM_CALL).
> To me the fact that PTRACE_SET_SYSCALL can be undefined and syscall_set_nr()
> is very much arch-dependant (but most probably trivial) means that this code
> should live in arch_ptrace().
Thinking of Oleg's comment above, it doesn't make sense neither to define generic
NT_SYSTEM_CALL (user_regset) in uapi/linux/elf.h and implement it in ptrace_regset()
in kernel/ptrace.c with arch-defined syscall_(g)set_nr().
Since we should have the same interface on arm and arm64, we'd better implement
ptrace(PTRACE_SET_SYSCALL) locally on arm64 for now (as I originally submitted).
-Takahiro AKASHI
> In any case, I think it doesn't make sense to pass task_pt_regs(child), this
> helper can do this itself if it needs struct pt_regs.
>
> Oleg.
>
next prev parent reply other threads:[~2014-11-12 10:46 UTC|newest]
Thread overview: 48+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-11-07 7:47 [RFC] ptrace: add generic SET_SYSCALL request AKASHI Takahiro
2014-11-07 7:47 ` AKASHI Takahiro
2014-11-07 9:30 ` Arnd Bergmann
2014-11-07 9:30 ` Arnd Bergmann
2014-11-07 11:55 ` Will Deacon
2014-11-07 11:55 ` Will Deacon
2014-11-07 12:03 ` Arnd Bergmann
2014-11-07 12:03 ` Arnd Bergmann
2014-11-07 12:11 ` Russell King - ARM Linux
2014-11-07 12:11 ` Russell King - ARM Linux
2014-11-07 12:44 ` Arnd Bergmann
2014-11-07 12:44 ` Arnd Bergmann
2014-11-07 13:11 ` Will Deacon
2014-11-07 13:11 ` Will Deacon
2014-11-07 14:30 ` Arnd Bergmann
2014-11-07 14:30 ` Arnd Bergmann
2014-11-07 16:44 ` Kees Cook
2014-11-07 16:44 ` Kees Cook
2014-11-07 23:05 ` Roland McGrath
2014-11-07 23:05 ` Roland McGrath
2014-11-07 12:27 ` Will Deacon
2014-11-07 12:27 ` Will Deacon
2014-11-10 6:36 ` AKASHI Takahiro
2014-11-10 6:36 ` AKASHI Takahiro
2014-11-07 14:04 ` Oleg Nesterov
2014-11-07 14:04 ` Oleg Nesterov
2014-11-12 10:46 ` AKASHI Takahiro [this message]
2014-11-12 10:46 ` AKASHI Takahiro
2014-11-12 11:00 ` Will Deacon
2014-11-12 11:00 ` Will Deacon
2014-11-12 11:06 ` AKASHI Takahiro
2014-11-12 11:06 ` AKASHI Takahiro
2014-11-12 11:13 ` Will Deacon
2014-11-12 11:13 ` Will Deacon
2014-11-12 11:19 ` Arnd Bergmann
2014-11-12 11:19 ` Arnd Bergmann
2014-11-12 12:05 ` Russell King - ARM Linux
2014-11-12 12:05 ` Russell King - ARM Linux
2014-11-13 7:02 ` AKASHI Takahiro
2014-11-13 7:02 ` AKASHI Takahiro
2014-11-13 10:21 ` Arnd Bergmann
2014-11-13 10:21 ` Arnd Bergmann
2014-11-13 14:49 ` Ulrich Weigand
2014-11-13 14:49 ` Ulrich Weigand
2014-11-13 22:25 ` Arnd Bergmann
2014-11-13 22:25 ` Arnd Bergmann
2014-11-14 1:40 ` AKASHI Takahiro
2014-11-14 1:40 ` AKASHI Takahiro
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=54633A69.6090202@linaro.org \
--to=takahiro.akashi@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.