On Thu, Aug 04, 2011 at 11:43:31PM +0100, Peter Maydell wrote:
> On 4 August 2011 23:16, An-Cheng Huang <anch...@ubnt.com> wrote:
> > 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.
Ok the following patch changes the number of arguments for sys_syscall to 8 in
mips_syscall_args and also skips the do_syscall() call if any of the get_user()
calls fails. Do you think combining these makes sense or should they be two
separate patches? Thanks.
Signed-off-by: An-Cheng Huang <anch...@ubnt.com>
---
linux-user/main.c | 24 ++++++++++++++++++------
1 files changed, 18 insertions(+), 6 deletions(-)
diff --git a/linux-user/main.c b/linux-user/main.c
index 6a8f4bd..701d96e 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -1669,7 +1669,7 @@ void cpu_loop(CPUPPCState *env)
#define MIPS_SYS(name, args) args,
static const uint8_t mips_syscall_args[] = {
- MIPS_SYS(sys_syscall , 0) /* 4000 */
+ MIPS_SYS(sys_syscall , 8) /* 4000 */
MIPS_SYS(sys_exit , 1)
MIPS_SYS(sys_fork , 0)
MIPS_SYS(sys_read , 3)
@@ -2090,11 +2090,22 @@ void cpu_loop(CPUMIPSState *env)
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);
- case 7: get_user_ual(arg7, sp_reg + 24);
- case 6: get_user_ual(arg6, sp_reg + 20);
- case 5: get_user_ual(arg5, sp_reg + 16);
+ case 8:
+ if ((ret = get_user_ual(arg8, sp_reg + 28)) != 0) {
+ goto done_syscall;
+ }
+ case 7:
+ if ((ret = get_user_ual(arg7, sp_reg + 24)) != 0) {
+ goto done_syscall;
+ }
+ case 6:
+ if ((ret = get_user_ual(arg6, sp_reg + 20)) != 0) {
+ goto done_syscall;
+ }
+ case 5:
+ if ((ret = get_user_ual(arg5, sp_reg + 16)) != 0) {
+ goto done_syscall;
+ }
default:
break;
}
@@ -2105,6 +2116,7 @@ void cpu_loop(CPUMIPSState *env)
env->active_tc.gpr[7],
arg5, arg6, arg7, arg8);
}
+done_syscall:
if (ret == -TARGET_QEMU_ESIGRETURN) {
/* Returning from a successful sigreturn syscall.
Avoid clobbering register state. */
