qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Peter Maydell
Subject: Re: [Qemu-devel] [PATCH][v2] linux-user: correct semctl() and shmctl()
Date: Sun, 20 Jan 2013 11:24:15 +0000

On 2 January 2013 20:38, Laurent Vivier <address@hidden> wrote:
> The parameter "union semun" of semctl() is not a value
> but a pointer to the value.
>
> Moreover, all fields of target_su must be swapped (if needed).
>
> The third argument of shmctl is a pointer.
>
> WITHOUT this patch:
>
> $ ipcs
>
> kernel not configured for shared memory
>
> qemu: uncaught target signal 11 (Segmentation fault) - core dumped
>
> WITH this patch:
>
> $ ipcs
>
> ------ Shared Memory Segments --------
> key        shmid      owner      perms      bytes      nattch     status
> 0x4e545030 0          root      600        96         1
> 0x4e545031 32769      root      600        96         1
> 0x4e545032 65538      root      666        96         1
> 0x4e545033 98307      root      666        96         1
> 0x47505344 131076     root      666        8240       1
> 0x3c81b7f5 163845     laurent   666        4096       0
> 0x00000000 729513990  laurent   600        393216     2          dest
> 0x00000000 729546759  laurent   600        393216     2          dest
> 0x00000000 1879179273 laurent   600        393216     2          dest
>
> ------ Semaphore Arrays --------
> key        semid      owner      perms      nsems
> 0x3c81b7f6 32768      laurent   666        1
> 0x1c44ac47 6586369    laurent   600        1
>
> ------ Message Queues --------
> key        msqid      owner      perms      used-bytes   messages
> 0x1c44ac45 458752     laurent    600        0            0
> 0x1c44ac46 491521     laurent    600        0            0
>
> Signed-off-by: Laurent Vivier <address@hidden>
> ---
> [v2] move lock_user_struct() in do_semctl()
>
>  linux-user/syscall.c |   39 ++++++++++++++++++++-------------------
>  1 file changed, 20 insertions(+), 19 deletions(-)
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index e99adab..b2687e1 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -2637,8 +2637,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;
> @@ -2647,43 +2648,42 @@ 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;
> +    }
>      switch( cmd ) {
>         case GETVAL:
>         case SETVAL:
> -            arg.val = tswap32(target_su.val);
> +            arg.val = tswap32(target_su->val);
>              ret = get_errno(semctl(semid, semnum, cmd, arg));
> -            target_su.val = tswap32(arg.val);
> +            target_su->val = tswap32(arg.val);
>              break;
>         case GETALL:
>         case SETALL:
> -            err = target_to_host_semarray(semid, &array, target_su.array);
> +            err = target_to_host_semarray(semid, &array,
> +                                          tswapal(target_su->array));
>              if (err)
> -                return err;
> +                break;

(1) Coding style demands braces
(2) More importantly, this is going to break the return value -- instead
of returning 'err' we will break out of the switch and then return 'ret'.
There are similar issues in other cases.

-- PMM



reply via email to

[Prev in Thread] Current Thread [Next in Thread]