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

[Peter Maydell]

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.

> --- 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
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().

-- PMM