Re: [Qemu-devel] [PATCH][v5] linux-user: correct semctl() and shmctl()

[Laurent Vivier]

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