From patchwork Fri Aug 5 00:05:49 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: An-Cheng Huang X-Patchwork-Id: 108588 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [140.186.70.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 96C00B6F6F for ; Fri, 5 Aug 2011 10:06:15 +1000 (EST) Received: from localhost ([::1]:55108 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Qp7vR-0005eT-4Z for incoming@patchwork.ozlabs.org; Thu, 04 Aug 2011 20:06:05 -0400 Received: from eggs.gnu.org ([140.186.70.92]:40765) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Qp7vJ-0005e9-Lk for qemu-devel@nongnu.org; Thu, 04 Aug 2011 20:05:58 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Qp7vI-00004u-0P for qemu-devel@nongnu.org; Thu, 04 Aug 2011 20:05:57 -0400 Received: from na3sys009aog117.obsmtp.com ([74.125.149.242]:60472) by eggs.gnu.org with smtp (Exim 4.71) (envelope-from ) id 1Qp7vH-0008WU-Lz for qemu-devel@nongnu.org; Thu, 04 Aug 2011 20:05:55 -0400 Received: from mail-iy0-f175.google.com ([209.85.210.175]) (using TLSv1) by na3sys009aob117.postini.com ([74.125.148.12]) with SMTP ID DSNKTjsz4AG4ZcpejHNWCptBn33+bwxMQ65h@postini.com; Thu, 04 Aug 2011 17:05:55 PDT Received: by mail-iy0-f175.google.com with SMTP id 15so1490020iyn.6 for ; Thu, 04 Aug 2011 17:05:52 -0700 (PDT) Received: by 10.42.197.1 with SMTP id ei1mr1313192icb.222.1312502752696; Thu, 04 Aug 2011 17:05:52 -0700 (PDT) Received: from debian.localdomain (204.11.230.58.static.etheric.net [204.11.230.58]) by mx.google.com with ESMTPS id h6sm3257618icy.1.2011.08.04.17.05.51 (version=SSLv3 cipher=OTHER); Thu, 04 Aug 2011 17:05:52 -0700 (PDT) Date: Thu, 4 Aug 2011 17:05:49 -0700 From: An-Cheng Huang To: Peter Maydell Message-ID: <20110805000549.GB5323@debian.localdomain> References: <20110804221600.GA5323@debian.localdomain> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.20 (2009-06-14) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6, seldom 2.4 (older, 4) X-Received-From: 74.125.149.242 Cc: riku.voipio@iki.fi, qemu-devel@nongnu.org Subject: Re: [Qemu-devel] [PATCH] linux-user: Fix indirect syscall handling for MIPS X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org On Thu, Aug 04, 2011 at 11:43:31PM +0100, Peter Maydell wrote: > On 4 August 2011 23:16, An-Cheng Huang 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 --- 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. */