| Message ID | 20110804221600.GA5323@debian.localdomain |
|---|---|
| State | New |
| Headers | show |
On 4 August 2011 23:16, An-Cheng Huang <ancheng@ubnt.com> wrote: > I ran into the problem of indirect syscalls not working with > mips-linux-user and found that the number of arguments for sys_syscall > is 0 in the mips_syscall_args table, which means the "higher" arguments > (5, 6, 7, and 8) are never obtained from the stack for the do_syscall() > invocation for indirect syscalls. So the actual syscall will not get the > correct argument(s) if it needs more than three. Yes, I noticed this last time I was looking at this code. > A simpler approach would be to just change the number of arguments for > sys_syscall to 8 in the mips_syscall_args table so that for indirect > syscalls the "higher" arguments are always taken from the stack with > get_user_ual(). However, since there is a comment about "what to do > if get_user() fails", I don't know if this may cause breakage when the > arguments are not actually there? If someone can confirm that this is > harmless, the simple approach is probably better? Thanks. In fact the Linux kernel will always read all four arguments off the stack for sys_syscall, regardless: http://lxr.linux.no/#linux+v3.0/arch/mips/kernel/scall32-o32.S#L188 So setting sys_syscall to 8 is not just easier but actually the Right Thing. The comment about get_user() is cut-n-paste from various other places in the file where it applies just as much -- it is no more of an issue for MIPS or for sys_syscall than for any other architecture or syscall. [ie it is a bug, but not in practice a very serious one, and you can ignore it for the purposes of fixing the bug you've found here.] Incidentally, you can find the answer to the "what if get_user fails" question for MIPS here: http://lxr.linux.no/#linux+v3.0/arch/mips/kernel/scall32-o32.S#L166 ...we should set ret to -TARGET_EFAULT and skip the call to do_syscall. -- PMM
diff --git a/linux-user/main.c b/linux-user/main.c index 6a8f4bd..1560c1c 100644 --- a/linux-user/main.c +++ b/linux-user/main.c @@ -2068,27 +2068,42 @@ static int do_store_exclusive(CPUMIPSState *env) void cpu_loop(CPUMIPSState *env) { target_siginfo_t info; int trapnr, ret; unsigned int syscall_num; + unsigned int syscall_idx; + unsigned int real_syscall_idx; /* only for indirect syscall */ for(;;) { cpu_exec_start(env); trapnr = cpu_mips_exec(env); cpu_exec_end(env); switch(trapnr) { case EXCP_SYSCALL: - syscall_num = env->active_tc.gpr[2] - 4000; + syscall_num = env->active_tc.gpr[2]; + syscall_idx = syscall_num - 4000; + real_syscall_idx = env->active_tc.gpr[4] - 4000; env->active_tc.PC += 4; - if (syscall_num >= sizeof(mips_syscall_args)) { + if (syscall_idx >= sizeof(mips_syscall_args) + || (syscall_num == TARGET_NR_syscall + && real_syscall_idx >= sizeof(mips_syscall_args))) { + /* invalid direct or indirect syscalls */ ret = -TARGET_ENOSYS; } else { int nb_args; abi_ulong sp_reg; abi_ulong arg5 = 0, arg6 = 0, arg7 = 0, arg8 = 0; - nb_args = mips_syscall_args[syscall_num]; + if (syscall_num == TARGET_NR_syscall) { + /* indirect syscall, so need to look at the real + * syscall to figure out nb_args. also, plus 1 is + * needed to account for the indirect syscall itself. + */ + nb_args = mips_syscall_args[real_syscall_idx] + 1; + } else { + nb_args = mips_syscall_args[syscall_idx]; + } sp_reg = env->active_tc.gpr[29]; switch (nb_args) { /* these arguments are taken from the stack */ /* FIXME - what to do if get_user() fails? */ case 8: get_user_ual(arg8, sp_reg + 28);
I ran into the problem of indirect syscalls not working with mips-linux-user and found that the number of arguments for sys_syscall is 0 in the mips_syscall_args table, which means the "higher" arguments (5, 6, 7, and 8) are never obtained from the stack for the do_syscall() invocation for indirect syscalls. So the actual syscall will not get the correct argument(s) if it needs more than three. The patch checks for indirect syscall and in such cases uses the actual syscall number to look up the number of arguments so that the actual arguments can be taken from the stack. A simpler approach would be to just change the number of arguments for sys_syscall to 8 in the mips_syscall_args table so that for indirect syscalls the "higher" arguments are always taken from the stack with get_user_ual(). However, since there is a comment about "what to do if get_user() fails", I don't know if this may cause breakage when the arguments are not actually there? If someone can confirm that this is harmless, the simple approach is probably better? Thanks. Signed-off-by: An-Cheng Huang <ancheng@ubnt.com> --- linux-user/main.c | 21 ++++++++++++++++++--- 1 files changed, 18 insertions(+), 3 deletions(-)