From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:58112) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TsJg0-0001Nu-Nx for qemu-devel@nongnu.org; Mon, 07 Jan 2013 15:52:11 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TsJfv-0001PK-Ty for qemu-devel@nongnu.org; Mon, 07 Jan 2013 15:52:08 -0500 Received: from moutng.kundenserver.de ([212.227.17.8]:60364) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TsJfv-0001Nk-J0 for qemu-devel@nongnu.org; Mon, 07 Jan 2013 15:52:03 -0500 Message-ID: <1357591919.3256.3.camel@Quad> From: Laurent Vivier Date: Mon, 07 Jan 2013 21:51:59 +0100 In-Reply-To: References: <1357590634-9768-1-git-send-email-laurent@vivier.eu> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH] linux-user: correct reboot() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: Riku Voipio , qemu-devel@nongnu.org Le lundi 07 janvier 2013 =C3=A0 20:42 +0000, Peter Maydell a =C3=A9crit : > On 7 January 2013 20:30, Laurent Vivier wrote: > > According to man reboot(2), the 4th argument is only used with > > LINUX_REBOOT_CMD_RESTART2. In other cases, trying to convert > > the value can generate EFAULT. > > > > Signed-off-by: Laurent Vivier > > --- > > linux-user/syscall.c | 14 ++++++++++---- > > 1 file changed, 10 insertions(+), 4 deletions(-) > > > > diff --git a/linux-user/syscall.c b/linux-user/syscall.c > > index 3167a87..730e428 100644 > > --- a/linux-user/syscall.c > > +++ b/linux-user/syscall.c > > @@ -101,6 +101,7 @@ int __clone2(int (*fn)(void *), void *child_stack_b= ase, > > #include > > #include > > #include > > +#include > > #include "linux_loop.h" > > #include "cpu-uname.h" > > > > @@ -6415,10 +6416,15 @@ abi_long do_syscall(void *cpu_env, int num, abi= _long arg1, > > break; > > #endif > > case TARGET_NR_reboot: > > - if (!(p =3D lock_user_string(arg4))) > > - goto efault; > > - ret =3D reboot(arg1, arg2, arg3, p); > > - unlock_user(p, arg4, 0); > > + if (arg3 =3D=3D LINUX_REBOOT_CMD_RESTART2) { > > + /* arg4 must be ignored in all other cases */ > > + if (!(p =3D lock_user_string(arg4))) > > + goto efault; >=20 > Coding style requires braces; please use checkpatch.pl. Yes, sorry for that. >=20 > > + ret =3D reboot(arg1, arg2, arg3, p); > > + unlock_user(p, arg4, 0); > > + } else { > > + ret =3D reboot(arg1, arg2, arg3, (void*)(unsigned long)arg4= ); >=20 > I don't think we should pass arg4 in this case. It's a pointer, so it's > definitely wrong to pass a pointer we haven't converted somehow. > Just passing NULL would be better, I think; that will be safe and > make it reasonably obvious we need to fix something if the kernel > ever for some reason adds a new command that takes an argument. Yes, but in the traces I have, arg4 is 1. Can we accept to loose it ? Regards, Laurent --=20 "Just play. Have fun. Enjoy the game." - Michael Jordan