From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37629) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YQXVj-0006vv-Nu for qemu-devel@nongnu.org; Wed, 25 Feb 2015 03:40:09 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YQXVe-0004uj-QY for qemu-devel@nongnu.org; Wed, 25 Feb 2015 03:40:03 -0500 Date: Wed, 25 Feb 2015 09:39:36 +0100 (CET) From: Laurent Vivier Message-ID: <1414725099.11975.1424853577110.JavaMail.open-xchange@oxbsltgw39.schlund.de> In-Reply-To: <1424814498-6993-30-git-send-email-mdroth@linux.vnet.ibm.com> References: <1424814498-6993-1-git-send-email-mdroth@linux.vnet.ibm.com> <1424814498-6993-30-git-send-email-mdroth@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: multipart/alternative; boundary="----=_Part_11974_299243952.1424853576014" Subject: Re: [Qemu-devel] [PATCH 29/43] linux-user: Fix broken m68k signal handling on 64 bit hosts Reply-To: Laurent Vivier List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org, Michael Roth Cc: Peter Maydell , Riku Voipio , qemu-stable@nongnu.org ------=_Part_11974_299243952.1424853576014 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Hi, =20 I think you should use abi_long instead of uint32_t. =20 abi_long has an "aligned" attribute, and on m68k long are aligned on a shor= t boundary. =20 #ifdef TARGET_M68K #define ABI_INT_ALIGNMENT 2 #define ABI_LONG_ALIGNMENT 2 #define ABI_LLONG_ALIGNMENT 2 #endif typedef uint32_t abi_ulong __attribute__((aligned(ABI_LONG_ALIGNMENT))); Regards, Laurent > Le 24 f=C3=A9vrier 2015 =C3=A0 22:48, Michael Roth a =C3=A9crit : > > > From: Peter Maydell > > The m68k signal frame setup code which writes the signal return > trampoline code to the stack was assuming that a 'long' was 32 bits; > on 64 bit systems this meant we would end up writing the 32 bit > (2 insn) trampoline sequence to retaddr+4,retaddr+6 instead of > the intended retaddr+0,retaddr+2, resulting in a guest crash when > it tried to execute the invalid zero-bytes at retaddr+0. > Fix by using uint32_t instead; also use uint16_t rather than short > for consistency. This fixes bug LP:1404690. > > Reported-by: Michel Boaventura > Signed-off-by: Peter Maydell > Signed-off-by: Riku Voipio > (cherry picked from commit 1669add752d9f29283f8ebf6a863d7b1e2d0f146) > Signed-off-by: Michael Roth > --- > linux-user/signal.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/linux-user/signal.c b/linux-user/signal.c > index e11b208..a324fd1 100644 > --- a/linux-user/signal.c > +++ b/linux-user/signal.c > @@ -5091,7 +5091,7 @@ static void setup_frame(int sig, struct target_siga= ction > *ka, > /* moveq #,d0; trap #0 */ > > __put_user(0x70004e40 + (TARGET_NR_sigreturn << 16), > - (long *)(frame->retcode)); > + (uint32_t *)(frame->retcode)); > > /* Set up to return from userspace */ > > @@ -5225,8 +5225,8 @@ static void setup_rt_frame(int sig, struct > target_sigaction *ka, > /* moveq #,d0; notb d0; trap #0 */ > > __put_user(0x70004600 + ((TARGET_NR_rt_sigreturn ^ 0xff) << 16), > - (long *)(frame->retcode + 0)); > - __put_user(0x4e40, (short *)(frame->retcode + 4)); > + (uint32_t *)(frame->retcode + 0)); > + __put_user(0x4e40, (uint16_t *)(frame->retcode + 4)); > > if (err) > goto give_sigsegv; > -- > 1.9.1 > > ------=_Part_11974_299243952.1424853576014 MIME-Version: 1.0 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: 7bit
Hi,
 
I think you should use abi_long instead of  uint32_t.
 
abi_long has an "aligned" attribute, and on m68k long are aligned on a short boundary.
 

#ifdef TARGET_M68K
#define ABI_INT_ALIGNMENT 2
#define ABI_LONG_ALIGNMENT 2
#define ABI_LLONG_ALIGNMENT 2
#endif

typedef uint32_t abi_ulong __attribute__((aligned(ABI_LONG_ALIGNMENT)));

Regards,
Laurent

> Le 24 février 2015 à 22:48, Michael Roth <mdroth@linux.vnet.ibm.com> a écrit :
>
>
> From: Peter Maydell <peter.maydell@linaro.org>
>
> The m68k signal frame setup code which writes the signal return
> trampoline code to the stack was assuming that a 'long' was 32 bits;
> on 64 bit systems this meant we would end up writing the 32 bit
> (2 insn) trampoline sequence to retaddr+4,retaddr+6 instead of
> the intended retaddr+0,retaddr+2, resulting in a guest crash when
> it tried to execute the invalid zero-bytes at retaddr+0.
> Fix by using uint32_t instead; also use uint16_t rather than short
> for consistency. This fixes bug LP:1404690.
>
> Reported-by: Michel Boaventura
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Riku Voipio <riku.voipio@linaro.org>
> (cherry picked from commit 1669add752d9f29283f8ebf6a863d7b1e2d0f146)
> Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> ---
> linux-user/signal.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/linux-user/signal.c b/linux-user/signal.c
> index e11b208..a324fd1 100644
> --- a/linux-user/signal.c
> +++ b/linux-user/signal.c
> @@ -5091,7 +5091,7 @@ static void setup_frame(int sig, struct target_sigaction *ka,
> /* moveq #,d0; trap #0 */
>
> __put_user(0x70004e40 + (TARGET_NR_sigreturn << 16),
> - (long *)(frame->retcode));
> + (uint32_t *)(frame->retcode));
>
> /* Set up to return from userspace */
>
> @@ -5225,8 +5225,8 @@ static void setup_rt_frame(int sig, struct target_sigaction *ka,
> /* moveq #,d0; notb d0; trap #0 */
>
> __put_user(0x70004600 + ((TARGET_NR_rt_sigreturn ^ 0xff) << 16),
> - (long *)(frame->retcode + 0));
> - __put_user(0x4e40, (short *)(frame->retcode + 4));
> + (uint32_t *)(frame->retcode + 0));
> + __put_user(0x4e40, (uint16_t *)(frame->retcode + 4));
>
> if (err)
> goto give_sigsegv;
> --
> 1.9.1
>
>
------=_Part_11974_299243952.1424853576014--