[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH][v5] linux-user: correct semctl() and shmctl()
From: |
Laurent Vivier |
Subject: |
Re: [Qemu-devel] [PATCH][v5] linux-user: correct semctl() and shmctl() |
Date: |
Mon, 04 Feb 2013 22:03:30 +0100 |
Le lundi 04 février 2013 à 15:16 +0000, Peter Maydell a écrit :
> On 31 January 2013 19:50, Laurent Vivier <address@hidden> wrote:
> > The parameter "union semun" of semctl() is not a value
> > but a pointer to the value.
>
> Hi. For your next patch could you make sure you send it as
> a fresh email rather than a followup to the previous version?
> Anthony's patch-handling tools don't really like followups.
OK
> > --- a/linux-user/syscall.c
> > +++ b/linux-user/syscall.c
> > @@ -2652,8 +2652,9 @@ static inline abi_long host_to_target_semarray(int
> > semid, abi_ulong target_addr,
> > }
> >
> > static inline abi_long do_semctl(int semid, int semnum, int cmd,
> > - union target_semun target_su)
> > + abi_ulong ptr)
> > {
> > + union target_semun *target_su;
> > union semun arg;
> > struct semid_ds dsarg;
> > unsigned short *array = NULL;
> > @@ -2662,43 +2663,55 @@ static inline abi_long do_semctl(int semid, int
> > semnum, int cmd,
> > abi_long err;
> > cmd &= 0xff;
> >
> > + if (!lock_user_struct(VERIFY_READ, target_su, ptr, 1)) {
> > + return -TARGET_EFAULT;
> > + }
>
> This breaks x86_64 linux-user. The fourth argument to semctl()
> is a union of pointers, not a pointer to a union. That means that
In fact, it depends on the architecture. After a look in the kernel
sources, it seems compat_sys_semctl() uses a pointer, sys_semctl() an
union. compat_sys_semctl() seems to be used by mips32, pp32, sparc32 and
x86_32.
> the lock_user_struct/whatever has to be done differently for the
> individual cases, depending on how we are supposed to interpret
> the argument (which field of the union we're using).
>
> My testcase is simple:
>
> QEMU_STRACE=1 ./x86_64-linux-user/qemu-x86_64 /usr/bin/ipcs
>
> which before your patch does this:
>
> 14654 semctl(0,0,SEM_INFO,0x0000004000800490) = 0
> 14654 write(1,0x10d4000,33)------ Semaphore Arrays --------
> = 33
>
> (ie we successfully get back the info)
>
> 14654 write(1,0x10d4000,55)key semid owner perms
> nsems
> = 55
> 14654 semctl(0,0,SEM_STAT,0x0000004000800420) = -1 errno=22 (Invalid argument)
> 14654 write(1,0x10d4000,1)
> = 1
>
> and afterwards does this:
>
> 14723 semctl(0,0,SEM_INFO,0x0000004000800490) = -1 errno=14 (Bad address)
> 14723 write(1,0x10d4000,37)kernel not configured for semaphores
> = 37
>
> (SEM_INFO fails and ipcs prints a failure message)
>
> because we end up with target_su->__buf == 11 which isn't a
> valid address to pass to host_to_target_seminfo().
Thank you for your help,
Laurent
--
"Just play. Have fun. Enjoy the game."
- Michael Jordan