From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1LvDDW-0007qR-Nz for qemu-devel@nongnu.org; Sat, 18 Apr 2009 12:16:34 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1LvDDV-0007p4-O9 for qemu-devel@nongnu.org; Sat, 18 Apr 2009 12:16:33 -0400 Received: from [199.232.76.173] (port=44444 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1LvDDV-0007ou-9s for qemu-devel@nongnu.org; Sat, 18 Apr 2009 12:16:33 -0400 Received: from hall.aurel32.net ([88.191.82.174]:36327) by monty-python.gnu.org with esmtps (TLS-1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1LvDDU-0006CW-KW for qemu-devel@nongnu.org; Sat, 18 Apr 2009 12:16:33 -0400 Date: Sat, 18 Apr 2009 18:16:30 +0200 From: Aurelien Jarno Subject: Re: [Qemu-devel] [PATCH 04/10] fix IPCOP_sem* and implement sem* Message-ID: <20090418161630.GA29955@volta.aurel32.net> References: <20090415152823.GA9642@volta.aurel32.net> <20090416142309.GH28127@kos.to> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-15 Content-Disposition: inline In-Reply-To: <20090416142309.GH28127@kos.to> Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Riku Voipio Cc: qemu-devel@nongnu.org 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 > > + __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 > Date: Fri, 3 Apr 2009 00:28:14 +0300 > Subject: [PATCH] [v2] fix IPCOP_sem* and implement sem* > > From: Kirill A. Shutemov > > 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 > --- > 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 + __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 + __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 + __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 aurelien@aurel32.net http://www.aurel32.net