Skip to contentSkip to conversation

sycalls: Remove args i and n from syscall_get_arguments()

RFC In review Steven Rostedt 3 participants 12 messages Nov 7, 2016 – Nov 8, 2016

Subsystems (21)
Arm (32-bit) x86 PowerPC Library code +17 more subsystems
lore
Share & tools
lore

Steven Rostedt

author maintainer
#0 lore raw

cover sycalls: Remove args i and n from syscall_get_arguments()

At Linux Plumbers, Andy Lutomirski approached me to tell me that the syscall_get_arguments() implementation in x86 was horrible and gcc certainly gets it wrong. He said that since the tracepoints only pass in 0 and 6 for i and n repectively, it should be optimized for that case. Inspecting the kernel, I discovered that all users pass in 0 for i and only one file passing in something other than 6 for the number of arguments. That code happens to be my own code used for the special syscall tracing. That can easily be converted to just using 0 and 6 as well, and only copying what is needed. Which is probably the faster path anyway for that case.

I haven't run the numbers (I can do that when I get some time), but since pretty much all use cases use 0 and 6 and that would allow these functions not to need strange logic to handle odd cases, I think this is still a win.

I haven't looked at removing those same parameters from syscall_set_arguments() yet.

Thoughts?

-- Steve

Steven Rostedt (3):
ptrace: Remove maxargs from task_current_syscall() tracing/syscalls: Pass in hardcoded '6' into syscall_get_arguments()
syscalls: Remove start and number from syscall_get_arguments() args

----

 arch/arc/include/asm/syscall.h        |  7 ++--
 arch/arm/include/asm/syscall.h        | 23 ++---------
 arch/arm64/include/asm/syscall.h      | 22 ++---------
 arch/blackfin/include/asm/syscall.h   | 22 +++++++----
 arch/c6x/include/asm/syscall.h        | 41 ++++----------------
 arch/frv/include/asm/syscall.h        | 26 +++----------
 arch/h8300/include/asm/syscall.h      | 34 ++++-------------
 arch/hexagon/include/asm/syscall.h    |  4 +-
 arch/ia64/include/asm/syscall.h       |  5 +--
 arch/metag/include/asm/syscall.h      |  4 +-
 arch/microblaze/include/asm/syscall.h |  4 +-
 arch/mips/include/asm/syscall.h       |  3 +-
 arch/mn10300/include/asm/syscall.h    | 32 +++-------------
 arch/nios2/include/asm/syscall.h      | 42 ++++----------------
 arch/openrisc/include/asm/syscall.h   |  6 +--
 arch/parisc/include/asm/syscall.h     | 30 ++++-----------
 arch/powerpc/include/asm/syscall.h    |  8 ++--
 arch/s390/include/asm/syscall.h       | 11 +++---
 arch/sh/include/asm/syscall_32.h      | 26 +++----------
 arch/sh/include/asm/syscall_64.h      |  4 +-
 arch/sparc/include/asm/syscall.h      |  4 +-
 arch/tile/include/asm/syscall.h       |  4 +-
 arch/um/include/asm/syscall-generic.h | 39 +++----------------
 arch/x86/include/asm/syscall.h        | 72 ++++++++---------------------------
 fs/proc/base.c                        |  2 +-
 include/asm-generic/syscall.h         | 11 ++----
 include/linux/ptrace.h                |  4 +-
 include/trace/events/syscalls.h       |  2 +-
 kernel/seccomp.c                      |  2 +-
 kernel/trace/trace_syscalls.c         | 10 +++--
 lib/syscall.c                         | 22 ++++-------
 31 files changed, 139 insertions(+), 387 deletions(-)
author maintainer
#1 lore raw

RFC 1/3 ptrace: Remove maxargs from task_current_syscall()

From: Steven Rostedt <[email protected]>

task_current_syscall() has a single user that passes in 6 for maxargs, which is the maximum arguments that can be used to get system calls from syscall_get_arguments(). Instead of passing in a number of arguments to grab, just get 6 arguments. The args argument even specifies that it's an array of 6 items.

This will also allow changing syscall_get_arguments() to not get a variable number of arguments, but always grab 6.

Signed-off-by Steven Rostedt <[email protected]>
---
 fs/proc/base.c         |  2 +-
 include/linux/ptrace.h |  4 ++--
 lib/syscall.c          | 22 ++++++++--------------
 3 files changed, 11 insertions(+), 17 deletions(-)
fs/proc/base.c +1 -1
index 8e654468ab67..25cd58bd7236 100644
@@ -650,7 +650,7 @@ static int proc_pid_syscall(struct seq_file *m, struct pid_namespace *ns,
 	if (res)
 		return res;
 
-	if (task_current_syscall(task, &nr, args, 6, &sp, &pc))
+	if (task_current_syscall(task, &nr, args, &sp, &pc))
 		seq_puts(m, "running\n");
 	else if (nr < 0)
 		seq_printf(m, "%ld 0x%lx 0x%lx\n", nr, sp, pc);
include/linux/ptrace.h +2 -2
index 504c98a278d4..8af5226d2ee6 100644
@@ -403,7 +403,7 @@ static inline void user_single_step_siginfo(struct task_struct *tsk,
 #endif
 
 extern int task_current_syscall(struct task_struct *target, long *callno,
-				unsigned long args[6], unsigned int maxargs,
-				unsigned long *sp, unsigned long *pc);
+				unsigned long args[6], unsigned long *sp,
+				unsigned long *pc);
 
 #endif
lib/syscall.c +8 -14
index 63239e097b13..cbd376c66bbc 100644
@@ -4,8 +4,8 @@
 #include <asm/syscall.h>
 
 static int collect_syscall(struct task_struct *target, long *callno,
-			   unsigned long args[6], unsigned int maxargs,
-			   unsigned long *sp, unsigned long *pc)
+			   unsigned long args[6], unsigned long *sp,
+			   unsigned long *pc)
 {
 	struct pt_regs *regs;
 

@@ -25,8 +25,8 @@ static int collect_syscall(struct task_struct *target, long *callno,
 	*pc = instruction_pointer(regs);
 
 	*callno = syscall_get_nr(target, regs);
-	if (*callno != -1L && maxargs > 0)
-		syscall_get_arguments(target, regs, 0, maxargs, args);
+	if (*callno != -1L)
+		syscall_get_arguments(target, regs, 0, 6, args);
 
 	put_task_stack(target);
 	return 0;

@@ -37,7 +37,6 @@ static int collect_syscall(struct task_struct *target, long *callno,
  * @target:		thread to examine
  * @callno:		filled with system call number or -1
  * @args:		filled with @maxargs system call arguments
- * @maxargs:		number of elements in @args to fill
  * @sp:			filled with user stack pointer
  * @pc:			filled with user PC
  *

@@ -55,21 +54,16 @@ static int collect_syscall(struct task_struct *target, long *callno,
  * get() calls as long as we're sure @target won't return to user mode.
  *
  * Returns -%EAGAIN if @target does not remain blocked.
- *
- * Returns -%EINVAL if @maxargs is too large (maximum is six).
  */
 int task_current_syscall(struct task_struct *target, long *callno,
-			 unsigned long args[6], unsigned int maxargs,
-			 unsigned long *sp, unsigned long *pc)
+			 unsigned long args[6], unsigned long *sp,
+			 unsigned long *pc)
 {
 	long state;
 	unsigned long ncsw;
 
-	if (unlikely(maxargs > 6))
-		return -EINVAL;
-
 	if (target == current)
-		return collect_syscall(target, callno, args, maxargs, sp, pc);
+		return collect_syscall(target, callno, args, sp, pc);
 
 	state = target->state;
 	if (unlikely(!state))

@@ -77,7 +71,7 @@ int task_current_syscall(struct task_struct *target, long *callno,
 
 	ncsw = wait_task_inactive(target, state);
 	if (unlikely(!ncsw) ||
-	    unlikely(collect_syscall(target, callno, args, maxargs, sp, pc)) ||
+	    unlikely(collect_syscall(target, callno, args, sp, pc)) ||
 	    unlikely(wait_task_inactive(target, state) != ncsw))
 		return -EAGAIN;
Signature · 1 line
2.9.3
reviewer Reviewed-by on v1 1/3
#2 lore raw

Re: [RFC][ATCH 1/3] ptrace: Remove maxargs from task_current_syscall()

Quoted 10 lines · Steven Rostedt

From: Steven Rostedt <[email protected]>

task_current_syscall() has a single user that passes in 6 for maxargs, which is the maximum arguments that can be used to get system calls from syscall_get_arguments(). Instead of passing in a number of arguments to grab, just get 6 arguments. The args argument even specifies that it's an array of 6 items.

This will also allow changing syscall_get_arguments() to not get a variable number of arguments, but always grab 6.

Reviewed-by Andy Lutomirski <[email protected]>
#3 lore raw

Re: [RFC][ATCH 1/3] ptrace: Remove maxargs from task_current_syscall()

In reply to Steven Rostedt

So I definitely approve of the change, but I wonder if we should go one step further:

include/linux/ptrace.h static inline void user_single_step_siginfo(struct task_struct *tsk, · line 407

Quoted 6 lines · Steven Rostedt

extern int task_current_syscall(struct task_struct *target, long *callno,

include/linux/ptrace.h +2 -2
-                               unsigned long args[6], unsigned int maxargs,
-                               unsigned long *sp, unsigned long *pc);
+                               unsigned long args[6], unsigned long *sp,
+                               unsigned long *pc);

The thing is, in C, having an array in a function declaration is pretty much exactly the same as just having a pointer, so from a type checking standpoint it doesn't really help all that much (but from a "human documentation" side the "args[6]" is much better than "*args").

However, what would really help type checking is making it a structure. And maybe that structure could just contain "callno", "sp" and "pc" too? That would not only fix the type checking, it would make the calling convention even cleaner. Just have one single structure that contains all the relevant data.

That would also allow us (later - don't do it now) to replace the odd collection of "get registers one by one" with a single architecture-specific routine that fills it all in.Right now we do

        *sp = user_stack_pointer(regs);
        *pc = instruction_pointer(regs);

        *callno = syscall_get_nr(target, regs);
        if (*callno != -1L && maxargs > 0)
                syscall_get_arguments(target, regs, 0, maxargs, args);

and it feels like this could/should just be a single "syscall_get_info()" helper.

For example, kernel/seccomp.c does this instead:

        sd->nr = syscall_get_nr(task, regs);
        sd->arch = syscall_get_arch();
        syscall_get_arguments(task, regs, 0, 6, args);
        sd->args[0] = args[0];
        sd->args[1] = args[1];
        sd->args[2] = args[2];
        sd->args[3] = args[3];
        sd->args[4] = args[4];
        sd->args[5] = args[5];
        sd->instruction_pointer = KSTK_EIP(task);

and notice how it wants "pc" too, but it used a completely different way to get them? So the ad-hoc nature of the current interfaces really does shine through here (ok, so seccomp doesn't need the user stack pointer, but it really won't hurt there either.

Hmm?

Linus

reviewer
#4 lore raw

Re: [RFC][ATCH 1/3] ptrace: Remove maxargs from task_current_syscall()

Quoted 16 lines · Linus Torvalds

So I definitely approve of the change, but I wonder if we should go one step further:

include/linux/ptrace.h static inline void user_single_step_siginfo(struct task_struct *tsk, · line 407

Quoted 6 lines · Steven Rostedt

extern int task_current_syscall(struct task_struct *target, long *callno,

include/linux/ptrace.h +2 -2
-                               unsigned long args[6], unsigned int maxargs,
-                               unsigned long *sp, unsigned long *pc);
+                               unsigned long args[6], unsigned long *sp,
+                               unsigned long *pc);

The thing is, in C, having an array in a function declaration is pretty much exactly the same as just having a pointer, so from a type checking standpoint it doesn't really help all that much (but from a "human documentation" side the "args[6]" is much better than "*args").

Quoted 5 lines

However, what would really help type checking is making it a structure. And maybe that structure could just contain "callno", "sp" and "pc" too? That would not only fix the type checking, it would make the calling convention even cleaner. Just have one single structure that contains all the relevant data.

I would propose calling this 'struct seccomp_data'.

Quoted 13 lines

For example, kernel/seccomp.c does this instead:

        sd->nr = syscall_get_nr(task, regs);
        sd->arch = syscall_get_arch();
        syscall_get_arguments(task, regs, 0, 6, args);
        sd->args[0] = args[0];
        sd->args[1] = args[1];
        sd->args[2] = args[2];
        sd->args[3] = args[3];
        sd->args[4] = args[4];
        sd->args[5] = args[5];
        sd->instruction_pointer = KSTK_EIP(task);

It's a bit hard to tell from seccomp.c, but x86 carefully arranges for that code to never get run -- instead the entry code supplies a struct seccomp_data. Other arches could follow suit for a nice speedup.

--Andy

author maintainer
#5 lore raw

Re: [RFC][ATCH 1/3] ptrace: Remove maxargs from task_current_syscall()

Quoted 18 lines · On Tue, 8 Nov 2016 08:20:48 -0800 Andy Lutomirski <[email protected]> wrote:
Quoted 16 lines · Linus Torvalds

So I definitely approve of the change, but I wonder if we should go one step further:

include/linux/ptrace.h static inline void user_single_step_siginfo(struct task_struct *tsk, · line 407

Quoted 6 lines · Steven Rostedt

extern int task_current_syscall(struct task_struct *target, long *callno,

include/linux/ptrace.h +2 -2
-                               unsigned long args[6], unsigned int maxargs,
-                               unsigned long *sp, unsigned long *pc);
+                               unsigned long args[6], unsigned long *sp,
+                               unsigned long *pc);

The thing is, in C, having an array in a function declaration is pretty much exactly the same as just having a pointer, so from a type checking standpoint it doesn't really help all that much (but from a "human documentation" side the "args[6]" is much better than "*args").

Quoted 7 lines
Quoted 5 lines

However, what would really help type checking is making it a structure. And maybe that structure could just contain "callno", "sp" and "pc" too? That would not only fix the type checking, it would make the calling convention even cleaner. Just have one single structure that contains all the relevant data.

I would propose calling this 'struct seccomp_data'.

I'm assuming you mean to use the existing seccomp_data? But isn't that already defined as a user structure? Thus, we can't add sp and pc to it.

I can change syscall_get_arguments() to take the seccomp_data as an input, and just fill in the arguments directly.

-- Steve

reviewer
#6 lore raw

Re: [RFC][ATCH 1/3] ptrace: Remove maxargs from task_current_syscall()

Quoted 28 lines · Steven Rostedt
Quoted 24 lines · On Tue, 8 Nov 2016 08:20:48 -0800 Andy Lutomirski <[email protected]> wrote:
Quoted 21 lines · Linus Torvalds

So I definitely approve of the change, but I wonder if we should go one step further:

include/linux/ptrace.h static inline void user_single_step_siginfo(struct task_struct *tsk, · line 407

Quoted 6 lines · Steven Rostedt

extern int task_current_syscall(struct task_struct *target, long *callno,

include/linux/ptrace.h +2 -2
-                               unsigned long args[6], unsigned int maxargs,
-                               unsigned long *sp, unsigned long *pc);
+                               unsigned long args[6], unsigned long *sp,
+                               unsigned long *pc);

The thing is, in C, having an array in a function declaration is pretty much exactly the same as just having a pointer, so from a type checking standpoint it doesn't really help all that much (but from a "human documentation" side the "args[6]" is much better than "*args").

However, what would really help type checking is making it a structure. And maybe that structure could just contain "callno", "sp" and "pc" too? That would not only fix the type checking, it would make the calling convention even cleaner. Just have one single structure that contains all the relevant data.

Quoted 4 lines
Quoted 1 line

I would propose calling this 'struct seccomp_data'.

I'm assuming you mean to use the existing seccomp_data? But isn't that already defined as a user structure? Thus, we can't add sp and pc to it.

pc is there. sp isn't, but that could be separate. Or you could embed seccomp_data in a bigger structure.

author maintainer
#7 lore raw

Re: [RFC][ATCH 1/3] ptrace: Remove maxargs from task_current_syscall()

Quoted 2 lines · On Tue, 8 Nov 2016 13:06:35 -0800 Andy Lutomirski <[email protected]> wrote:

Or you could
embed seccomp_data in a bigger structure.

I was thinking about doing just that.

But for now, I may just use seccomp_data and when we want to add more, we can do the embedding.

-- Steve

author maintainer
#8 lore raw

RFC 2/3 tracing/syscalls: Pass in hardcoded 6 into syscall_get_arguments()

In reply to Steven Rostedt

From: Steven Rostedt <[email protected]>

The only users that calls syscall_get_arguments() with a variable and not a hard coded '6' is ftrace_syscall_enter(). syscall_get_arguments() can be optimized by removing a variable input, and always grabbing 6 arguments regardless of what the system call actually uses.

Change ftrace_syscall_enter() to pass the 6 args into a local stack array and copy the necessary arguments into the trace event as needed.

This is needed to remove two parameters from syscall_get_arguments().

Signed-off-by Steven Rostedt <[email protected]>
---
 kernel/trace/trace_syscalls.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)
kernel/trace/trace_syscalls.c +7 -3
index 5e10395da88e..52c1503cedfa 100644
@@ -313,6 +313,7 @@ static void ftrace_syscall_enter(void *data, struct pt_regs *regs, long id)
 	struct ring_buffer_event *event;
 	struct ring_buffer *buffer;
 	unsigned long irq_flags;
+	unsigned long args[6];
 	int pc;
 	int syscall_nr;
 	int size;

@@ -346,7 +347,8 @@ static void ftrace_syscall_enter(void *data, struct pt_regs *regs, long id)
 
 	entry = ring_buffer_event_data(event);
 	entry->nr = syscall_nr;
-	syscall_get_arguments(current, regs, 0, sys_data->nb_args, entry->args);
+	syscall_get_arguments(current, regs, 0, 6, args);
+	memcpy(entry->args, args, sizeof(unsigned long) * sys_data->nb_args);
 
 	event_trigger_unlock_commit(trace_file, buffer, event, entry,
 				    irq_flags, pc);

@@ -564,6 +566,7 @@ static void perf_syscall_enter(void *ignore, struct pt_regs *regs, long id)
 	struct syscall_metadata *sys_data;
 	struct syscall_trace_enter *rec;
 	struct hlist_head *head;
+	unsigned long args[6];
 	int syscall_nr;
 	int rctx;
 	int size;

@@ -592,8 +595,9 @@ static void perf_syscall_enter(void *ignore, struct pt_regs *regs, long id)
 		return;
 
 	rec->nr = syscall_nr;
-	syscall_get_arguments(current, regs, 0, sys_data->nb_args,
-			       (unsigned long *)&rec->args);
+	syscall_get_arguments(current, regs, 0, 6, args);
+	memcpy(&rec->args, args, sizeof(unsigned long) * sys_data->nb_args);
+
 	perf_trace_buf_submit(rec, size, rctx,
 			      sys_data->enter_event->event.type, 1, regs,
 			      head, NULL);
Signature · 1 line
2.9.3
author maintainer
#9 lore raw

RFC 3/3 syscalls: Remove start and number from syscall_get_arguments() args

In reply to Steven Rostedt

From: Steven Rostedt <[email protected]>

At Linux Plumbers, Andy Lutomirski approached me and pointed out that the function call syscall_get_arguments() implemented in x86 was horribly written and not optimized for the standard case of passing in 0 and 6 for the starting index and the number of system calls to get. When looking at all the users of this function, I discovered that all instances pass in only 0 and 6 for these arguments. Instead of having this function handle different cases that are never used, simply rewrite it to return the first 6 arguments of a system call.

This should help out the performance of tracing system calls by ptrace, ftrace and perf.

Reported-by Andy Lutomirski <[email protected]>
Signed-off-by Steven Rostedt <[email protected]>
---
 arch/arc/include/asm/syscall.h        |  7 ++--
 arch/arm/include/asm/syscall.h        | 23 ++---------
 arch/arm64/include/asm/syscall.h      | 22 ++---------
 arch/blackfin/include/asm/syscall.h   | 22 +++++++----
 arch/c6x/include/asm/syscall.h        | 41 ++++----------------
 arch/frv/include/asm/syscall.h        | 26 +++----------
 arch/h8300/include/asm/syscall.h      | 34 ++++-------------
 arch/hexagon/include/asm/syscall.h    |  4 +-
 arch/ia64/include/asm/syscall.h       |  5 +--
 arch/metag/include/asm/syscall.h      |  4 +-
 arch/microblaze/include/asm/syscall.h |  4 +-
 arch/mips/include/asm/syscall.h       |  3 +-
 arch/mn10300/include/asm/syscall.h    | 32 +++-------------
 arch/nios2/include/asm/syscall.h      | 42 ++++----------------
 arch/openrisc/include/asm/syscall.h   |  6 +--
 arch/parisc/include/asm/syscall.h     | 30 ++++-----------
 arch/powerpc/include/asm/syscall.h    |  8 ++--
 arch/s390/include/asm/syscall.h       | 11 +++---
 arch/sh/include/asm/syscall_32.h      | 26 +++----------
 arch/sh/include/asm/syscall_64.h      |  4 +-
 arch/sparc/include/asm/syscall.h      |  4 +-
 arch/tile/include/asm/syscall.h       |  4 +-
 arch/um/include/asm/syscall-generic.h | 39 +++----------------
 arch/x86/include/asm/syscall.h        | 72 ++++++++---------------------------
 include/asm-generic/syscall.h         | 11 ++----
 include/trace/events/syscalls.h       |  2 +-
 kernel/seccomp.c                      |  2 +-
 kernel/trace/trace_syscalls.c         |  4 +-
 lib/syscall.c                         |  2 +-
 29 files changed, 124 insertions(+), 370 deletions(-)
arch/arc/include/asm/syscall.h +3 -4
index 29de09804306..c7a4201ed62b 100644
@@ -55,12 +55,11 @@ syscall_set_return_value(struct task_struct *task, struct pt_regs *regs,
  */
 static inline void
 syscall_get_arguments(struct task_struct *task, struct pt_regs *regs,
-		      unsigned int i, unsigned int n, unsigned long *args)
+		      unsigned long *args)
 {
 	unsigned long *inside_ptregs = &(regs->r0);
-	inside_ptregs -= i;
-
-	BUG_ON((i + n) > 6);
+	unsigned int n = 6;
+	unsigned int i = 0;
 
 	while (n--) {
 		args[i++] = (*inside_ptregs);
arch/arm/include/asm/syscall.h +4 -19
index e86c985b8c7a..1521b465b581 100644
@@ -54,29 +54,14 @@ static inline void syscall_set_return_value(struct task_struct *task,
 
 static inline void syscall_get_arguments(struct task_struct *task,
 					 struct pt_regs *regs,
-					 unsigned int i, unsigned int n,
 					 unsigned long *args)
 {
-	if (n == 0)
-		return;
-
-	if (i + n > SYSCALL_MAX_ARGS) {
-		unsigned long *args_bad = args + SYSCALL_MAX_ARGS - i;
-		unsigned int n_bad = n + i - SYSCALL_MAX_ARGS;
-		pr_warn("%s called with max args %d, handling only %d\n",
-			__func__, i + n, SYSCALL_MAX_ARGS);
-		memset(args_bad, 0, n_bad * sizeof(args[0]));
-		n = SYSCALL_MAX_ARGS - i;
-	}
+	unsigned int n;
 
-	if (i == 0) {
-		args[0] = regs->ARM_ORIG_r0;
-		args++;
-		i++;
-		n--;
-	}
+	args[0] = regs->ARM_ORIG_r0;
+	args++;
 
-	memcpy(args, &regs->ARM_r0 + i, n * sizeof(args[0]));
+	memcpy(args, &regs->ARM_r0 + 1, 5 * sizeof(args[0]));
 }
 
 static inline void syscall_set_arguments(struct task_struct *task,
arch/arm64/include/asm/syscall.h +3 -19
index 709a574468f0..fb6668df435b 100644
@@ -59,28 +59,12 @@ static inline void syscall_set_return_value(struct task_struct *task,
 
 static inline void syscall_get_arguments(struct task_struct *task,
 					 struct pt_regs *regs,
-					 unsigned int i, unsigned int n,
 					 unsigned long *args)
 {
-	if (n == 0)
-		return;
-
-	if (i + n > SYSCALL_MAX_ARGS) {
-		unsigned long *args_bad = args + SYSCALL_MAX_ARGS - i;
-		unsigned int n_bad = n + i - SYSCALL_MAX_ARGS;
-		pr_warning("%s called with max args %d, handling only %d\n",
-			   __func__, i + n, SYSCALL_MAX_ARGS);
-		memset(args_bad, 0, n_bad * sizeof(args[0]));
-	}
-
-	if (i == 0) {
-		args[0] = regs->orig_x0;
-		args++;
-		i++;
-		n--;
-	}
+	args[0] = regs->orig_x0;
+	args++;
 
-	memcpy(args, &regs->regs[i], n * sizeof(args[0]));
+	memcpy(args, &regs->regs[1], 5 * sizeof(args[0]));
 }
 
 static inline void syscall_set_arguments(struct task_struct *task,
arch/blackfin/include/asm/syscall.h +14 -8
index 4921a4815cce..cef32063f54b 100644
@@ -57,30 +57,36 @@ syscall_set_return_value(struct task_struct *task, struct pt_regs *regs,
  *	syscall_get_arguments()
  *	@task:   unused
  *	@regs:   the register layout to extract syscall arguments from
- *	@i:      first syscall argument to extract
- *	@n:      number of syscall arguments to extract
  *	@args:   array to return the syscall arguments in
  *
- * args[0] gets i'th argument, args[n - 1] gets the i+n-1'th argument
+ * Gets the 6 arguments of the system call
  */
 static inline void
 syscall_get_arguments(struct task_struct *task, struct pt_regs *regs,
-                      unsigned int i, unsigned int n, unsigned long *args)
+                      unsigned long *args)
 {
 	/*
 	 * Assume the ptrace layout doesn't change -- r5 is first in memory,
 	 * then r4, ..., then r0.  So we simply reverse the ptrace register
 	 * array in memory to store into the args array.
 	 */
-	long *aregs = &regs->r0 - i;
-
-	BUG_ON(i > 5 || i + n > 6);
+	long *aregs = &regs->r0;
+	unsigned int n = 6;
 
 	while (n--)
 		*args++ = *aregs--;
 }
 
-/* See syscall_get_arguments() comments */
+/**
+ *	syscall_set_arguments()
+ *	@task:   unused
+ *	@regs:   the register layout to extract syscall arguments from
+ *	@i:      first syscall argument to extract
+ *	@n:      number of syscall arguments to extract
+ *	@args:   array to return the syscall arguments in
+ *
+ * args[0] gets i'th argument, args[n - 1] gets the i+n-1'th argument
+ */
 static inline void
 syscall_set_arguments(struct task_struct *task, struct pt_regs *regs,
                       unsigned int i, unsigned int n, const unsigned long *args)
arch/c6x/include/asm/syscall.h +8 -33

48 lines not on this page · raw message

arch/frv/include/asm/syscall.h +6 -20
index 70689eb29b98..4778fa399f52 100644
@@ -71,29 +71,15 @@ static inline void syscall_set_return_value(struct task_struct *task,
  */
 static inline void syscall_get_arguments(struct task_struct *task,
 					 struct pt_regs *regs,
-					 unsigned int i, unsigned int n,
 					 unsigned long *args)
 {
-	/*
-	 * Do this simply for now. If we need to start supporting
-	 * fetching arguments from arbitrary indices, this will need some
-	 * extra logic. Presently there are no in-tree users that depend
-	 * on this behaviour.
-	 */
-	BUG_ON(i);
-
 	/* Argument pattern is: GR8, GR9, GR10, GR11, GR12, GR13 */
-	switch (n) {
-	case 6: args[5] = regs->gr13;
-	case 5: args[4] = regs->gr12;
-	case 4: args[3] = regs->gr11;
-	case 3: args[2] = regs->gr10;
-	case 2: args[1] = regs->gr9;
-	case 1:	args[0] = regs->gr8;
-		break;
-	default:
-		BUG();
-	}
+	args[5] = regs->gr13;
+	args[4] = regs->gr12;
+	args[3] = regs->gr11;
+	args[2] = regs->gr10;
+	args[1] = regs->gr9;
+	args[0] = regs->gr8;
 }
 
 /*
arch/h8300/include/asm/syscall.h +7 -27
index b41f688d02cf..c0bb2e4c152f 100644
@@ -16,34 +16,14 @@ syscall_get_nr(struct task_struct *task, struct pt_regs *regs)
 
 static inline void
 syscall_get_arguments(struct task_struct *task, struct pt_regs *regs,
-		      unsigned int i, unsigned int n, unsigned long *args)
+		      unsigned long *args)
 {
-	BUG_ON(i + n > 6);
-
-	while (n > 0) {
-		switch (i) {
-		case 0:
-			*args++ = regs->er1;
-			break;
-		case 1:
-			*args++ = regs->er2;
-			break;
-		case 2:
-			*args++ = regs->er3;
-			break;
-		case 3:
-			*args++ = regs->er4;
-			break;
-		case 4:
-			*args++ = regs->er5;
-			break;
-		case 5:
-			*args++ = regs->er6;
-			break;
-		}
-		i++;
-		n--;
-	}
+	*args++ = regs->er1;
+	*args++ = regs->er2;
+	*args++ = regs->er3;
+	*args++ = regs->er4;
+	*args++ = regs->er5;
+	*args   = regs->er6;
 }
arch/hexagon/include/asm/syscall.h +1 -3
index 4af9c7b6f13a..ae3a1e24fabd 100644
@@ -37,10 +37,8 @@ static inline long syscall_get_nr(struct task_struct *task,
 
 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);
-	memcpy(args, &(&regs->r00)[i], n * sizeof(args[0]));
+	memcpy(args, &(&regs->r00)[0], 6 * sizeof(args[0]));
 }
 #endif
arch/ia64/include/asm/syscall.h +1 -4
index 1d0b875fec44..8204c1ff70ce 100644
@@ -63,12 +63,9 @@ extern void ia64_syscall_get_set_arguments(struct task_struct *task,
 	unsigned long *args, int rw);
 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);
-
-	ia64_syscall_get_set_arguments(task, regs, i, n, args, 0);
+	ia64_syscall_get_set_arguments(task, regs, 0, 6, args, 0);
 }
 
 static inline void syscall_set_arguments(struct task_struct *task,
arch/metag/include/asm/syscall.h +1 -3
index 24fc97939f77..981bf74c2ec5 100644
@@ -66,13 +66,11 @@ static inline void syscall_set_return_value(struct task_struct *task,
 
 static inline void syscall_get_arguments(struct task_struct *task,
 					 struct pt_regs *regs,
-					 unsigned int i, unsigned int n,
 					 unsigned long *args)
 {
 	unsigned int reg, j;
-	BUG_ON(i + n > 6);
 
-	for (j = i, reg = 6 - i; j < (i + n); j++, reg--) {
+	for (j = 0, reg = 6; j < 6; j++, reg--) {
 		if (reg % 2)
 			args[j] = regs->ctx.DX[(reg + 1) / 2].U0;
 		else
arch/microblaze/include/asm/syscall.h +3 -1
index 04a5bece8168..ec2b0be2076e 100644
@@ -81,9 +81,11 @@ static inline void microblaze_set_syscall_arg(struct pt_regs *regs,
 
 static inline void syscall_get_arguments(struct task_struct *task,
 					 struct pt_regs *regs,
-					 unsigned int i, unsigned int n,
 					 unsigned long *args)
 {
+	unsigned int i = 0;
+	unsigned int n = 6;
+
 	while (n--)
 		*args++ = microblaze_get_syscall_arg(regs, i++);
 }
arch/mips/include/asm/syscall.h +2 -1
index d87882513ee3..279878f58191 100644
@@ -94,9 +94,10 @@ static inline void syscall_set_return_value(struct task_struct *task,
 
 static inline void syscall_get_arguments(struct task_struct *task,
 					 struct pt_regs *regs,
-					 unsigned int i, unsigned int n,
 					 unsigned long *args)
 {
+	unsigned int i = 0;
+	unsigned int n = 6;
 	int ret;
 	/* O32 ABI syscall() - Either 64-bit with O32 or 32-bit */
 	if ((IS_ENABLED(CONFIG_32BIT) ||
arch/mn10300/include/asm/syscall.h +6 -26
index b44b0bb75a01..0b2767144a60 100644
@@ -52,34 +52,14 @@ static inline void syscall_set_return_value(struct task_struct *task,
 
 static inline void syscall_get_arguments(struct task_struct *task,
 					 struct pt_regs *regs,
-					 unsigned int i, unsigned int n,
 					 unsigned long *args)
 {
-	switch (i) {
-	case 0:
-		if (!n--) break;
-		*args++ = regs->a0;
-	case 1:
-		if (!n--) break;
-		*args++ = regs->d1;
-	case 2:
-		if (!n--) break;
-		*args++ = regs->a3;
-	case 3:
-		if (!n--) break;
-		*args++ = regs->a2;
-	case 4:
-		if (!n--) break;
-		*args++ = regs->d3;
-	case 5:
-		if (!n--) break;
-		*args++ = regs->d2;
-	case 6:
-		if (!n--) break;
-	default:
-		BUG();
-		break;
-	}
+	*args++ = regs->a0;
+	*args++ = regs->d1;
+	*args++ = regs->a3;
+	*args++ = regs->a2;
+	*args++ = regs->d3;
+	*args   = regs->d2;
 }
 
 static inline void syscall_set_arguments(struct task_struct *task,
arch/nios2/include/asm/syscall.h +7 -35

49 lines not on this page · raw message

arch/openrisc/include/asm/syscall.h +2 -4
index 2db9f1cf0694..72607860cd55 100644
@@ -56,11 +56,9 @@ syscall_set_return_value(struct task_struct *task, struct pt_regs *regs,
 
 static inline void
 syscall_get_arguments(struct task_struct *task, struct pt_regs *regs,
-		      unsigned int i, unsigned int n, unsigned long *args)
+		      unsigned long *args)
 {
-	BUG_ON(i + n > 6);
-
-	memcpy(args, &regs->gpr[3 + i], n * sizeof(args[0]));
+	memcpy(args, &regs->gpr[3], 6 * sizeof(args[0]));
 }
 
 static inline void
arch/parisc/include/asm/syscall.h +8 -22
index 5e0b4e6bd99d..f7296bf30eb4 100644
@@ -17,29 +17,15 @@ static inline long syscall_get_nr(struct task_struct *tsk,
 }
 
 static inline void syscall_get_arguments(struct task_struct *tsk,
-					 struct pt_regs *regs, unsigned int i,
-					 unsigned int n, unsigned long *args)
+					 struct pt_regs *regs,
+					 unsigned long *args)
 {
-	BUG_ON(i);
-
-	switch (n) {
-	case 6:
-		args[5] = regs->gr[21];
-	case 5:
-		args[4] = regs->gr[22];
-	case 4:
-		args[3] = regs->gr[23];
-	case 3:
-		args[2] = regs->gr[24];
-	case 2:
-		args[1] = regs->gr[25];
-	case 1:
-		args[0] = regs->gr[26];
-	case 0:
-		break;
-	default:
-		BUG();
-	}
+	args[5] = regs->gr[21];
+	args[4] = regs->gr[22];
+	args[3] = regs->gr[23];
+	args[2] = regs->gr[24];
+	args[1] = regs->gr[25];
+	args[0] = regs->gr[26];
 }
 
 static inline long syscall_get_return_value(struct task_struct *task,
arch/powerpc/include/asm/syscall.h +3 -5
index ab9f3f0a8637..70dcb199c47c 100644
@@ -66,22 +66,20 @@ static inline void syscall_set_return_value(struct task_struct *task,
 
 static inline void syscall_get_arguments(struct task_struct *task,
 					 struct pt_regs *regs,
-					 unsigned int i, unsigned int n,
 					 unsigned long *args)
 {
 	unsigned long val, mask = -1UL;
-
-	BUG_ON(i + n > 6);
+	unsigned int n = 6;
 
 #ifdef CONFIG_COMPAT
 	if (test_tsk_thread_flag(task, TIF_32BIT))
 		mask = 0xffffffff;
 #endif
 	while (n--) {
-		if (n == 0 && i == 0)
+		if (n == 0)
 			val = regs->orig_gpr3;
 		else
-			val = regs->gpr[3 + i + n];
+			val = regs->gpr[3 + n];
 
 		args[n] = val & mask;
 	}
arch/s390/include/asm/syscall.h +5 -6
index 6ba0bf928909..2b520aa8ad62 100644
@@ -59,21 +59,20 @@ static inline void syscall_set_return_value(struct task_struct *task,
 
 static inline void syscall_get_arguments(struct task_struct *task,
 					 struct pt_regs *regs,
-					 unsigned int i, unsigned int n,
 					 unsigned long *args)
 {
 	unsigned long mask = -1UL;
+	unsigned int n = 6;
 
-	BUG_ON(i + n > 6);
 #ifdef CONFIG_COMPAT
 	if (test_tsk_thread_flag(task, TIF_31BIT))
 		mask = 0xffffffff;
 #endif
 	while (n-- > 0)
-		if (i + n > 0)
-			args[n] = regs->gprs[2 + i + n] & mask;
-	if (i == 0)
-		args[0] = regs->orig_gpr2 & mask;
+		if (n > 0)
+			args[n] = regs->gprs[2 + n] & mask;
+
+	args[0] = regs->orig_gpr2 & mask;
 }
 
 static inline void syscall_set_arguments(struct task_struct *task,
arch/sh/include/asm/syscall_32.h +6 -20
index 95be3b0ce0ac..09f4942c1b11 100644
@@ -47,30 +47,16 @@ static inline void syscall_set_return_value(struct task_struct *task,
 
 static inline void syscall_get_arguments(struct task_struct *task,
 					 struct pt_regs *regs,
-					 unsigned int i, unsigned int n,
 					 unsigned long *args)
 {
-	/*
-	 * Do this simply for now. If we need to start supporting
-	 * fetching arguments from arbitrary indices, this will need some
-	 * extra logic. Presently there are no in-tree users that depend
-	 * on this behaviour.
-	 */
-	BUG_ON(i);
 
 	/* Argument pattern is: R4, R5, R6, R7, R0, R1 */
-	switch (n) {
-	case 6: args[5] = regs->regs[1];
-	case 5: args[4] = regs->regs[0];
-	case 4: args[3] = regs->regs[7];
-	case 3: args[2] = regs->regs[6];
-	case 2: args[1] = regs->regs[5];
-	case 1:	args[0] = regs->regs[4];
-	case 0:
-		break;
-	default:
-		BUG();
-	}
+	args[5] = regs->regs[1];
+	args[4] = regs->regs[0];
+	args[3] = regs->regs[7];
+	args[2] = regs->regs[6];
+	args[1] = regs->regs[5];
+	args[0] = regs->regs[4];
 }
 
 static inline void syscall_set_arguments(struct task_struct *task,
arch/sh/include/asm/syscall_64.h +1 -3
index c6a797b90b80..0c5d28f47827 100644
@@ -46,11 +46,9 @@ static inline void syscall_set_return_value(struct task_struct *task,
 
 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);
-	memcpy(args, &regs->regs[2 + i], n * sizeof(args[0]));
+	memcpy(args, &regs->regs[2], 6 * sizeof(args[0]));
 }
 
 static inline void syscall_set_arguments(struct task_struct *task,
arch/sparc/include/asm/syscall.h +2 -2
index 1757cd6c521b..419ed27ae60b 100644
@@ -95,11 +95,11 @@ static inline void syscall_set_return_value(struct task_struct *task,
 
 static inline void syscall_get_arguments(struct task_struct *task,
 					 struct pt_regs *regs,
-					 unsigned int i, unsigned int n,
 					 unsigned long *args)
 {
 	int zero_extend = 0;
 	unsigned int j;
+	unsigned int n = 6;
 
 #ifdef CONFIG_SPARC64
 	if (test_tsk_thread_flag(task, TIF_32BIT))

@@ -107,7 +107,7 @@ static inline void syscall_get_arguments(struct task_struct *task,
 #endif
 
 	for (j = 0; j < n; j++) {
-		unsigned long val = regs->u_regs[UREG_I0 + i + j];
+		unsigned long val = regs->u_regs[UREG_I0 + j];
 
 		if (zero_extend)
 			args[j] = (u32) val;
arch/tile/include/asm/syscall.h +1 -3
index 373d73064ea1..42c6204b3c30 100644
@@ -76,11 +76,9 @@ static inline void syscall_set_return_value(struct task_struct *task,
 
 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);
-	memcpy(args, &regs[i], n * sizeof(args[0]));
+	memcpy(args, &regs[0], 6 * sizeof(args[0]));
 }
 
 static inline void syscall_set_arguments(struct task_struct *task,
arch/um/include/asm/syscall-generic.h +6 -33
index 9fb9cf8cd39a..25d00acd1322 100644
@@ -53,43 +53,16 @@ static inline void syscall_set_return_value(struct task_struct *task,
 
 static inline void syscall_get_arguments(struct task_struct *task,
 					 struct pt_regs *regs,
-					 unsigned int i, unsigned int n,
 					 unsigned long *args)
 {
 	const struct uml_pt_regs *r = &regs->regs;
 
-	switch (i) {
-	case 0:
-		if (!n--)
-			break;
-		*args++ = UPT_SYSCALL_ARG1(r);
-	case 1:
-		if (!n--)
-			break;
-		*args++ = UPT_SYSCALL_ARG2(r);
-	case 2:
-		if (!n--)
-			break;
-		*args++ = UPT_SYSCALL_ARG3(r);
-	case 3:
-		if (!n--)
-			break;
-		*args++ = UPT_SYSCALL_ARG4(r);
-	case 4:
-		if (!n--)
-			break;
-		*args++ = UPT_SYSCALL_ARG5(r);
-	case 5:
-		if (!n--)
-			break;
-		*args++ = UPT_SYSCALL_ARG6(r);
-	case 6:
-		if (!n--)
-			break;
-	default:
-		BUG();
-		break;
-	}
+	*args++ = UPT_SYSCALL_ARG1(r);
+	*args++ = UPT_SYSCALL_ARG2(r);
+	*args++ = UPT_SYSCALL_ARG3(r);
+	*args++ = UPT_SYSCALL_ARG4(r);
+	*args++ = UPT_SYSCALL_ARG5(r);
+	*args   = UPT_SYSCALL_ARG6(r);
 }
 
 static inline void syscall_set_arguments(struct task_struct *task,
arch/x86/include/asm/syscall.h +16 -56

90 lines not on this page · raw message

include/asm-generic/syscall.h +3 -8
index 0c938a4354f6..269e9412ef42 100644
@@ -105,21 +105,16 @@ void syscall_set_return_value(struct task_struct *task, struct pt_regs *regs,
  * syscall_get_arguments - extract system call parameter values
  * @task:	task of interest, must be blocked
  * @regs:	task_pt_regs() of @task
- * @i:		argument index [0,5]
- * @n:		number of arguments; n+i must be [1,6].
  * @args:	array filled with argument values
  *
- * Fetches @n arguments to the system call starting with the @i'th argument
- * (from 0 through 5).  Argument @i is stored in @args[0], and so on.
- * An arch inline version is probably optimal when @i and @n are constants.
+ * Fetches 6 arguments to the system call.  First argument is stored in
+*  @args[0], and so on.
  *
  * 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.
  */
 void syscall_get_arguments(struct task_struct *task, struct pt_regs *regs,
-			   unsigned int i, unsigned int n, unsigned long *args);
+			   unsigned long *args);
 
 /**
  * syscall_set_arguments - change system call parameter value
include/trace/events/syscalls.h +1 -1
index 14e49c798135..72aa1694a6cf 100644
@@ -26,7 +26,7 @@ TRACE_EVENT_FN(sys_enter,
 
 	TP_fast_assign(
 		__entry->id	= id;
-		syscall_get_arguments(current, regs, 0, 6, __entry->args);
+		syscall_get_arguments(current, regs, __entry->args);
 	),
 
 	TP_printk("NR %ld (%lx, %lx, %lx, %lx, %lx, %lx)",
kernel/seccomp.c +1 -1
index 0db7c8a2afe2..31d1a3c04bec 100644
@@ -75,7 +75,7 @@ static void populate_seccomp_data(struct seccomp_data *sd)
 
 	sd->nr = syscall_get_nr(task, regs);
 	sd->arch = syscall_get_arch();
-	syscall_get_arguments(task, regs, 0, 6, args);
+	syscall_get_arguments(task, regs, args);
 	sd->args[0] = args[0];
 	sd->args[1] = args[1];
 	sd->args[2] = args[2];
kernel/trace/trace_syscalls.c +2 -2
index 52c1503cedfa..3aff1f5e7fc3 100644
@@ -347,7 +347,7 @@ static void ftrace_syscall_enter(void *data, struct pt_regs *regs, long id)
 
 	entry = ring_buffer_event_data(event);
 	entry->nr = syscall_nr;
-	syscall_get_arguments(current, regs, 0, 6, args);
+	syscall_get_arguments(current, regs, args);
 	memcpy(entry->args, args, sizeof(unsigned long) * sys_data->nb_args);
 
 	event_trigger_unlock_commit(trace_file, buffer, event, entry,

@@ -595,7 +595,7 @@ static void perf_syscall_enter(void *ignore, struct pt_regs *regs, long id)
 		return;
 
 	rec->nr = syscall_nr;
-	syscall_get_arguments(current, regs, 0, 6, args);
+	syscall_get_arguments(current, regs, args);
 	memcpy(&rec->args, args, sizeof(unsigned long) * sys_data->nb_args);
 
 	perf_trace_buf_submit(rec, size, rctx,
lib/syscall.c +1 -1
index cbd376c66bbc..cc83c9e2d682 100644
@@ -26,7 +26,7 @@ static int collect_syscall(struct task_struct *target, long *callno,
 
 	*callno = syscall_get_nr(target, regs);
 	if (*callno != -1L)
-		syscall_get_arguments(target, regs, 0, 6, args);
+		syscall_get_arguments(target, regs, args);
 
 	put_task_stack(target);
 	return 0;
Signature · 1 line
2.9.3
reviewer Reviewed-by on v1 3/3
#10 lore raw

Re: [RFC][ATCH 3/3] syscalls: Remove start and number from syscall_get_arguments() args

arch/x86/include/asm/syscall.h

Quoted 7 lines · Steven Rostedt

From: Steven Rostedt <[email protected]>

arch/x86/include/asm/syscall.h +0 -0
index e3c95e8e61c5..050891169b51 100644

x86 part is:

Reviewed-by Andy Lutomirski <[email protected]>

Although I wonder why this takes a task argument.

author maintainer
#11 lore raw

Re: [RFC][ATCH 3/3] syscalls: Remove start and number from syscall_get_arguments() args

Quoted 11 lines · On Mon, 7 Nov 2016 15:54:14 -0800 Andy Lutomirski <[email protected]> wrote:

arch/x86/include/asm/syscall.h

Quoted 7 lines · Steven Rostedt

From: Steven Rostedt <[email protected]>

arch/x86/include/asm/syscall.h +0 -0
index e3c95e8e61c5..050891169b51 100644

x86 part is:

Quoted 3 lines
Reviewed-by Andy Lutomirski <[email protected]>

Although I wonder why this takes a task argument.

Thanks, although kbuild bot discovered a slight mistake:

	} else {
# endif
		*args++ = regs->di;

Should be:

	} else
# endif
	{
		*args++ = regs->di;

I'll update, and also look at making it a struct.

-- Steve