Move the code that calculates the type of the system call stop
out of ptrace_get_syscall_info() into a separate function
ptrace_get_syscall_info_op() which is going to be used later
to implement PTRACE_SET_SYSCALL_INFO API.
Signed-off-by: Dmitry V. Levin <ldv@strace.io>
---
kernel/ptrace.c | 52 ++++++++++++++++++++++++++++++-------------------
1 file changed, 32 insertions(+), 20 deletions(-)
diff --git a/kernel/ptrace.c b/kernel/ptrace.c
index d5f89f9ef29f..e7e0003cc8e0 100644
--- a/kernel/ptrace.c
+++ b/kernel/ptrace.c
@@ -965,19 +965,8 @@ ptrace_get_syscall_info_exit(struct task_struct *child, struct pt_regs *regs,
}
static int
-ptrace_get_syscall_info(struct task_struct *child, unsigned long user_size,
- void __user *datavp)
+ptrace_get_syscall_info_op(struct task_struct *child)
{
- struct pt_regs *regs = task_pt_regs(child);
- struct ptrace_syscall_info info = {
- .op = PTRACE_SYSCALL_INFO_NONE,
- .arch = syscall_get_arch(child),
- .instruction_pointer = instruction_pointer(regs),
- .stack_pointer = user_stack_pointer(regs),
- };
- unsigned long actual_size = offsetof(struct ptrace_syscall_info, entry);
- unsigned long write_size;
-
/*
* This does not need lock_task_sighand() to access
* child->last_siginfo because ptrace_freeze_traced()
@@ -988,18 +977,41 @@ ptrace_get_syscall_info(struct task_struct *child, unsigned long user_size,
case SIGTRAP | 0x80:
switch (child->ptrace_message) {
case PTRACE_EVENTMSG_SYSCALL_ENTRY:
- actual_size = ptrace_get_syscall_info_entry(child, regs,
- &info);
- break;
+ return PTRACE_SYSCALL_INFO_ENTRY;
case PTRACE_EVENTMSG_SYSCALL_EXIT:
- actual_size = ptrace_get_syscall_info_exit(child, regs,
- &info);
- break;
+ return PTRACE_SYSCALL_INFO_EXIT;
}
break;
case SIGTRAP | (PTRACE_EVENT_SECCOMP << 8):
- actual_size = ptrace_get_syscall_info_seccomp(child, regs,
- &info);
+ return PTRACE_SYSCALL_INFO_SECCOMP;
+ }
+
+ return PTRACE_SYSCALL_INFO_NONE;
+}
+
+static int
+ptrace_get_syscall_info(struct task_struct *child, unsigned long user_size,
+ void __user *datavp)
+{
+ struct pt_regs *regs = task_pt_regs(child);
+ struct ptrace_syscall_info info = {
+ .op = ptrace_get_syscall_info_op(child),
+ .arch = syscall_get_arch(child),
+ .instruction_pointer = instruction_pointer(regs),
+ .stack_pointer = user_stack_pointer(regs),
+ };
+ unsigned long actual_size = offsetof(struct ptrace_syscall_info, entry);
+ unsigned long write_size;
+
+ switch (info.op) {
+ case PTRACE_SYSCALL_INFO_ENTRY:
+ actual_size = ptrace_get_syscall_info_entry(child, regs, &info);
+ break;
+ case PTRACE_SYSCALL_INFO_EXIT:
+ actual_size = ptrace_get_syscall_info_exit(child, regs, &info);
+ break;
+ case PTRACE_SYSCALL_INFO_SECCOMP:
+ actual_size = ptrace_get_syscall_info_seccomp(child, regs, &info);
break;
}
--
ldvOn 01/08, Dmitry V. Levin wrote:
>
> +ptrace_get_syscall_info_op(struct task_struct *child)
> {
...
> case SIGTRAP | 0x80:
> switch (child->ptrace_message) {
> case PTRACE_EVENTMSG_SYSCALL_ENTRY:
> - actual_size = ptrace_get_syscall_info_entry(child, regs,
> - &info);
> - break;
> + return PTRACE_SYSCALL_INFO_ENTRY;
> case PTRACE_EVENTMSG_SYSCALL_EXIT:
> - actual_size = ptrace_get_syscall_info_exit(child, regs,
> - &info);
> - break;
> + return PTRACE_SYSCALL_INFO_EXIT;
> }
> break;
> case SIGTRAP | (PTRACE_EVENT_SECCOMP << 8):
> - actual_size = ptrace_get_syscall_info_seccomp(child, regs,
> - &info);
> + return PTRACE_SYSCALL_INFO_SECCOMP;
> + }
> +
> + return PTRACE_SYSCALL_INFO_NONE;
Cosmetic, I won't insist, but I'd suggest to do
default:
return PTRACE_SYSCALL_INFO_SECCOMP;
}
to make it more symmetric.
Oleg.
On Thu, Jan 09, 2025 at 11:41:47AM +0100, Oleg Nesterov wrote:
> On 01/08, Dmitry V. Levin wrote:
> >
> > +ptrace_get_syscall_info_op(struct task_struct *child)
> > {
> ...
> > case SIGTRAP | 0x80:
> > switch (child->ptrace_message) {
> > case PTRACE_EVENTMSG_SYSCALL_ENTRY:
> > - actual_size = ptrace_get_syscall_info_entry(child, regs,
> > - &info);
> > - break;
> > + return PTRACE_SYSCALL_INFO_ENTRY;
> > case PTRACE_EVENTMSG_SYSCALL_EXIT:
> > - actual_size = ptrace_get_syscall_info_exit(child, regs,
> > - &info);
> > - break;
> > + return PTRACE_SYSCALL_INFO_EXIT;
> > }
> > break;
> > case SIGTRAP | (PTRACE_EVENT_SECCOMP << 8):
> > - actual_size = ptrace_get_syscall_info_seccomp(child, regs,
> > - &info);
> > + return PTRACE_SYSCALL_INFO_SECCOMP;
> > + }
> > +
> > + return PTRACE_SYSCALL_INFO_NONE;
>
> Cosmetic, I won't insist, but I'd suggest to do
>
> default:
> return PTRACE_SYSCALL_INFO_SECCOMP;
> }
>
> to make it more symmetric.
OK, but I suppose you mean PTRACE_SYSCALL_INFO_NONE here, though.
--
ldv
On 01/08, Dmitry V. Levin wrote:
>
> +static int
> +ptrace_get_syscall_info(struct task_struct *child, unsigned long user_size,
> + void __user *datavp)
> +{
> + struct pt_regs *regs = task_pt_regs(child);
> + struct ptrace_syscall_info info = {
> + .op = ptrace_get_syscall_info_op(child),
> + .arch = syscall_get_arch(child),
> + .instruction_pointer = instruction_pointer(regs),
> + .stack_pointer = user_stack_pointer(regs),
> + };
> + unsigned long actual_size = offsetof(struct ptrace_syscall_info, entry);
> + unsigned long write_size;
> +
> + switch (info.op) {
> + case PTRACE_SYSCALL_INFO_ENTRY:
> + actual_size = ptrace_get_syscall_info_entry(child, regs, &info);
> + break;
> + case PTRACE_SYSCALL_INFO_EXIT:
> + actual_size = ptrace_get_syscall_info_exit(child, regs, &info);
> + break;
> + case PTRACE_SYSCALL_INFO_SECCOMP:
> + actual_size = ptrace_get_syscall_info_seccomp(child, regs, &info);
> break;
OK... but unless I misread this patch, all 3 ptrace_get_syscall_info_xxx()
helpers will do the pointless info->op = PTRACE_SYSCALL_INFO_XXX ?
Oleg.
On Thu, Jan 09, 2025 at 11:09:44AM +0100, Oleg Nesterov wrote:
> On 01/08, Dmitry V. Levin wrote:
> >
> > +static int
> > +ptrace_get_syscall_info(struct task_struct *child, unsigned long user_size,
> > + void __user *datavp)
> > +{
> > + struct pt_regs *regs = task_pt_regs(child);
> > + struct ptrace_syscall_info info = {
> > + .op = ptrace_get_syscall_info_op(child),
> > + .arch = syscall_get_arch(child),
> > + .instruction_pointer = instruction_pointer(regs),
> > + .stack_pointer = user_stack_pointer(regs),
> > + };
> > + unsigned long actual_size = offsetof(struct ptrace_syscall_info, entry);
> > + unsigned long write_size;
> > +
> > + switch (info.op) {
> > + case PTRACE_SYSCALL_INFO_ENTRY:
> > + actual_size = ptrace_get_syscall_info_entry(child, regs, &info);
> > + break;
> > + case PTRACE_SYSCALL_INFO_EXIT:
> > + actual_size = ptrace_get_syscall_info_exit(child, regs, &info);
> > + break;
> > + case PTRACE_SYSCALL_INFO_SECCOMP:
> > + actual_size = ptrace_get_syscall_info_seccomp(child, regs, &info);
> > break;
>
> OK... but unless I misread this patch, all 3 ptrace_get_syscall_info_xxx()
> helpers will do the pointless info->op = PTRACE_SYSCALL_INFO_XXX ?
Thanks, your analysis is correct, with this change those assignments
become redundant, I'll remove them in the next iteration of this patch.
--
ldv
© 2016 - 2026 Red Hat, Inc.