[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH][v4] linux-user: correct semctl() and shmctl()
|
From: |
Laurent Vivier |
|
Subject: |
Re: [Qemu-devel] [PATCH][v4] linux-user: correct semctl() and shmctl() |
|
Date: |
Thu, 31 Jan 2013 20:40:36 +0100 |
Sorry, this one is wrong, I missed some returns...
Regards,
LAurent
Le jeudi 31 janvier 2013 à 20:36 +0100, Laurent Vivier a écrit :
> 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()
> v3: correctly set the return value
> v3: don't duplicat unlock_user_struct(), set err to ret instead
>
> linux-user/syscall.c | 44 +++++++++++++++++++++++++++-----------------
> 1 file changed, 27 insertions(+), 17 deletions(-)
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 08538fc..1aef535 100644
> --- 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,51 @@ 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;
> arg.array = array;
> ret = get_errno(semctl(semid, semnum, cmd, arg));
> - err = host_to_target_semarray(semid, target_su.array, &array);
> - if (err)
> - return err;
> + err = host_to_target_semarray(semid, tswapal(target_su->array),
> + &array);
> + if (err) {
> + ret = err;
> + }
> break;
> case IPC_STAT:
> case IPC_SET:
> case SEM_STAT:
> - err = target_to_host_semid_ds(&dsarg, target_su.buf);
> + err = target_to_host_semid_ds(&dsarg, tswapal(target_su->buf));
> if (err)
> return err;
> arg.buf = &dsarg;
> ret = get_errno(semctl(semid, semnum, cmd, arg));
> - err = host_to_target_semid_ds(target_su.buf, &dsarg);
> - if (err)
> - return err;
> + err = host_to_target_semid_ds(tswapal(target_su->buf), &dsarg);
> + if (err) {
> + ret = err;
> + }
> break;
> case IPC_INFO:
> case SEM_INFO:
> arg.__buf = &seminfo;
> ret = get_errno(semctl(semid, semnum, cmd, arg));
> - err = host_to_target_seminfo(target_su.__buf, &seminfo);
> - if (err)
> - return err;
> + err = host_to_target_seminfo(tswapal(target_su->__buf),
> &seminfo);
> + if (err) {
> + ret = err;
> + }
> break;
> case IPC_RMID:
> case GETPID:
> @@ -2707,6 +2716,7 @@ static inline abi_long do_semctl(int semid, int semnum,
> int cmd,
> ret = get_errno(semctl(semid, semnum, cmd, NULL));
> break;
> }
> + unlock_user_struct(target_su, ptr, 0);
>
> return ret;
> }
> @@ -3177,7 +3187,7 @@ static abi_long do_ipc(unsigned int call, int first,
> break;
>
> case IPCOP_semctl:
> - ret = do_semctl(first, second, third, (union
> target_semun)(abi_ulong) ptr);
> + ret = do_semctl(first, second, third, ptr);
> break;
>
> case IPCOP_msgget:
> @@ -3244,7 +3254,7 @@ static abi_long do_ipc(unsigned int call, int first,
>
> /* IPC_* and SHM_* command values are the same on all linux platforms */
> case IPCOP_shmctl:
> - ret = do_shmctl(first, second, third);
> + ret = do_shmctl(first, second, ptr);
> break;
> default:
> gemu_log("Unsupported ipc call: %d (version %d)\n", call, version);
> @@ -6933,7 +6943,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long
> arg1,
> #endif
> #ifdef TARGET_NR_semctl
> case TARGET_NR_semctl:
> - ret = do_semctl(arg1, arg2, arg3, (union
> target_semun)(abi_ulong)arg4);
> + ret = do_semctl(arg1, arg2, arg3, arg4);
> break;
> #endif
> #ifdef TARGET_NR_msgctl
--
"Just play. Have fun. Enjoy the game."
- Michael Jordan