[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 04/10] fix IPCOP_sem* and implement sem*
From: |
Aurelien Jarno |
Subject: |
Re: [Qemu-devel] [PATCH 04/10] fix IPCOP_sem* and implement sem* |
Date: |
Sat, 18 Apr 2009 18:16:30 +0200 |
User-agent: |
Mutt/1.5.18 (2008-05-17) |
On Thu, Apr 16, 2009 at 05:23:09PM +0300, Riku Voipio wrote:
> On Wed, Apr 15, 2009 at 05:28:23PM +0200, Aurelien Jarno wrote:
> > > +struct target_sembuf {
> > > + unsigned short sem_num;
> > > + short sem_op;
> > > + short sem_flg;
> > > +};
> > > +
> > > +static inline abi_long target_to_host_sembuf(struct sembuf *host_sembuf,
> > > + abi_ulong target_addr,
> > > + unsigned nsops)
> > > +{
> > > + struct target_sembuf *target_sembuf;
> > > + int i;
> > > +
> > > + target_sembuf = lock_user(VERIFY_READ, target_addr,
> > > + nsops*sizeof(struct target_sembuf), 1);
> > > + if (!target_sembuf)
> > > + return -TARGET_EFAULT;
> > > +
> > > + for(i=0; i<nsops; i++) {
> > > + __put_user(target_sembuf[i].sem_num, &host_sembuf[i].sem_num);
> > > + __put_user(target_sembuf[i].sem_op, &host_sembuf[i].sem_op);
> > > + __put_user(target_sembuf[i].sem_flg, &host_sembuf[i].sem_flg);
> > > + }
>
> > I don't follow fully the subtle difference between __put_user() and
> > __get_user,() but given lock_user is called with VERIFY_READ, I think
> > __get_user() should be used instead.
>
> Yes.. since the target and host args are swapped, it still works like
> __get_user.
> Corrected version attached.
Thanks, applied.
> (This error appears also once in the shm* patch too)
>
> From 32b512d9b521d2b146658f932b9c3553cd3ec1cd Mon Sep 17 00:00:00 2001
> From: Riku Voipio <address@hidden>
> Date: Fri, 3 Apr 2009 00:28:14 +0300
> Subject: [PATCH] [v2] fix IPCOP_sem* and implement sem*
>
> From: Kirill A. Shutemov <address@hidden>
>
> Fix and cleanup IPCOP_sem* ipc calls handling and
> implement sem* syscalls.
>
> Riku:
>
> 1) Uglify whitespace so that diff gets smaller and easier
> to review
>
> 2) use __get_user in target_to_host_sembuf
>
> Signed-off-by: Riku Voipio <address@hidden>
> ---
> linux-user/syscall.c | 286
> +++++++++++++++++++++++++++++++++-----------------
> 1 files changed, 188 insertions(+), 98 deletions(-)
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 74b41a8..92e5159 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -2005,7 +2005,8 @@ static inline abi_long target_to_host_semid_ds(struct
> semid_ds *host_sd,
>
> if (!lock_user_struct(VERIFY_READ, target_sd, target_addr, 1))
> return -TARGET_EFAULT;
> - target_to_host_ipc_perm(&(host_sd->sem_perm),target_addr);
> + if (target_to_host_ipc_perm(&(host_sd->sem_perm),target_addr))
> + return -TARGET_EFAULT;
> host_sd->sem_nsems = tswapl(target_sd->sem_nsems);
> host_sd->sem_otime = tswapl(target_sd->sem_otime);
> host_sd->sem_ctime = tswapl(target_sd->sem_ctime);
> @@ -2020,7 +2021,8 @@ static inline abi_long
> host_to_target_semid_ds(abi_ulong target_addr,
>
> if (!lock_user_struct(VERIFY_WRITE, target_sd, target_addr, 0))
> return -TARGET_EFAULT;
> - host_to_target_ipc_perm(target_addr,&(host_sd->sem_perm));
> + if (host_to_target_ipc_perm(target_addr,&(host_sd->sem_perm)))
> + return -TARGET_EFAULT;;
> target_sd->sem_nsems = tswapl(host_sd->sem_nsems);
> target_sd->sem_otime = tswapl(host_sd->sem_otime);
> target_sd->sem_ctime = tswapl(host_sd->sem_ctime);
> @@ -2028,135 +2030,214 @@ static inline abi_long
> host_to_target_semid_ds(abi_ulong target_addr,
> return 0;
> }
>
> +struct target_seminfo {
> + int semmap;
> + int semmni;
> + int semmns;
> + int semmnu;
> + int semmsl;
> + int semopm;
> + int semume;
> + int semusz;
> + int semvmx;
> + int semaem;
> +};
> +
> +static inline abi_long host_to_target_seminfo(abi_ulong target_addr,
> + struct seminfo *host_seminfo)
> +{
> + struct target_seminfo *target_seminfo;
> + if (!lock_user_struct(VERIFY_WRITE, target_seminfo, target_addr, 0))
> + return -TARGET_EFAULT;
> + __put_user(host_seminfo->semmap, &target_seminfo->semmap);
> + __put_user(host_seminfo->semmni, &target_seminfo->semmni);
> + __put_user(host_seminfo->semmns, &target_seminfo->semmns);
> + __put_user(host_seminfo->semmnu, &target_seminfo->semmnu);
> + __put_user(host_seminfo->semmsl, &target_seminfo->semmsl);
> + __put_user(host_seminfo->semopm, &target_seminfo->semopm);
> + __put_user(host_seminfo->semume, &target_seminfo->semume);
> + __put_user(host_seminfo->semusz, &target_seminfo->semusz);
> + __put_user(host_seminfo->semvmx, &target_seminfo->semvmx);
> + __put_user(host_seminfo->semaem, &target_seminfo->semaem);
> + unlock_user_struct(target_seminfo, target_addr, 1);
> + return 0;
> +}
> +
> union semun {
> int val;
> struct semid_ds *buf;
> unsigned short *array;
> + struct seminfo *__buf;
> };
>
> union target_semun {
> int val;
> - abi_long buf;
> - unsigned short int *array;
> + abi_ulong buf;
> + abi_ulong array;
> + abi_ulong __buf;
> };
>
> -static inline abi_long target_to_host_semun(int cmd,
> - union semun *host_su,
> - abi_ulong target_addr,
> - struct semid_ds *ds)
> +static inline abi_long target_to_host_semarray(int semid, unsigned short
> **host_array,
> + abi_ulong target_addr)
> {
> - union target_semun *target_su;
> + int nsems;
> + unsigned short *array;
> + union semun semun;
> + struct semid_ds semid_ds;
> + int i, ret;
>
> - switch( cmd ) {
> - case IPC_STAT:
> - case IPC_SET:
> - if (!lock_user_struct(VERIFY_READ, target_su, target_addr, 1))
> - return -TARGET_EFAULT;
> - target_to_host_semid_ds(ds,target_su->buf);
> - host_su->buf = ds;
> - unlock_user_struct(target_su, target_addr, 0);
> - break;
> - case GETVAL:
> - case SETVAL:
> - if (!lock_user_struct(VERIFY_READ, target_su, target_addr, 1))
> - return -TARGET_EFAULT;
> - host_su->val = tswapl(target_su->val);
> - unlock_user_struct(target_su, target_addr, 0);
> - break;
> - case GETALL:
> - case SETALL:
> - if (!lock_user_struct(VERIFY_READ, target_su, target_addr, 1))
> - return -TARGET_EFAULT;
> - *host_su->array = tswap16(*target_su->array);
> - unlock_user_struct(target_su, target_addr, 0);
> - break;
> - default:
> - gemu_log("semun operation not fully supported: %d\n", (int)cmd);
> + semun.buf = &semid_ds;
> +
> + ret = semctl(semid, 0, IPC_STAT, semun);
> + if (ret == -1)
> + return get_errno(ret);
> +
> + nsems = semid_ds.sem_nsems;
> +
> + *host_array = malloc(nsems*sizeof(unsigned short));
> + array = lock_user(VERIFY_READ, target_addr,
> + nsems*sizeof(unsigned short), 1);
> + if (!array)
> + return -TARGET_EFAULT;
> +
> + for(i=0; i<nsems; i++) {
> + __get_user((*host_array)[i], &array[i]);
> }
> + unlock_user(array, target_addr, 0);
> +
> return 0;
> }
>
> -static inline abi_long host_to_target_semun(int cmd,
> - abi_ulong target_addr,
> - union semun *host_su,
> - struct semid_ds *ds)
> +static inline abi_long host_to_target_semarray(int semid, abi_ulong
> target_addr,
> + unsigned short **host_array)
> {
> - union target_semun *target_su;
> + int nsems;
> + unsigned short *array;
> + union semun semun;
> + struct semid_ds semid_ds;
> + int i, ret;
>
> - switch( cmd ) {
> - case IPC_STAT:
> - case IPC_SET:
> - if (lock_user_struct(VERIFY_WRITE, target_su, target_addr, 0))
> - return -TARGET_EFAULT;
> - host_to_target_semid_ds(target_su->buf,ds);
> - unlock_user_struct(target_su, target_addr, 1);
> - break;
> - case GETVAL:
> - case SETVAL:
> - if (lock_user_struct(VERIFY_WRITE, target_su, target_addr, 0))
> - return -TARGET_EFAULT;
> - target_su->val = tswapl(host_su->val);
> - unlock_user_struct(target_su, target_addr, 1);
> - break;
> - case GETALL:
> - case SETALL:
> - if (lock_user_struct(VERIFY_WRITE, target_su, target_addr, 0))
> - return -TARGET_EFAULT;
> - *target_su->array = tswap16(*host_su->array);
> - unlock_user_struct(target_su, target_addr, 1);
> - break;
> - default:
> - gemu_log("semun operation not fully supported: %d\n", (int)cmd);
> + semun.buf = &semid_ds;
> +
> + ret = semctl(semid, 0, IPC_STAT, semun);
> + if (ret == -1)
> + return get_errno(ret);
> +
> + nsems = semid_ds.sem_nsems;
> +
> + array = lock_user(VERIFY_WRITE, target_addr,
> + nsems*sizeof(unsigned short), 0);
> + if (!array)
> + return -TARGET_EFAULT;
> +
> + for(i=0; i<nsems; i++) {
> + __put_user((*host_array)[i], &array[i]);
> }
> + free(*host_array);
> + unlock_user(array, target_addr, 1);
> +
> return 0;
> }
>
> -static inline abi_long do_semctl(int first, int second, int third,
> - abi_long ptr)
> +static inline abi_long do_semctl(int semid, int semnum, int cmd,
> + union target_semun target_su)
> {
> union semun arg;
> struct semid_ds dsarg;
> - int cmd = third&0xff;
> - abi_long ret = 0;
> + unsigned short *array;
> + struct seminfo seminfo;
> + abi_long ret = -TARGET_EINVAL;
> + abi_long err;
> + cmd &= 0xff;
>
> switch( cmd ) {
> case GETVAL:
> - target_to_host_semun(cmd,&arg,ptr,&dsarg);
> - ret = get_errno(semctl(first, second, cmd, arg));
> - host_to_target_semun(cmd,ptr,&arg,&dsarg);
> - break;
> case SETVAL:
> - target_to_host_semun(cmd,&arg,ptr,&dsarg);
> - ret = get_errno(semctl(first, second, cmd, arg));
> - host_to_target_semun(cmd,ptr,&arg,&dsarg);
> + arg.val = tswapl(target_su.val);
> + ret = get_errno(semctl(semid, semnum, cmd, arg));
> + target_su.val = tswapl(arg.val);
> break;
> case GETALL:
> - target_to_host_semun(cmd,&arg,ptr,&dsarg);
> - ret = get_errno(semctl(first, second, cmd, arg));
> - host_to_target_semun(cmd,ptr,&arg,&dsarg);
> - break;
> case SETALL:
> - target_to_host_semun(cmd,&arg,ptr,&dsarg);
> - ret = get_errno(semctl(first, second, cmd, arg));
> - host_to_target_semun(cmd,ptr,&arg,&dsarg);
> + err = target_to_host_semarray(semid, &array, 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;
> break;
> case IPC_STAT:
> - target_to_host_semun(cmd,&arg,ptr,&dsarg);
> - ret = get_errno(semctl(first, second, cmd, arg));
> - host_to_target_semun(cmd,ptr,&arg,&dsarg);
> - break;
> case IPC_SET:
> - target_to_host_semun(cmd,&arg,ptr,&dsarg);
> - ret = get_errno(semctl(first, second, cmd, arg));
> - host_to_target_semun(cmd,ptr,&arg,&dsarg);
> + case SEM_STAT:
> + err = target_to_host_semid_ds(&dsarg, 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;
> + 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;
> + break;
> + case IPC_RMID:
> + case GETPID:
> + case GETNCNT:
> + case GETZCNT:
> + ret = get_errno(semctl(semid, semnum, cmd, NULL));
> break;
> - default:
> - ret = get_errno(semctl(first, second, cmd, arg));
> }
>
> return ret;
> }
>
> +struct target_sembuf {
> + unsigned short sem_num;
> + short sem_op;
> + short sem_flg;
> +};
> +
> +static inline abi_long target_to_host_sembuf(struct sembuf *host_sembuf,
> + abi_ulong target_addr,
> + unsigned nsops)
> +{
> + struct target_sembuf *target_sembuf;
> + int i;
> +
> + target_sembuf = lock_user(VERIFY_READ, target_addr,
> + nsops*sizeof(struct target_sembuf), 1);
> + if (!target_sembuf)
> + return -TARGET_EFAULT;
> +
> + for(i=0; i<nsops; i++) {
> + __get_user(host_sembuf[i].sem_num, &target_sembuf[i].sem_num);
> + __get_user(host_sembuf[i].sem_op, &target_sembuf[i].sem_op);
> + __get_user(host_sembuf[i].sem_flg, &target_sembuf[i].sem_flg);
> + }
> +
> + unlock_user(target_sembuf, target_addr, 0);
> +
> + return 0;
> +}
> +
> +static inline abi_long do_semop(int semid, abi_long ptr, unsigned nsops)
> +{
> + struct sembuf sops[nsops];
> +
> + if (target_to_host_sembuf(sops, ptr, nsops))
> + return -TARGET_EFAULT;
> +
> + return semop(semid, sops, nsops);
> +}
> +
> struct target_msqid_ds
> {
> struct target_ipc_perm msg_perm;
> @@ -2360,7 +2441,7 @@ static abi_long do_ipc(unsigned int call, int first,
>
> switch (call) {
> case IPCOP_semop:
> - ret = get_errno(semop(first,(struct sembuf *)g2h(ptr), second));
> + ret = do_semop(first, ptr, second);
> break;
>
> case IPCOP_semget:
> @@ -2368,12 +2449,7 @@ static abi_long do_ipc(unsigned int call, int first,
> break;
>
> case IPCOP_semctl:
> - ret = do_semctl(first, second, third, ptr);
> - break;
> -
> - case IPCOP_semtimedop:
> - gemu_log("Unsupported ipc call: %d (version %d)\n", call, version);
> - ret = -TARGET_ENOSYS;
> + ret = do_semctl(first, second, third, (union
> target_semun)(abi_ulong) ptr);
> break;
>
> case IPCOP_msgget:
> @@ -5184,7 +5260,21 @@ abi_long do_syscall(void *cpu_env, int num, abi_long
> arg1,
> ret = do_ipc(arg1, arg2, arg3, arg4, arg5, arg6);
> break;
> #endif
> -
> +#ifdef TARGET_NR_semget
> + case TARGET_NR_semget:
> + ret = get_errno(semget(arg1, arg2, arg3));
> + break;
> +#endif
> +#ifdef TARGET_NR_semop
> + case TARGET_NR_semop:
> + ret = get_errno(do_semop(arg1, arg2, arg3));
> + break;
> +#endif
> +#ifdef TARGET_NR_semctl
> + case TARGET_NR_semctl:
> + ret = do_semctl(arg1, arg2, arg3, (union
> target_semun)(abi_ulong)arg4);
> + break;
> +#endif
> #ifdef TARGET_NR_msgctl
> case TARGET_NR_msgctl:
> ret = do_msgctl(arg1, arg2, arg3);
> --
> 1.6.2.1
>
>
--
Aurelien Jarno GPG: 1024D/F1BCDB73
address@hidden http://www.aurel32.net
- [Qemu-devel] [PATCH 08/10] linux-user: removed unnecessary MAX_SOCK_ADDR checks for socket syscalls, (continued)