From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45667) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XH6ed-00064w-3X for qemu-devel@nongnu.org; Tue, 12 Aug 2014 03:38:00 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XH6eb-00048j-Sq for qemu-devel@nongnu.org; Tue, 12 Aug 2014 03:37:59 -0400 Received: from [2001:4b98:dc0:45:216:3eff:fe3d:166f] (port=42126 helo=afflict.kos.to) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XH6eb-00048a-Hy for qemu-devel@nongnu.org; Tue, 12 Aug 2014 03:37:57 -0400 Date: Tue, 12 Aug 2014 10:37:57 +0300 From: Riku Voipio Message-ID: <20140812073757.GD5555@afflict.kos.to> References: <1407595352-12821-1-git-send-email-peter.maydell@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1407595352-12821-1-git-send-email-peter.maydell@linaro.org> Subject: Re: [Qemu-devel] [PATCH] linux-user: Fix conversion of sigevent argument to timer_create List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: Riku Voipio , qemu-devel@nongnu.org, Erik de Castro Lopo , patches@linaro.org On Sat, Aug 09, 2014 at 03:42:32PM +0100, Peter Maydell wrote: > There were a number of bugs in the conversion of the sigevent > argument to timer_create from target to host format: > * signal number not converted from target to host > * thread ID not copied across > * sigev_value not copied across > * we never unlocked the struct when we were done > > Between them, these problems meant that SIGEV_THREAD_ID > timers (and the glibc-implemented SIGEV_THREAD timers which > depend on them) didn't work. > > Fix these problems and clean up the code a little by pulling > the struct conversion out into its own function, in line with > how we convert various other structs. This allows the test > program in bug LP:1042388 to run. > > Signed-off-by: Peter Maydell Thanks, applied to linux-user - replacing Erik's patch. Riku > --- > Riku: this is going to conflict with and supersede Erik's oneliner > patch which adds the unlock_user_struct() (I'm afraid I forgot about > that when I was writing this and independently re-fixed the bug), > so it might be simplest to drop that from the linux-user queue if > you apply this instead. Otherwise if that patch makes it to master > I'll rebase and resend this one at that point. > > linux-user/syscall.c | 38 ++++++++++++++++++++++++++++++-------- > 1 file changed, 30 insertions(+), 8 deletions(-) > > diff --git a/linux-user/syscall.c b/linux-user/syscall.c > index a50229d..4ce7455 100644 > --- a/linux-user/syscall.c > +++ b/linux-user/syscall.c > @@ -4914,6 +4914,32 @@ static inline abi_long host_to_target_itimerspec(abi_ulong target_addr, > return 0; > } > > +static inline abi_long target_to_host_sigevent(struct sigevent *host_sevp, > + abi_ulong target_addr) > +{ > + struct target_sigevent *target_sevp; > + > + if (!lock_user_struct(VERIFY_READ, target_sevp, target_addr, 1)) { > + return -TARGET_EFAULT; > + } > + > + /* This union is awkward on 64 bit systems because it has a 32 bit > + * integer and a pointer in it; we follow the conversion approach > + * used for handling sigval types in signal.c so the guest should get > + * the correct value back even if we did a 64 bit byteswap and it's > + * using the 32 bit integer. > + */ > + host_sevp->sigev_value.sival_ptr = > + (void *)(uintptr_t)tswapal(target_sevp->sigev_value.sival_ptr); > + host_sevp->sigev_signo = > + target_to_host_signal(tswap32(target_sevp->sigev_signo)); > + host_sevp->sigev_notify = tswap32(target_sevp->sigev_notify); > + host_sevp->_sigev_un._tid = tswap32(target_sevp->_sigev_un._tid); > + > + unlock_user_struct(target_sevp, target_addr, 1); > + return 0; > +} > + > #if defined(TARGET_NR_stat64) || defined(TARGET_NR_newfstatat) > static inline abi_long host_to_target_stat64(void *cpu_env, > abi_ulong target_addr, > @@ -9413,7 +9439,6 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, > /* args: clockid_t clockid, struct sigevent *sevp, timer_t *timerid */ > > struct sigevent host_sevp = { {0}, }, *phost_sevp = NULL; > - struct target_sigevent *ptarget_sevp; > struct target_timer_t *ptarget_timer; > > int clkid = arg1; > @@ -9425,14 +9450,11 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, > timer_t *phtimer = g_posix_timers + timer_index; > > if (arg2) { > - if (!lock_user_struct(VERIFY_READ, ptarget_sevp, arg2, 1)) { > - goto efault; > - } > - > - host_sevp.sigev_signo = tswap32(ptarget_sevp->sigev_signo); > - host_sevp.sigev_notify = tswap32(ptarget_sevp->sigev_notify); > - > phost_sevp = &host_sevp; > + ret = target_to_host_sigevent(phost_sevp, arg2); > + if (ret != 0) { > + break; > + } > } > > ret = get_errno(timer_create(clkid, phost_sevp, phtimer)); > -- > 1.9.1 >