* [Qemu-devel] [PATCH] linux-user EFAULT implementation @ 2007-07-06 15:15 Stuart Anderson 2007-07-06 16:08 ` Fabrice Bellard 0 siblings, 1 reply; 13+ messages in thread From: Stuart Anderson @ 2007-07-06 15:15 UTC (permalink / raw) To: qemu-devel [-- Attachment #1: Type: TEXT/PLAIN, Size: 660 bytes --] Here is a refresh of the linux-user EFAULT patch vs todays cvs. This patch has now received a lot more runtime, and a couple of bug have been fixed that affected some of the socket APIs. The purpose of this patch is to detect bad pointers and return EFAULT instead of just crashing when the bad pointers are used by qemu. Stuart Stuart R. Anderson anderson@netsweng.com Network & Software Engineering http://www.netsweng.com/ 1024D/37A79149: 0791 D3B8 9A4C 2CDC A31F BD03 0A62 E534 37A7 9149 [-- Attachment #2: efault patch --] [-- Type: TEXT/x-diff, Size: 45594 bytes --] Index: qemu/exec.c =================================================================== --- qemu.orig/exec.c 2007-07-06 09:46:45.000000000 -0400 +++ qemu/exec.c 2007-07-06 09:49:37.000000000 -0400 @@ -1862,6 +1862,29 @@ spin_unlock(&tb_lock); } +int page_check_range(target_ulong start, target_ulong len, int flags) +{ + PageDesc *p; + target_ulong end; + target_ulong addr; + + end = TARGET_PAGE_ALIGN(start+len); /* must do before we loose bits in the next step */ + start = start & TARGET_PAGE_MASK; + + if( end < start ) return EFAULT; /* we've wrapped around */ + for(addr = start; addr < end; addr += TARGET_PAGE_SIZE) { + p = page_find(addr >> TARGET_PAGE_BITS); + if( !p ) return EFAULT; + if( !(p->flags & PAGE_VALID) ) return EFAULT; + + if (!(p->flags & PAGE_READ) && + (flags & PAGE_READ) ) return EFAULT; + if (!(p->flags & PAGE_WRITE) && + (flags & PAGE_WRITE) ) return EFAULT; + } + return 0; +} + /* called from signal handler: invalidate the code and unprotect the page. Return TRUE if the fault was succesfully handled. */ int page_unprotect(target_ulong address, unsigned long pc, void *puc) Index: qemu/cpu-all.h =================================================================== --- qemu.orig/cpu-all.h 2007-07-06 09:47:42.000000000 -0400 +++ qemu/cpu-all.h 2007-07-06 09:49:37.000000000 -0400 @@ -691,6 +691,7 @@ int page_get_flags(target_ulong address); void page_set_flags(target_ulong start, target_ulong end, int flags); void page_unprotect_range(target_ulong data, target_ulong data_size); +int page_check_range(target_ulong start, target_ulong len, int flags); CPUState *cpu_copy(CPUState *env); Index: qemu/linux-user/syscall.c =================================================================== --- qemu.orig/linux-user/syscall.c 2007-07-06 09:46:45.000000000 -0400 +++ qemu/linux-user/syscall.c 2007-07-06 09:49:37.000000000 -0400 @@ -404,12 +404,13 @@ #endif } -static inline void host_to_target_rusage(target_ulong target_addr, +static inline long host_to_target_rusage(target_ulong target_addr, const struct rusage *rusage) { + long ret = 0; struct target_rusage *target_rusage; - lock_user_struct(target_rusage, target_addr, 0); + if( (ret=lock_and_check_user_struct(&target_rusage,target_addr,sizeof(*target_rusage),0,PAGE_WRITE)) != 0 ) return -ret; target_rusage->ru_utime.tv_sec = tswapl(rusage->ru_utime.tv_sec); target_rusage->ru_utime.tv_usec = tswapl(rusage->ru_utime.tv_usec); target_rusage->ru_stime.tv_sec = tswapl(rusage->ru_stime.tv_sec); @@ -431,26 +432,32 @@ unlock_user_struct(target_rusage, target_addr, 1); } -static inline void target_to_host_timeval(struct timeval *tv, +static inline long target_to_host_timeval(struct timeval *tv, target_ulong target_addr) { + long ret = 0; struct target_timeval *target_tv; - lock_user_struct(target_tv, target_addr, 1); + if( (ret=lock_and_check_user_struct(&target_tv,target_addr,sizeof(*target_tv),1,PAGE_READ)) != 0 ) return -ret; tv->tv_sec = tswapl(target_tv->tv_sec); tv->tv_usec = tswapl(target_tv->tv_usec); unlock_user_struct(target_tv, target_addr, 0); + + return ret; } -static inline void host_to_target_timeval(target_ulong target_addr, +static inline long host_to_target_timeval(target_ulong target_addr, const struct timeval *tv) { + long ret = 0; struct target_timeval *target_tv; - lock_user_struct(target_tv, target_addr, 0); + if( (ret=lock_and_check_user_struct(&target_tv,target_addr,sizeof(*target_tv),0,PAGE_WRITE)) != 0 ) return -ret; target_tv->tv_sec = tswapl(tv->tv_sec); target_tv->tv_usec = tswapl(tv->tv_usec); unlock_user_struct(target_tv, target_addr, 1); + + return ret; } @@ -462,25 +469,25 @@ fd_set *rfds_ptr, *wfds_ptr, *efds_ptr; target_long *target_rfds, *target_wfds, *target_efds; struct timeval tv, *tv_ptr; - long ret; + long ret,ret2; int ok; if (rfd_p) { - target_rfds = lock_user(rfd_p, sizeof(target_long) * n, 1); + if( (ret=lock_and_check_user_struct(&target_rfds,rfd_p,sizeof(target_long)*n,1,PAGE_READ)) != 0 ) return -ret; rfds_ptr = target_to_host_fds(&rfds, target_rfds, n); } else { target_rfds = NULL; rfds_ptr = NULL; } if (wfd_p) { - target_wfds = lock_user(wfd_p, sizeof(target_long) * n, 1); + if( (ret=lock_and_check_user_struct(&target_wfds,wfd_p,sizeof(target_long)*n,1,PAGE_READ)) != 0 ) return -ret; wfds_ptr = target_to_host_fds(&wfds, target_wfds, n); } else { target_wfds = NULL; wfds_ptr = NULL; } if (efd_p) { - target_efds = lock_user(efd_p, sizeof(target_long) * n, 1); + if( (ret=lock_and_check_user_struct(&target_efds,efd_p,sizeof(target_long)*n,1,PAGE_READ)) != 0 ) return -ret; efds_ptr = target_to_host_fds(&efds, target_efds, n); } else { target_efds = NULL; @@ -488,7 +495,7 @@ } if (target_tv) { - target_to_host_timeval(&tv, target_tv); + if( (ret=target_to_host_timeval(&tv, target_tv)) != 0 ) return ret; tv_ptr = &tv; } else { tv_ptr = NULL; @@ -502,7 +509,7 @@ host_to_target_fds(target_efds, efds_ptr, n); if (target_tv) { - host_to_target_timeval(target_tv, &tv); + if( (ret2=host_to_target_timeval(target_tv, &tv)) != 0 ) return ret2; } } if (target_rfds) @@ -515,28 +522,34 @@ return ret; } -static inline void target_to_host_sockaddr(struct sockaddr *addr, +static inline long target_to_host_sockaddr(struct sockaddr *addr, target_ulong target_addr, - socklen_t len) + socklen_t len, + int pg_access) { + long ret = 0; struct target_sockaddr *target_saddr; - target_saddr = lock_user(target_addr, len, 1); + if( (ret=lock_and_check_user_struct(&target_saddr,target_addr,len,1,pg_access)) != 0 ) return ret; memcpy(addr, target_saddr, len); addr->sa_family = tswap16(target_saddr->sa_family); unlock_user(target_saddr, target_addr, 0); + return ret; } -static inline void host_to_target_sockaddr(target_ulong target_addr, +static inline long host_to_target_sockaddr(target_ulong target_addr, struct sockaddr *addr, - socklen_t len) + socklen_t len, + int pg_access) { + long ret = 0; struct target_sockaddr *target_saddr; - target_saddr = lock_user(target_addr, len, 0); + if( (ret=lock_and_check_user_struct(&target_saddr,target_addr,len,1,pg_access)) != 0 ) return ret; memcpy(target_saddr, addr, len); target_saddr->sa_family = tswap16(addr->sa_family); unlock_user(target_saddr, target_addr, len); + return ret; } /* ??? Should this also swap msgh->name? */ @@ -897,18 +910,20 @@ static long do_bind(int sockfd, target_ulong target_addr, socklen_t addrlen) { + long ret = 0; void *addr = alloca(addrlen); - target_to_host_sockaddr(addr, target_addr, addrlen); + if( (ret=target_to_host_sockaddr(addr, target_addr, addrlen,PAGE_READ)) != 0 ) return -ret; return get_errno(bind(sockfd, addr, addrlen)); } static long do_connect(int sockfd, target_ulong target_addr, socklen_t addrlen) { + long ret = 0; void *addr = alloca(addrlen); - target_to_host_sockaddr(addr, target_addr, addrlen); + if( (ret=target_to_host_sockaddr(addr, target_addr, addrlen,PAGE_READ)) != 0 ) return -ret; return get_errno(connect(sockfd, addr, addrlen)); } @@ -922,12 +937,23 @@ struct iovec *vec; target_ulong target_vec; - lock_user_struct(msgp, target_msg, 1); + if( send ) { + if( (ret=lock_and_check_user_struct(&msgp,target_msg,sizeof(*msgp),1, + PAGE_READ)) != 0 ) return -ret; + } else { + if( (ret=lock_and_check_user_struct(&msgp,target_msg,sizeof(*msgp),1, + PAGE_WRITE)) != 0 ) return -ret; + } if (msgp->msg_name) { msg.msg_namelen = tswap32(msgp->msg_namelen); msg.msg_name = alloca(msg.msg_namelen); - target_to_host_sockaddr(msg.msg_name, tswapl(msgp->msg_name), - msg.msg_namelen); + if( send ) { + if( (ret=target_to_host_sockaddr(msg.msg_name, tswapl(msgp->msg_name), + msg.msg_namelen,PAGE_READ)) != 0 ) return -ret; + } else { + if( (ret=target_to_host_sockaddr(msg.msg_name, tswapl(msgp->msg_name), + msg.msg_namelen,PAGE_WRITE)) != 0 ) return -ret; + } } else { msg.msg_name = NULL; msg.msg_namelen = 0; @@ -958,13 +984,16 @@ static long do_accept(int fd, target_ulong target_addr, target_ulong target_addrlen) { - socklen_t addrlen = tget32(target_addrlen); + socklen_t addrlen; void *addr = alloca(addrlen); - long ret; + long ret,ret2; + + if( (ret2=page_check_range(target_addrlen,sizeof(socklen_t),PAGE_WRITE)) != 0 ) return -EINVAL; + addrlen = tget32(target_addrlen); ret = get_errno(accept(fd, addr, &addrlen)); if (!is_error(ret)) { - host_to_target_sockaddr(target_addr, addr, addrlen); + if( (ret2=host_to_target_sockaddr(target_addr, addr, addrlen, PAGE_WRITE)) != 0 ) return -ret2; tput32(target_addrlen, addrlen); } return ret; @@ -973,13 +1002,18 @@ static long do_getpeername(int fd, target_ulong target_addr, target_ulong target_addrlen) { - socklen_t addrlen = tget32(target_addrlen); - void *addr = alloca(addrlen); - long ret; + socklen_t addrlen; + void *addr; + long ret,ret2; + + if( (ret=page_check_range(target_addrlen,sizeof(socklen_t),PAGE_READ)) != 0 ) return -ret; + addrlen = tget32(target_addrlen); + addr = alloca(addrlen); ret = get_errno(getpeername(fd, addr, &addrlen)); if (!is_error(ret)) { - host_to_target_sockaddr(target_addr, addr, addrlen); + if( (ret2=host_to_target_sockaddr(target_addr, addr, addrlen, PAGE_WRITE)) != 0 ) return -ret2; + if( (ret2=page_check_range(target_addrlen,sizeof(socklen_t),PAGE_WRITE)) != 0 ) return -ret2; tput32(target_addrlen, addrlen); } return ret; @@ -988,13 +1022,17 @@ static long do_getsockname(int fd, target_ulong target_addr, target_ulong target_addrlen) { - socklen_t addrlen = tget32(target_addrlen); - void *addr = alloca(addrlen); - long ret; + socklen_t addrlen; + void *addr; + long ret,ret2; + + if( (ret=page_check_range(target_addrlen,sizeof(socklen_t),PAGE_WRITE)) != 0 ) return -ret; + addrlen = tget32(target_addrlen); + addr = alloca(addrlen); ret = get_errno(getsockname(fd, addr, &addrlen)); if (!is_error(ret)) { - host_to_target_sockaddr(target_addr, addr, addrlen); + if( (ret2=host_to_target_sockaddr(target_addr, addr, addrlen, PAGE_WRITE)) ) return -ret2; tput32(target_addrlen, addrlen); } return ret; @@ -1021,10 +1059,10 @@ void *host_msg; long ret; - host_msg = lock_user(msg, len, 1); + if( (ret=lock_and_check_user_struct(&host_msg,msg,len,1,PAGE_READ)) != 0 ) return -ret; if (target_addr) { addr = alloca(addrlen); - target_to_host_sockaddr(addr, target_addr, addrlen); + if( (ret=target_to_host_sockaddr(addr, target_addr, addrlen, PAGE_READ)) != 0 ) return -ret; ret = get_errno(sendto(fd, host_msg, len, flags, addr, addrlen)); } else { ret = get_errno(send(fd, host_msg, len, flags)); @@ -1039,9 +1077,9 @@ socklen_t addrlen; void *addr; void *host_msg; - long ret; + long ret,ret2; - host_msg = lock_user(msg, len, 0); + if( (ret2=lock_and_check_user_struct(&host_msg,msg,len,0,PAGE_WRITE)) != 0 ) return -ret2; if (target_addr) { addrlen = tget32(target_addrlen); addr = alloca(addrlen); @@ -1052,7 +1090,7 @@ } if (!is_error(ret)) { if (target_addr) { - host_to_target_sockaddr(target_addr, addr, addrlen); + if( (ret2=host_to_target_sockaddr(target_addr, addr, addrlen, PAGE_WRITE)) != 0 ) return -ret2; tput32(target_addrlen, addrlen); } unlock_user(host_msg, msg, len); @@ -1259,13 +1297,15 @@ target_ulong __unused4; }; -static inline void target_to_host_ipc_perm(struct ipc_perm *host_ip, - target_ulong target_addr) +static inline long target_to_host_ipc_perm(struct ipc_perm *host_ip, + target_ulong target_addr, + int pg_access) { + long ret = 0; struct target_ipc_perm *target_ip; struct target_semid_ds *target_sd; - lock_user_struct(target_sd, target_addr, 1); + if( (ret=lock_and_check_user_struct(&target_sd,target_addr,sizeof(*target_sd),1,pg_access)) ) return -ret; target_ip=&(target_sd->sem_perm); host_ip->__key = tswapl(target_ip->__key); host_ip->uid = tswapl(target_ip->uid); @@ -1274,15 +1314,19 @@ host_ip->cgid = tswapl(target_ip->cgid); host_ip->mode = tswapl(target_ip->mode); unlock_user_struct(target_sd, target_addr, 0); + + return ret; } -static inline void host_to_target_ipc_perm(target_ulong target_addr, - struct ipc_perm *host_ip) +static inline long host_to_target_ipc_perm(target_ulong target_addr, + struct ipc_perm *host_ip, + int pg_access) { + long ret = 0; struct target_ipc_perm *target_ip; struct target_semid_ds *target_sd; - lock_user_struct(target_sd, target_addr, 0); + if( (ret=lock_and_check_user_struct(&target_sd,target_addr,sizeof(*target_sd),0,pg_access)) ) return -ret; target_ip = &(target_sd->sem_perm); target_ip->__key = tswapl(host_ip->__key); target_ip->uid = tswapl(host_ip->uid); @@ -1291,32 +1335,42 @@ target_ip->cgid = tswapl(host_ip->cgid); target_ip->mode = tswapl(host_ip->mode); unlock_user_struct(target_sd, target_addr, 1); + + return ret; } -static inline void target_to_host_semid_ds(struct semid_ds *host_sd, - target_ulong target_addr) +static inline long target_to_host_semid_ds(struct semid_ds *host_sd, + target_ulong target_addr, + int pg_access) { + long ret = 0; struct target_semid_ds *target_sd; - lock_user_struct(target_sd, target_addr, 1); - target_to_host_ipc_perm(&(host_sd->sem_perm),target_addr); + if( (ret=lock_and_check_user_struct(&target_sd,target_addr,sizeof(*target_sd),1,pg_access)) != 0 ) return -ret; + if( (ret=target_to_host_ipc_perm(&(host_sd->sem_perm),target_addr,pg_access)) != 0 ) return ret; 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); unlock_user_struct(target_sd, target_addr, 0); + + return ret; } -static inline void host_to_target_semid_ds(target_ulong target_addr, - struct semid_ds *host_sd) +static inline long host_to_target_semid_ds(target_ulong target_addr, + struct semid_ds *host_sd, + int pg_access) { + long ret = 0; struct target_semid_ds *target_sd; - lock_user_struct(target_sd, target_addr, 0); - host_to_target_ipc_perm(target_addr,&(host_sd->sem_perm)); + if( (ret=lock_and_check_user_struct(&target_sd,target_addr,sizeof(*target_sd),0,pg_access)) != 0 ) return -ret; + if( (ret=host_to_target_ipc_perm(target_addr,&(host_sd->sem_perm),pg_access)) != 0 ) return ret; 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); unlock_user_struct(target_sd, target_addr, 1); + + return ret; } union semun { @@ -1331,67 +1385,75 @@ unsigned short int *array; }; -static inline void target_to_host_semun(unsigned long cmd, +static inline long target_to_host_semun(unsigned long cmd, union semun *host_su, target_ulong target_addr, - struct semid_ds *ds) + struct semid_ds *ds, + int pg_access) { + long ret = 0; union target_semun *target_su; switch( cmd ) { case IPC_STAT: case IPC_SET: - lock_user_struct(target_su, target_addr, 1); - target_to_host_semid_ds(ds,target_su->buf); + if( (ret=lock_and_check_user_struct(&target_su,target_addr,sizeof(*target_su),1,pg_access)) != 0 ) return -ret; + if( (ret=target_to_host_semid_ds(ds,target_su->buf, pg_access)) != 0 ) return -ret; host_su->buf = ds; unlock_user_struct(target_su, target_addr, 0); break; case GETVAL: case SETVAL: - lock_user_struct(target_su, target_addr, 1); + if( (ret=lock_and_check_user_struct(&target_su,target_addr,sizeof(*target_su),1,pg_access)) != 0 ) return -ret; host_su->val = tswapl(target_su->val); unlock_user_struct(target_su, target_addr, 0); break; case GETALL: case SETALL: - lock_user_struct(target_su, target_addr, 1); + if( (ret=lock_and_check_user_struct(&target_su,target_addr,sizeof(*target_su),1,pg_access)) != 0 ) return -ret; *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); } + + return ret; } -static inline void host_to_target_semun(unsigned long cmd, +static inline long host_to_target_semun(unsigned long cmd, target_ulong target_addr, union semun *host_su, - struct semid_ds *ds) + struct semid_ds *ds, + int pg_access) { + long ret = 0; union target_semun *target_su; switch( cmd ) { case IPC_STAT: case IPC_SET: - lock_user_struct(target_su, target_addr, 0); - host_to_target_semid_ds(target_su->buf,ds); + if( (ret=lock_and_check_user_struct(&target_su,target_addr,sizeof(*target_su),0,pg_access)) != 0 ) return -ret; + if( (ret=host_to_target_semid_ds(target_su->buf,ds, pg_access)) != 0 ) return -ret; unlock_user_struct(target_su, target_addr, 1); break; case GETVAL: case SETVAL: - lock_user_struct(target_su, target_addr, 0); + if( (ret=lock_and_check_user_struct(&target_su,target_addr,sizeof(*target_su),0,pg_access)) != 0 ) return -ret; target_su->val = tswapl(host_su->val); unlock_user_struct(target_su, target_addr, 1); break; case GETALL: case SETALL: - lock_user_struct(target_su, target_addr, 0); + if( (ret=lock_and_check_user_struct(&target_su,target_addr,sizeof(*target_su),0,pg_access)) != 0 ) return -ret; *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); } + + return ret; } static inline long do_semctl(long first, long second, long third, long ptr) @@ -1403,34 +1465,34 @@ switch( cmd ) { case GETVAL: - target_to_host_semun(cmd,&arg,ptr,&dsarg); + if( (ret=target_to_host_semun(cmd,&arg,ptr,&dsarg,PAGE_WRITE)) != 0 ) return -ret; ret = get_errno(semctl(first, second, cmd, arg)); - host_to_target_semun(cmd,ptr,&arg,&dsarg); + host_to_target_semun(cmd,ptr,&arg,&dsarg,PAGE_WRITE); break; case SETVAL: - target_to_host_semun(cmd,&arg,ptr,&dsarg); + if( (ret=target_to_host_semun(cmd,&arg,ptr,&dsarg,PAGE_READ)) != 0 ) return -ret; ret = get_errno(semctl(first, second, cmd, arg)); - host_to_target_semun(cmd,ptr,&arg,&dsarg); + host_to_target_semun(cmd,ptr,&arg,&dsarg,PAGE_READ); break; case GETALL: - target_to_host_semun(cmd,&arg,ptr,&dsarg); + if( (ret=target_to_host_semun(cmd,&arg,ptr,&dsarg,PAGE_WRITE)) != 0 ) return -ret; ret = get_errno(semctl(first, second, cmd, arg)); - host_to_target_semun(cmd,ptr,&arg,&dsarg); + host_to_target_semun(cmd,ptr,&arg,&dsarg,PAGE_WRITE); break; case SETALL: - target_to_host_semun(cmd,&arg,ptr,&dsarg); + if( (ret=target_to_host_semun(cmd,&arg,ptr,&dsarg,PAGE_READ)) != 0 ) return -ret; ret = get_errno(semctl(first, second, cmd, arg)); - host_to_target_semun(cmd,ptr,&arg,&dsarg); + host_to_target_semun(cmd,ptr,&arg,&dsarg,PAGE_READ); break; case IPC_STAT: - target_to_host_semun(cmd,&arg,ptr,&dsarg); + if( (ret=target_to_host_semun(cmd,&arg,ptr,&dsarg,PAGE_WRITE)) != 0 ) return -ret; ret = get_errno(semctl(first, second, cmd, arg)); - host_to_target_semun(cmd,ptr,&arg,&dsarg); + host_to_target_semun(cmd,ptr,&arg,&dsarg,PAGE_READ); break; case IPC_SET: - target_to_host_semun(cmd,&arg,ptr,&dsarg); + if( (ret=target_to_host_semun(cmd,&arg,ptr,&dsarg,PAGE_READ)) != 0 ) return -ret; ret = get_errno(semctl(first, second, cmd, arg)); - host_to_target_semun(cmd,ptr,&arg,&dsarg); + host_to_target_semun(cmd,ptr,&arg,&dsarg,PAGE_READ); break; default: ret = get_errno(semctl(first, second, cmd, arg)); @@ -1457,13 +1519,15 @@ target_ulong __unused5; }; -static inline void target_to_host_msqid_ds(struct msqid_ds *host_md, - target_ulong target_addr) +static inline long target_to_host_msqid_ds(struct msqid_ds *host_md, + target_ulong target_addr, + int pg_access) { + long ret = 0; struct target_msqid_ds *target_md; - lock_user_struct(target_md, target_addr, 1); - target_to_host_ipc_perm(&(host_md->msg_perm),target_addr); + if( (ret=lock_and_check_user_struct(&target_md,target_addr,sizeof(*target_md),1,pg_access)) != 0 ) return -ret; + if( (ret=target_to_host_ipc_perm(&(host_md->msg_perm),target_addr,pg_access)) != 0 ) return ret; host_md->msg_stime = tswapl(target_md->msg_stime); host_md->msg_rtime = tswapl(target_md->msg_rtime); host_md->msg_ctime = tswapl(target_md->msg_ctime); @@ -1473,15 +1537,18 @@ host_md->msg_lspid = tswapl(target_md->msg_lspid); host_md->msg_lrpid = tswapl(target_md->msg_lrpid); unlock_user_struct(target_md, target_addr, 0); + return ret; } -static inline void host_to_target_msqid_ds(target_ulong target_addr, - struct msqid_ds *host_md) +static inline long host_to_target_msqid_ds(target_ulong target_addr, + struct msqid_ds *host_md, + int pg_access) { + long ret = 0; struct target_msqid_ds *target_md; - lock_user_struct(target_md, target_addr, 0); - host_to_target_ipc_perm(target_addr,&(host_md->msg_perm)); + if( (ret=lock_and_check_user_struct(&target_md,target_addr,sizeof(*target_md),0,pg_access)) != 0 ) return -ret; + if( (ret=host_to_target_ipc_perm(target_addr,&(host_md->msg_perm),pg_access)) != 0 ) return ret; target_md->msg_stime = tswapl(host_md->msg_stime); target_md->msg_rtime = tswapl(host_md->msg_rtime); target_md->msg_ctime = tswapl(host_md->msg_ctime); @@ -1491,6 +1558,8 @@ target_md->msg_lspid = tswapl(host_md->msg_lspid); target_md->msg_lrpid = tswapl(host_md->msg_lrpid); unlock_user_struct(target_md, target_addr, 1); + + return ret; } static inline long do_msgctl(long first, long second, long ptr) @@ -1500,10 +1569,15 @@ long ret = 0; switch( cmd ) { case IPC_STAT: + if( (ret=target_to_host_msqid_ds(&dsarg,ptr,PAGE_WRITE)) != 0 ) return -ret; + ret = get_errno(msgctl(first, cmd, &dsarg)); + host_to_target_msqid_ds(ptr,&dsarg,PAGE_WRITE); + break; case IPC_SET: - target_to_host_msqid_ds(&dsarg,ptr); + if( (ret=target_to_host_msqid_ds(&dsarg,ptr,PAGE_READ)) != 0 ) return -ret; ret = get_errno(msgctl(first, cmd, &dsarg)); - host_to_target_msqid_ds(ptr,&dsarg); + host_to_target_msqid_ds(ptr,&dsarg,PAGE_WRITE); + break; default: ret = get_errno(msgctl(first, cmd, &dsarg)); } @@ -1521,7 +1595,7 @@ struct msgbuf *host_mb; long ret = 0; - lock_user_struct(target_mb,msgp,0); + if( (ret=lock_and_check_user_struct(&target_mb,msgp,sizeof(long)+msgsz,1,PAGE_READ)) != 0 ) return -ret; host_mb = malloc(msgsz+sizeof(long)); host_mb->mtype = tswapl(target_mb->mtype); memcpy(host_mb->mtext,target_mb->mtext,msgsz); @@ -1538,7 +1612,7 @@ struct msgbuf *host_mb; long ret = 0; - lock_user_struct(target_mb, msgp, 0); + if( (ret=lock_and_check_user_struct(&target_mb,msgp,sizeof(long)+msgsz,0,PAGE_WRITE)) != 0 ) return -ret; host_mb = malloc(msgsz+sizeof(long)); ret = get_errno(msgrcv(msqid, host_mb, msgsz, 1, msgflg)); if (ret > 0) @@ -2018,6 +2092,7 @@ static int write_ldt(CPUX86State *env, target_ulong ptr, unsigned long bytecount, int oldmode) { + long ret = 0; struct target_modify_ldt_ldt_s ldt_info; struct target_modify_ldt_ldt_s *target_ldt_info; int seg_32bit, contents, read_exec_only, limit_in_pages; @@ -2026,7 +2101,7 @@ if (bytecount != sizeof(ldt_info)) return -EINVAL; - lock_user_struct(target_ldt_info, ptr, 1); + if( (ret=lock_and_check_user_struct(&target_ldt_info,ptr,sizeof(struct target_modify_ldt_ldt_s),1,PAGE_READ)) != 0 ) return -ret; ldt_info.entry_number = tswap32(target_ldt_info->entry_number); ldt_info.base_addr = tswapl(target_ldt_info->base_addr); ldt_info.limit = tswap32(target_ldt_info->limit); @@ -2225,7 +2300,7 @@ switch(cmd) { case TARGET_F_GETLK: - lock_user_struct(target_fl, arg, 1); + if( (ret=lock_and_check_user_struct(&target_fl,arg,sizeof(struct target_flock),1,PAGE_READ)) != 0 ) return -ret; fl.l_type = tswap16(target_fl->l_type); fl.l_whence = tswap16(target_fl->l_whence); fl.l_start = tswapl(target_fl->l_start); @@ -2234,6 +2309,7 @@ unlock_user_struct(target_fl, arg, 0); ret = fcntl(fd, cmd, &fl); if (ret == 0) { + if( (ret=lock_and_check_user_struct(&target_fl,arg,sizeof(struct target_flock),0,PAGE_WRITE)) != 0 ) return -ret; lock_user_struct(target_fl, arg, 0); target_fl->l_type = tswap16(fl.l_type); target_fl->l_whence = tswap16(fl.l_whence); @@ -2246,7 +2322,7 @@ case TARGET_F_SETLK: case TARGET_F_SETLKW: - lock_user_struct(target_fl, arg, 1); + if( (ret=lock_and_check_user_struct(&target_fl,arg,sizeof(struct target_flock),1,PAGE_READ)) != 0 ) return -ret; fl.l_type = tswap16(target_fl->l_type); fl.l_whence = tswap16(target_fl->l_whence); fl.l_start = tswapl(target_fl->l_start); @@ -2257,7 +2333,7 @@ break; case TARGET_F_GETLK64: - lock_user_struct(target_fl64, arg, 1); + if( (ret=lock_and_check_user_struct(&target_fl64,arg,sizeof(struct target_flock64),1,PAGE_READ)) != 0 ) return -ret; fl64.l_type = tswap16(target_fl64->l_type) >> 1; fl64.l_whence = tswap16(target_fl64->l_whence); fl64.l_start = tswapl(target_fl64->l_start); @@ -2266,7 +2342,7 @@ unlock_user_struct(target_fl64, arg, 0); ret = fcntl(fd, cmd >> 1, &fl64); if (ret == 0) { - lock_user_struct(target_fl64, arg, 0); + if( (ret=lock_and_check_user_struct(&target_fl64,arg,sizeof(struct target_flock64),0,PAGE_WRITE)) != 0 ) return -ret; target_fl64->l_type = tswap16(fl64.l_type) >> 1; target_fl64->l_whence = tswap16(fl64.l_whence); target_fl64->l_start = tswapl(fl64.l_start); @@ -2277,7 +2353,7 @@ break; case TARGET_F_SETLK64: case TARGET_F_SETLKW64: - lock_user_struct(target_fl64, arg, 1); + if( (ret=lock_and_check_user_struct(&target_fl64,arg,sizeof(struct target_flock64),1,PAGE_READ)) != 0 ) return -ret; fl64.l_type = tswap16(target_fl64->l_type) >> 1; fl64.l_whence = tswap16(target_fl64->l_whence); fl64.l_start = tswapl(target_fl64->l_start); @@ -2419,23 +2495,26 @@ } #endif -static inline void target_to_host_timespec(struct timespec *host_ts, +static inline long target_to_host_timespec(struct timespec *host_ts, target_ulong target_addr) { + long ret = 0; struct target_timespec *target_ts; - lock_user_struct(target_ts, target_addr, 1); + if( (ret=lock_and_check_user_struct(&target_ts,target_addr,sizeof(struct target_timespec),1,PAGE_READ)) != 0 ) return -ret; host_ts->tv_sec = tswapl(target_ts->tv_sec); host_ts->tv_nsec = tswapl(target_ts->tv_nsec); unlock_user_struct(target_ts, target_addr, 0); } -static inline void host_to_target_timespec(target_ulong target_addr, +static inline long host_to_target_timespec(target_ulong target_addr, struct timespec *host_ts) { + long ret = 0; struct target_timespec *target_ts; lock_user_struct(target_ts, target_addr, 0); + if( (ret=lock_and_check_user_struct(&target_ts,target_addr,sizeof(struct target_timespec),0,PAGE_WRITE)) != 0 ) return -ret; target_ts->tv_sec = tswapl(host_ts->tv_sec); target_ts->tv_nsec = tswapl(host_ts->tv_nsec); unlock_user_struct(target_ts, target_addr, 1); @@ -2444,7 +2523,7 @@ long do_syscall(void *cpu_env, int num, long arg1, long arg2, long arg3, long arg4, long arg5, long arg6) { - long ret; + long ret = 0; struct stat st; struct statfs stfs; void *p; @@ -2671,7 +2750,7 @@ struct utimbuf tbuf, *host_tbuf; struct target_utimbuf *target_tbuf; if (arg2) { - lock_user_struct(target_tbuf, arg2, 1); + if( (ret=lock_and_check_user_struct(&target_tbuf,arg2,sizeof(struct target_utimbuf),1,PAGE_READ)) != 0 ) return -ret; tbuf.actime = tswapl(target_tbuf->actime); tbuf.modtime = tswapl(target_tbuf->modtime); unlock_user_struct(target_tbuf, arg2, 0); @@ -2689,9 +2768,9 @@ { struct timeval *tvp, tv[2]; if (arg2) { - target_to_host_timeval(&tv[0], arg2); - target_to_host_timeval(&tv[1], - arg2 + sizeof (struct target_timeval)); + if( (ret=target_to_host_timeval(&tv[0], arg2)) != 0 ) return ret; + if( (ret=target_to_host_timeval(&tv[1], + arg2 + sizeof (struct target_timeval))) != 0 ) return ret; tvp = tv; } else { tvp = NULL; @@ -2860,7 +2939,7 @@ struct target_old_sigaction *old_act; struct target_sigaction act, oact, *pact; if (arg2) { - lock_user_struct(old_act, arg2, 1); + if( (ret=lock_and_check_user_struct(&old_act,arg2,sizeof(*old_act),1,PAGE_READ)) != 0 ) return -ret; act._sa_handler = old_act->_sa_handler; target_siginitset(&act.sa_mask, old_act->sa_mask); act.sa_flags = old_act->sa_flags; @@ -2872,7 +2951,7 @@ } ret = get_errno(do_sigaction(arg1, pact, &oact)); if (!is_error(ret) && arg3) { - lock_user_struct(old_act, arg3, 0); + if( (ret=lock_and_check_user_struct(&old_act,arg3,sizeof(*old_act),0,PAGE_WRITE)) != 0 ) return -ret; old_act->_sa_handler = oact._sa_handler; old_act->sa_mask = oact.sa_mask.sig[0]; old_act->sa_flags = oact.sa_flags; @@ -2883,7 +2962,7 @@ struct target_sigaction act, oact, *pact, *old_act; if (arg2) { - lock_user_struct(old_act, arg2, 1); + if( (ret=lock_and_check_user_struct(&old_act,arg2,sizeof(*old_act),1,PAGE_READ)) != 0 ) return -ret; act._sa_handler = old_act->_sa_handler; target_siginitset(&act.sa_mask, old_act->sa_mask.sig[0]); act.sa_flags = old_act->sa_flags; @@ -2896,7 +2975,7 @@ ret = get_errno(do_sigaction(arg1, pact, &oact)); if (!is_error(ret) && arg3) { - lock_user_struct(old_act, arg3, 0); + if( (ret=lock_and_check_user_struct(&old_act,arg3,sizeof(*old_act),0,PAGE_WRITE)) != 0 ) return -ret; old_act->_sa_handler = oact._sa_handler; old_act->sa_flags = oact.sa_flags; old_act->sa_mask.sig[0] = oact.sa_mask.sig[0]; @@ -2914,12 +2993,14 @@ struct target_sigaction *act; struct target_sigaction *oact; - if (arg2) - lock_user_struct(act, arg2, 1); + if (arg2) { + if( (ret=lock_and_check_user_struct(&act,arg2,sizeof(*act),1,PAGE_READ)) != 0 ) return -ret; + } else act = NULL; - if (arg3) - lock_user_struct(oact, arg3, 0); + if (arg3) { + if( (ret=lock_and_check_user_struct(&oact,arg2,sizeof(*oact),0,PAGE_WRITE)) != 0 ) return -ret; + } else oact = NULL; ret = get_errno(do_sigaction(arg1, act, oact)); @@ -3046,7 +3127,7 @@ sigset_t set; ret = get_errno(sigpending(&set)); if (!is_error(ret)) { - p = lock_user(arg1, sizeof(target_sigset_t), 0); + if( (ret=lock_and_check_user_struct(&p,arg1,sizeof(target_sigset_t),0,PAGE_WRITE)) != 0 ) return -ret; host_to_target_sigset(p, &set); unlock_user(p, arg1, sizeof(target_sigset_t)); } @@ -3056,7 +3137,7 @@ case TARGET_NR_sigsuspend: { sigset_t set; - p = lock_user(arg1, sizeof(target_sigset_t), 1); + if( (ret=lock_and_check_user_struct(&p,arg1,sizeof(target_sigset_t),1,PAGE_READ)) != 0 ) return -ret; target_to_host_old_sigset(&set, p); unlock_user(p, arg1, 0); ret = get_errno(sigsuspend(&set)); @@ -3066,7 +3147,7 @@ case TARGET_NR_rt_sigsuspend: { sigset_t set; - p = lock_user(arg1, sizeof(target_sigset_t), 1); + if( (ret=lock_and_check_user_struct(&p,arg1,sizeof(target_sigset_t),1,PAGE_READ)) != 0 ) return -ret; target_to_host_sigset(&set, p); unlock_user(p, arg1, 0); ret = get_errno(sigsuspend(&set)); @@ -3078,18 +3159,18 @@ struct timespec uts, *puts; siginfo_t uinfo; - p = lock_user(arg1, sizeof(target_sigset_t), 1); + if( (ret=lock_and_check_user_struct(&p,arg1,sizeof(target_sigset_t),1,PAGE_READ)) != 0 ) return -ret; target_to_host_sigset(&set, p); unlock_user(p, arg1, 0); if (arg3) { puts = &uts; - target_to_host_timespec(puts, arg3); + if( (ret=target_to_host_timespec(puts, arg3)) != 0 ) return ret; } else { puts = NULL; } ret = get_errno(sigtimedwait(&set, &uinfo, puts)); if (!is_error(ret) && arg2) { - p = lock_user(arg2, sizeof(target_sigset_t), 0); + if( (ret=lock_and_check_user_struct(&p,arg2,sizeof(target_sigset_t),0,PAGE_WRITE)) != 0 ) return -ret; host_to_target_siginfo(p, &uinfo); unlock_user(p, arg2, sizeof(target_sigset_t)); } @@ -3098,7 +3179,7 @@ case TARGET_NR_rt_sigqueueinfo: { siginfo_t uinfo; - p = lock_user(arg3, sizeof(target_sigset_t), 1); + if( (ret=lock_and_check_user_struct(&p,arg3,sizeof(target_sigset_t),1,PAGE_READ)) != 0 ) return -ret; target_to_host_siginfo(&uinfo, p); unlock_user(p, arg1, 0); ret = get_errno(sys_rt_sigqueueinfo(arg1, arg2, &uinfo)); @@ -3153,7 +3234,7 @@ struct rusage rusage; ret = get_errno(getrusage(arg1, &rusage)); if (!is_error(ret)) { - host_to_target_rusage(arg2, &rusage); + if( (ret=host_to_target_rusage(arg2, &rusage)) != 0 ) return ret; } } break; @@ -3162,14 +3243,14 @@ struct timeval tv; ret = get_errno(gettimeofday(&tv, NULL)); if (!is_error(ret)) { - host_to_target_timeval(arg1, &tv); + if( (ret=host_to_target_timeval(arg1, &tv)) != 0 ) return ret; } } break; case TARGET_NR_settimeofday: { struct timeval tv; - target_to_host_timeval(&tv, arg1); + if( (ret=target_to_host_timeval(&tv, arg1)) != 0 ) return ret; ret = get_errno(settimeofday(&tv, NULL)); } break; @@ -3488,19 +3569,19 @@ if (arg2) { pvalue = &value; - target_to_host_timeval(&pvalue->it_interval, - arg2); - target_to_host_timeval(&pvalue->it_value, - arg2 + sizeof(struct target_timeval)); + if( (ret=target_to_host_timeval(&pvalue->it_interval, + arg2)) != 0 ) return ret; + if( (ret=target_to_host_timeval(&pvalue->it_value, + arg2 + sizeof(struct target_timeval))) != 0 ) return ret; } else { pvalue = NULL; } ret = get_errno(setitimer(arg1, pvalue, &ovalue)); if (!is_error(ret) && arg3) { - host_to_target_timeval(arg3, - &ovalue.it_interval); - host_to_target_timeval(arg3 + sizeof(struct target_timeval), - &ovalue.it_value); + if( (ret=host_to_target_timeval(arg3, + &ovalue.it_interval)) != 0 ) return ret; + if( (ret=host_to_target_timeval(arg3 + sizeof(struct target_timeval), + &ovalue.it_value)) != 0 ) return ret; } } break; @@ -3510,10 +3591,10 @@ ret = get_errno(getitimer(arg1, &value)); if (!is_error(ret) && arg2) { - host_to_target_timeval(arg2, - &value.it_interval); - host_to_target_timeval(arg2 + sizeof(struct target_timeval), - &value.it_value); + if( (ret=host_to_target_timeval(arg2, + &value.it_interval)) != 0 ) return ret; + if( (ret=host_to_target_timeval(arg2 + sizeof(struct target_timeval), + &value.it_value)) != 0 ) return ret; } } break; @@ -3534,7 +3615,7 @@ if (!is_error(ret)) { struct target_stat *target_st; - lock_user_struct(target_st, arg2, 0); + if( (ret=lock_and_check_user_struct(&target_st,arg2,sizeof(*target_st),9,PAGE_WRITE)) != 0 ) return -ret; #if defined(TARGET_MIPS) target_st->st_dev = tswap32(st.st_dev); #else @@ -3965,17 +4046,17 @@ struct timespec ts; ret = get_errno(sched_rr_get_interval(arg1, &ts)); if (!is_error(ret)) { - host_to_target_timespec(arg2, &ts); + if( (ret=host_to_target_timespec(arg2, &ts)) != 0 ) return ret; } } break; case TARGET_NR_nanosleep: { struct timespec req, rem; - target_to_host_timespec(&req, arg1); + if( (ret=target_to_host_timespec(&req, arg1)) != 0 ) return ret; ret = get_errno(nanosleep(&req, &rem)); if (is_error(ret) && arg2) { - host_to_target_timespec(arg2, &rem); + if( (ret=host_to_target_timespec(arg2, &rem)) != 0 ) return ret; } } break; @@ -4072,6 +4153,7 @@ #ifdef TARGET_NR_stat64 case TARGET_NR_stat64: p = lock_user_string(arg1); + if( ret=page_check_range(p,1,PAGE_READ) ) return -ret; ret = get_errno(stat(path(p), &st)); unlock_user(p, arg1, 0); goto do_stat64; @@ -4079,6 +4161,7 @@ #ifdef TARGET_NR_lstat64 case TARGET_NR_lstat64: p = lock_user_string(arg1); + if( ret=page_check_range(p,1,PAGE_READ) ) return -ret; ret = get_errno(lstat(path(p), &st)); unlock_user(p, arg1, 0); goto do_stat64; @@ -4092,7 +4175,7 @@ #ifdef TARGET_ARM if (((CPUARMState *)cpu_env)->eabi) { struct target_eabi_stat64 *target_st; - lock_user_struct(target_st, arg2, 1); + if( (ret=lock_and_check_user_struct(&target_st,arg2,sizeof(*target_st),1,PAGE_WRITE)) != 0 ) return -ret; memset(target_st, 0, sizeof(struct target_eabi_stat64)); /* put_user is probably wrong. */ put_user(st.st_dev, &target_st->st_dev); @@ -4117,7 +4200,7 @@ #endif { struct target_stat64 *target_st; - lock_user_struct(target_st, arg2, 1); + if( (ret=lock_and_check_user_struct(&target_st,arg2,sizeof(*target_st),1,PAGE_WRITE)) != 0 ) return -ret; memset(target_st, 0, sizeof(struct target_stat64)); /* ??? put_user is probably wrong. */ put_user(st.st_dev, &target_st->st_dev); @@ -4583,7 +4666,7 @@ struct timespec ts; ret = get_errno(clock_gettime(arg1, &ts)); if (!is_error(ret)) { - host_to_target_timespec(arg2, &ts); + if( (ret=host_to_target_timespec(arg2, &ts)) != 0 ) return ret; } break; } @@ -4594,7 +4677,7 @@ struct timespec ts; ret = get_errno(clock_getres(arg1, &ts)); if (!is_error(ret)) { - host_to_target_timespec(arg2, &ts); + if( (ret=host_to_target_timespec(arg2, &ts)) != 0 ) return ret; } break; } Index: qemu/linux-user/qemu.h =================================================================== --- qemu.orig/linux-user/qemu.h 2007-07-06 09:42:29.000000000 -0400 +++ qemu/linux-user/qemu.h 2007-07-06 09:49:37.000000000 -0400 @@ -295,6 +295,15 @@ #define unlock_user_struct(host_ptr, guest_addr, copy) \ unlock_user(host_ptr, guest_addr, (copy) ? sizeof(*host_ptr) : 0) +static inline long lock_and_check_user_struct(void *host_addr, target_ulong guest_addr, long len, int copy, int pg_access) +{ + long *haddr, ret = 0; + haddr = (long *)host_addr; + if( (ret=page_check_range(guest_addr,len,pg_access)) != 0 ) return ret; + *haddr = (long)lock_user(guest_addr, len, copy); + return 0; +} + #define tget8(addr) ldub(addr) #define tput8(addr, val) stb(addr, val) #define tget16(addr) lduw(addr) ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH] linux-user EFAULT implementation 2007-07-06 15:15 [Qemu-devel] [PATCH] linux-user EFAULT implementation Stuart Anderson @ 2007-07-06 16:08 ` Fabrice Bellard 2007-07-06 16:55 ` Stuart Anderson 0 siblings, 1 reply; 13+ messages in thread From: Fabrice Bellard @ 2007-07-06 16:08 UTC (permalink / raw) To: qemu-devel Please update page_check_range() (and other related functions) to return -EFAULT instead of EFAULT in case of error. Moreover, I believe using similar functions as Linux for memory access (copyfromuser, copytouser, get_user, put_user) would be cleaner. Regards, Fabrice. Stuart Anderson wrote: > > Here is a refresh of the linux-user EFAULT patch vs todays cvs. This > patch has now received a lot more runtime, and a couple of bug have been > fixed that affected some of the socket APIs. > > The purpose of this patch is to detect bad pointers and return EFAULT > instead of just crashing when the bad pointers are used by qemu. > > > Stuart ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH] linux-user EFAULT implementation 2007-07-06 16:08 ` Fabrice Bellard @ 2007-07-06 16:55 ` Stuart Anderson 2007-07-06 18:45 ` Stuart Anderson 0 siblings, 1 reply; 13+ messages in thread From: Stuart Anderson @ 2007-07-06 16:55 UTC (permalink / raw) To: qemu-devel On Fri, 6 Jul 2007, Fabrice Bellard wrote: > Please update page_check_range() (and other related functions) to return > -EFAULT instead of EFAULT in case of error. Will do. > Moreover, I believe using similar functions as Linux for memory access > (copyfromuser, copytouser, get_user, put_user) would be cleaner. This makes sense. That code that I sent was based on the idea that I was just extending the model that was already there, instead of giving it a complete overhaul. I'll look into how much work the overhaul would be. Stuart Stuart R. Anderson anderson@netsweng.com Network & Software Engineering http://www.netsweng.com/ 1024D/37A79149: 0791 D3B8 9A4C 2CDC A31F BD03 0A62 E534 37A7 9149 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH] linux-user EFAULT implementation 2007-07-06 16:55 ` Stuart Anderson @ 2007-07-06 18:45 ` Stuart Anderson 2007-07-09 11:41 ` Stuart Anderson 0 siblings, 1 reply; 13+ messages in thread From: Stuart Anderson @ 2007-07-06 18:45 UTC (permalink / raw) To: qemu-devel On Fri, 6 Jul 2007, Stuart Anderson wrote: >> Moreover, I believe using similar functions as Linux for memory access >> (copyfromuser, copytouser, get_user, put_user) would be cleaner. > > This makes sense. That code that I sent was based on the idea that I was > just extending the model that was already there, instead of giving it a > complete overhaul. I'll look into how much work the overhaul would be. After some digging, the one "design-level" issue I have found is that the current lock_user_struct() and the new lock_and_check_user_struct(), interfaces are based on the assumption that we need to map addresses between guest and host, so they provide a place to call g2h() and return the new address. This is different from copy_{to|from}_user(), which assumes all addresses are valid as is, but that the system just needs to ensure they are correctly mapped in for the data copy. Contrary to this, cpu-all.h has the following comment & code: #define GUEST_BASE 0 /* All direct uses of g2h and h2g need to go away for usermode softmmu. */ #define g2h(x) ((void *)((unsigned long)(x) + GUEST_BASE)) #define h2g(x) ((target_ulong)(x - GUEST_BASE)) It appears that g2h() and h2g() are mostly no-ops, with the typecasting being the only possible useful part remaining. So, the question is: Can I simplify this code to assume that guest and host addresses coexist and use the copy_*_user() or just the access_ok() interfaces? This would be one step closer to eliminating the use of g2h() and h2g() in the usermode code as is suggested by the comment in cpu-all.h. One other comment to make, is that in most cases, a simple copy is not sufficient. Unlike the real Linux kernel, in many places, we are doing structure mapping instead of a simple buffer copy. Stuart Stuart R. Anderson anderson@netsweng.com Network & Software Engineering http://www.netsweng.com/ 1024D/37A79149: 0791 D3B8 9A4C 2CDC A31F BD03 0A62 E534 37A7 9149 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH] linux-user EFAULT implementation 2007-07-06 18:45 ` Stuart Anderson @ 2007-07-09 11:41 ` Stuart Anderson 2007-07-09 12:02 ` Stuart Anderson 2007-07-09 21:15 ` Fabrice Bellard 0 siblings, 2 replies; 13+ messages in thread From: Stuart Anderson @ 2007-07-09 11:41 UTC (permalink / raw) To: qemu-devel On Fri, 6 Jul 2007, Stuart Anderson wrote: > So, the question is: > > Can I simplify this code to assume that guest and > host addresses coexist and use the copy_*_user() or > just the access_ok() interfaces? Attached is a diff that shows what this will look like for the struct sockaddr handling case. In short, the two functions for mapping this structure get renamed target_to_host_sockaddr -> copy_from_user_sockaddr host_to_target_sockaddr -> copy_to_user_sockaddr and lose the pg_access parameter as read/write access is determined by wether we are copying to or from "user space". The macro access_ok() is used to check the validity of the memory being accessed. This change also completely drops the notion of locking and unlocking memory, which was the previous model being used. Fabrice, Is this change what you had in mind? Does it cause a problem for anyone else? Stuart Stuart R. Anderson anderson@netsweng.com Network & Software Engineering http://www.netsweng.com/ 1024D/37A79149: 0791 D3B8 9A4C 2CDC A31F BD03 0A62 E534 37A7 9149 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH] linux-user EFAULT implementation 2007-07-09 11:41 ` Stuart Anderson @ 2007-07-09 12:02 ` Stuart Anderson 2007-07-09 21:15 ` Fabrice Bellard 1 sibling, 0 replies; 13+ messages in thread From: Stuart Anderson @ 2007-07-09 12:02 UTC (permalink / raw) To: qemu-devel [-- Attachment #1: Type: TEXT/PLAIN, Size: 423 bytes --] On Mon, 9 Jul 2007, Stuart Anderson wrote: > Attached is a diff ... Here's the diff. Stuart Stuart R. Anderson anderson@netsweng.com Network & Software Engineering http://www.netsweng.com/ 1024D/37A79149: 0791 D3B8 9A4C 2CDC A31F BD03 0A62 E534 37A7 9149 [-- Attachment #2: sockaddr diff --] [-- Type: TEXT/x-diff, Size: 8026 bytes --] *** linux-user/syscall.c 2007-07-09 07:33:46.000000000 -0400 --- linux-user/syscall.c.stu 2007-07-09 07:01:12.000000000 -0400 *************** *** 522,550 **** return ret; } ! static inline long copy_from_user_sockaddr(struct sockaddr *addr, target_ulong target_addr, ! socklen_t len) { long ret = 0; ! struct target_sockaddr *target_saddr = (struct target_sockaddr *)target_addr; ! if( (ret=access_ok(VERIFY_READ,target_saddr,len)) != 0 ) return ret; memcpy(addr, target_saddr, len); addr->sa_family = tswap16(target_saddr->sa_family); return ret; } ! static inline long copy_to_user_sockaddr(target_ulong target_addr, struct sockaddr *addr, ! socklen_t len) { long ret = 0; ! struct target_sockaddr *target_saddr = (struct target_sockaddr *)target_addr; ! if( (ret=access_ok(VERIFY_WRITE,target_saddr,len)) != 0 ) return ret; memcpy(target_saddr, addr, len); target_saddr->sa_family = tswap16(addr->sa_family); return ret; } --- 522,554 ---- return ret; } ! static inline long target_to_host_sockaddr(struct sockaddr *addr, target_ulong target_addr, ! socklen_t len, ! int pg_access) { long ret = 0; ! struct target_sockaddr *target_saddr; ! if( (ret=lock_and_check_user_struct(&target_saddr,target_addr,len,1,pg_access)) != 0 ) return ret; memcpy(addr, target_saddr, len); addr->sa_family = tswap16(target_saddr->sa_family); + unlock_user(target_saddr, target_addr, 0); return ret; } ! static inline long host_to_target_sockaddr(target_ulong target_addr, struct sockaddr *addr, ! socklen_t len, ! int pg_access) { long ret = 0; ! struct target_sockaddr *target_saddr; ! if( (ret=lock_and_check_user_struct(&target_saddr,target_addr,len,1,pg_access)) != 0 ) return ret; memcpy(target_saddr, addr, len); target_saddr->sa_family = tswap16(addr->sa_family); + unlock_user(target_saddr, target_addr, len); return ret; } *************** *** 909,915 **** long ret = 0; void *addr = alloca(addrlen); ! if( (ret=copy_from_user_sockaddr(addr, target_addr, addrlen)) != 0 ) return ret; return get_errno(bind(sockfd, addr, addrlen)); } --- 913,919 ---- long ret = 0; void *addr = alloca(addrlen); ! if( (ret=target_to_host_sockaddr(addr, target_addr, addrlen,PAGE_READ)) != 0 ) return ret; return get_errno(bind(sockfd, addr, addrlen)); } *************** *** 919,925 **** long ret = 0; void *addr = alloca(addrlen); ! if( (ret=copy_from_user_sockaddr(addr, target_addr, addrlen)) != 0 ) return ret; return get_errno(connect(sockfd, addr, addrlen)); } --- 923,929 ---- long ret = 0; void *addr = alloca(addrlen); ! if( (ret=target_to_host_sockaddr(addr, target_addr, addrlen,PAGE_READ)) != 0 ) return ret; return get_errno(connect(sockfd, addr, addrlen)); } *************** *** 944,956 **** msg.msg_namelen = tswap32(msgp->msg_namelen); msg.msg_name = alloca(msg.msg_namelen); if( send ) { ! if( (ret=copy_from_user_sockaddr(msg.msg_name, tswapl(msgp->msg_name), ! msg.msg_namelen)) != 0 ) return ret; } else { ! /* FIXME ! if( (ret=copy_from_user_sockaddr(msg.msg_name, tswapl(msgp->msg_name), msg.msg_namelen,PAGE_WRITE)) != 0 ) return ret; - */ } } else { msg.msg_name = NULL; --- 948,958 ---- msg.msg_namelen = tswap32(msgp->msg_namelen); msg.msg_name = alloca(msg.msg_namelen); if( send ) { ! if( (ret=target_to_host_sockaddr(msg.msg_name, tswapl(msgp->msg_name), ! msg.msg_namelen,PAGE_READ)) != 0 ) return ret; } else { ! if( (ret=target_to_host_sockaddr(msg.msg_name, tswapl(msgp->msg_name), msg.msg_namelen,PAGE_WRITE)) != 0 ) return ret; } } else { msg.msg_name = NULL; *************** *** 991,997 **** ret = get_errno(accept(fd, addr, &addrlen)); if (!is_error(ret)) { ! if( (ret2=copy_to_user_sockaddr(target_addr, addr, addrlen)) != 0 ) return ret2; tput32(target_addrlen, addrlen); } return ret; --- 993,999 ---- ret = get_errno(accept(fd, addr, &addrlen)); if (!is_error(ret)) { ! if( (ret2=host_to_target_sockaddr(target_addr, addr, addrlen, PAGE_WRITE)) != 0 ) return ret2; tput32(target_addrlen, addrlen); } return ret; *************** *** 1010,1016 **** ret = get_errno(getpeername(fd, addr, &addrlen)); if (!is_error(ret)) { ! if( (ret2=copy_to_user_sockaddr(target_addr, addr, addrlen)) != 0 ) return ret2; if( (ret2=page_check_range(target_addrlen,sizeof(socklen_t),PAGE_WRITE)) != 0 ) return ret2; tput32(target_addrlen, addrlen); } --- 1012,1018 ---- ret = get_errno(getpeername(fd, addr, &addrlen)); if (!is_error(ret)) { ! if( (ret2=host_to_target_sockaddr(target_addr, addr, addrlen, PAGE_WRITE)) != 0 ) return ret2; if( (ret2=page_check_range(target_addrlen,sizeof(socklen_t),PAGE_WRITE)) != 0 ) return ret2; tput32(target_addrlen, addrlen); } *************** *** 1030,1036 **** ret = get_errno(getsockname(fd, addr, &addrlen)); if (!is_error(ret)) { ! if( (ret2=copy_to_user_sockaddr(target_addr, addr, addrlen)) ) return ret2; tput32(target_addrlen, addrlen); } return ret; --- 1032,1038 ---- ret = get_errno(getsockname(fd, addr, &addrlen)); if (!is_error(ret)) { ! if( (ret2=host_to_target_sockaddr(target_addr, addr, addrlen, PAGE_WRITE)) ) return ret2; tput32(target_addrlen, addrlen); } return ret; *************** *** 1060,1066 **** if( (ret=lock_and_check_user_struct(&host_msg,msg,len,1,PAGE_READ)) != 0 ) return ret; if (target_addr) { addr = alloca(addrlen); ! if( (ret=copy_from_user_sockaddr(addr, target_addr, addrlen)) != 0 ) return ret; ret = get_errno(sendto(fd, host_msg, len, flags, addr, addrlen)); } else { ret = get_errno(send(fd, host_msg, len, flags)); --- 1062,1068 ---- if( (ret=lock_and_check_user_struct(&host_msg,msg,len,1,PAGE_READ)) != 0 ) return ret; if (target_addr) { addr = alloca(addrlen); ! if( (ret=target_to_host_sockaddr(addr, target_addr, addrlen, PAGE_READ)) != 0 ) return ret; ret = get_errno(sendto(fd, host_msg, len, flags, addr, addrlen)); } else { ret = get_errno(send(fd, host_msg, len, flags)); *************** *** 1088,1094 **** } if (!is_error(ret)) { if (target_addr) { ! if( (ret2=copy_to_user_sockaddr(target_addr, addr, addrlen)) != 0 ) return ret2; tput32(target_addrlen, addrlen); } unlock_user(host_msg, msg, len); --- 1090,1096 ---- } if (!is_error(ret)) { if (target_addr) { ! if( (ret2=host_to_target_sockaddr(target_addr, addr, addrlen, PAGE_WRITE)) != 0 ) return ret2; tput32(target_addrlen, addrlen); } unlock_user(host_msg, msg, len); ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH] linux-user EFAULT implementation 2007-07-09 11:41 ` Stuart Anderson 2007-07-09 12:02 ` Stuart Anderson @ 2007-07-09 21:15 ` Fabrice Bellard 2007-07-10 2:22 ` Stuart Anderson 1 sibling, 1 reply; 13+ messages in thread From: Fabrice Bellard @ 2007-07-09 21:15 UTC (permalink / raw) To: qemu-devel Stuart Anderson wrote: > On Fri, 6 Jul 2007, Stuart Anderson wrote: > >> So, the question is: >> >> Can I simplify this code to assume that guest and >> host addresses coexist and use the copy_*_user() or >> just the access_ok() interfaces? No. Ideally you should use the same conventions as the Linux kernel and assume that you cannot access the user data directly. To summarise: - access_ok() is needed before __copy_*_user and __[get|put]_user. - access_ok() is not needed before copy_[to|from]_user and [get|put]_user. - [get|put]_user do automatic endianness change. - Use tswapx and the other related functions to change the endianness before or after copy_*_user. - for specific cases requiring speed (e.g. buffers for read/write syscalls), specific tricks to avoid memory copies can be used with a lock/unlock style API. > Attached is a diff [...] Please send a unified diff. For the time being, I would suggest to minimize the number of changes and just extend lock_user()/unlock_user() as you began to do to handle -EFAULT. The rest is mostly a question of cosmetics. Regards, Fabrice. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH] linux-user EFAULT implementation 2007-07-09 21:15 ` Fabrice Bellard @ 2007-07-10 2:22 ` Stuart Anderson 2007-07-10 12:38 ` Paul Brook 0 siblings, 1 reply; 13+ messages in thread From: Stuart Anderson @ 2007-07-10 2:22 UTC (permalink / raw) To: qemu-devel [-- Attachment #1: Type: TEXT/PLAIN, Size: 1288 bytes --] On Mon, 9 Jul 2007, Fabrice Bellard wrote: > No. Ideally you should use the same conventions as the Linux kernel and > assume that you cannot access the user data directly. That's what I had already started doing today. > For the time being, I would suggest to minimize the number of changes and > just extend lock_user()/unlock_user() as you began to do to handle -EFAULT. > The rest is mostly a question of cosmetics. The attached patch is my in-progress work of the complete overhaul to use the kernel conventions. It needs some more work to finish the conversion, but enough should be done to see how it is going to turn out. Overall, I think the converted code is easier to read, especially if you are familiar with reading kernel code. I also think it will end up being more correct both becasue of the additional time now spent on reviewing each syscall, as well as the kernel conventions tend to make you be more thorough and explicite. Stuart Stuart R. Anderson anderson@netsweng.com Network & Software Engineering http://www.netsweng.com/ 1024D/37A79149: 0791 D3B8 9A4C 2CDC A31F BD03 0A62 E534 37A7 9149 [-- Attachment #2: efault overhaul --] [-- Type: TEXT/x-diff, Size: 52954 bytes --] Index: qemu/exec.c =================================================================== --- qemu.orig/exec.c 2007-07-09 13:18:56.000000000 -0400 +++ qemu/exec.c 2007-07-09 13:19:08.000000000 -0400 @@ -1862,6 +1862,29 @@ spin_unlock(&tb_lock); } +int page_check_range(target_ulong start, target_ulong len, int flags) +{ + PageDesc *p; + target_ulong end; + target_ulong addr; + + end = TARGET_PAGE_ALIGN(start+len); /* must do before we loose bits in the next step */ + start = start & TARGET_PAGE_MASK; + + if( end < start ) return EFAULT; /* we've wrapped around */ + for(addr = start; addr < end; addr += TARGET_PAGE_SIZE) { + p = page_find(addr >> TARGET_PAGE_BITS); + if( !p ) return -EFAULT; + if( !(p->flags & PAGE_VALID) ) return -EFAULT; + + if (!(p->flags & PAGE_READ) && + (flags & PAGE_READ) ) return -EFAULT; + if (!(p->flags & PAGE_WRITE) && + (flags & PAGE_WRITE) ) return -EFAULT; + } + return 0; +} + /* called from signal handler: invalidate the code and unprotect the page. Return TRUE if the fault was succesfully handled. */ int page_unprotect(target_ulong address, unsigned long pc, void *puc) Index: qemu/cpu-all.h =================================================================== --- qemu.orig/cpu-all.h 2007-07-09 13:18:56.000000000 -0400 +++ qemu/cpu-all.h 2007-07-09 13:19:08.000000000 -0400 @@ -691,6 +691,7 @@ int page_get_flags(target_ulong address); void page_set_flags(target_ulong start, target_ulong end, int flags); void page_unprotect_range(target_ulong data, target_ulong data_size); +int page_check_range(target_ulong start, target_ulong len, int flags); CPUState *cpu_copy(CPUState *env); Index: qemu/linux-user/syscall.c =================================================================== --- qemu.orig/linux-user/syscall.c 2007-07-09 13:18:56.000000000 -0400 +++ qemu/linux-user/syscall.c 2007-07-09 22:12:33.000000000 -0400 @@ -404,12 +404,13 @@ #endif } -static inline void host_to_target_rusage(target_ulong target_addr, +static inline long copy_from_user_rusage(target_ulong target_addr, const struct rusage *rusage) { struct target_rusage *target_rusage; - lock_user_struct(target_rusage, target_addr, 0); + if( !access_ok(VERIFY_READ,target_addr,sizeof(*target_rusage)) ) return -1; + target_rusage = (struct target_rusage *)g2h(target_addr); target_rusage->ru_utime.tv_sec = tswapl(rusage->ru_utime.tv_sec); target_rusage->ru_utime.tv_usec = tswapl(rusage->ru_utime.tv_usec); target_rusage->ru_stime.tv_sec = tswapl(rusage->ru_stime.tv_sec); @@ -428,29 +429,34 @@ target_rusage->ru_nsignals = tswapl(rusage->ru_nsignals); target_rusage->ru_nvcsw = tswapl(rusage->ru_nvcsw); target_rusage->ru_nivcsw = tswapl(rusage->ru_nivcsw); - unlock_user_struct(target_rusage, target_addr, 1); + + return 0; } -static inline void target_to_host_timeval(struct timeval *tv, +static inline long copy_from_user_timeval(struct timeval *tv, target_ulong target_addr) { struct target_timeval *target_tv; - lock_user_struct(target_tv, target_addr, 1); + if( !access_ok(VERIFY_READ,target_addr,sizeof(*target_tv)) ) return -1; + target_tv = (struct target_timeval *)g2h(target_addr); tv->tv_sec = tswapl(target_tv->tv_sec); tv->tv_usec = tswapl(target_tv->tv_usec); - unlock_user_struct(target_tv, target_addr, 0); + + return 0; } -static inline void host_to_target_timeval(target_ulong target_addr, +static inline long copy_to_user_timeval(target_ulong target_addr, const struct timeval *tv) { struct target_timeval *target_tv; - lock_user_struct(target_tv, target_addr, 0); + if( !access_ok(VERIFY_WRITE,target_addr,sizeof(*target_tv)) ) return -1; + target_tv = (struct target_timeval *)g2h(target_addr); target_tv->tv_sec = tswapl(tv->tv_sec); target_tv->tv_usec = tswapl(tv->tv_usec); - unlock_user_struct(target_tv, target_addr, 1); + + return 0; } @@ -466,21 +472,21 @@ int ok; if (rfd_p) { - target_rfds = lock_user(rfd_p, sizeof(target_long) * n, 1); + if( lock_and_check_user_struct(&target_rfds,rfd_p,sizeof(target_long)*n,1,PAGE_READ) ) return -EFAULT; rfds_ptr = target_to_host_fds(&rfds, target_rfds, n); } else { target_rfds = NULL; rfds_ptr = NULL; } if (wfd_p) { - target_wfds = lock_user(wfd_p, sizeof(target_long) * n, 1); + if( lock_and_check_user_struct(&target_wfds,wfd_p,sizeof(target_long)*n,1,PAGE_READ) ) return -EFAULT; wfds_ptr = target_to_host_fds(&wfds, target_wfds, n); } else { target_wfds = NULL; wfds_ptr = NULL; } if (efd_p) { - target_efds = lock_user(efd_p, sizeof(target_long) * n, 1); + if( lock_and_check_user_struct(&target_efds,efd_p,sizeof(target_long)*n,1,PAGE_READ) ) return -EFAULT; efds_ptr = target_to_host_fds(&efds, target_efds, n); } else { target_efds = NULL; @@ -488,7 +494,7 @@ } if (target_tv) { - target_to_host_timeval(&tv, target_tv); + if( copy_from_user_timeval(&tv, target_tv) ) return -EFAULT; tv_ptr = &tv; } else { tv_ptr = NULL; @@ -502,7 +508,7 @@ host_to_target_fds(target_efds, efds_ptr, n); if (target_tv) { - host_to_target_timeval(target_tv, &tv); + if( copy_to_user_timeval(target_tv, &tv) ) return -EFAULT; } } if (target_rfds) @@ -515,28 +521,32 @@ return ret; } -static inline void target_to_host_sockaddr(struct sockaddr *addr, +static inline long copy_from_user_sockaddr(struct sockaddr *addr, target_ulong target_addr, socklen_t len) { struct target_sockaddr *target_saddr; - target_saddr = lock_user(target_addr, len, 1); + if( !access_ok(VERIFY_READ,target_addr,len)) return -1; + target_saddr = (struct target_sockaddr *)g2h(target_addr); memcpy(addr, target_saddr, len); addr->sa_family = tswap16(target_saddr->sa_family); - unlock_user(target_saddr, target_addr, 0); + + return 0; } -static inline void host_to_target_sockaddr(target_ulong target_addr, +static inline long copy_to_user_sockaddr(target_ulong target_addr, struct sockaddr *addr, socklen_t len) { struct target_sockaddr *target_saddr; - target_saddr = lock_user(target_addr, len, 0); + if( !access_ok(VERIFY_WRITE,target_addr,len) ) return -1; + target_saddr = (struct target_sockaddr *)g2h(target_addr); memcpy(target_saddr, addr, len); target_saddr->sa_family = tswap16(addr->sa_family); - unlock_user(target_saddr, target_addr, len); + + return 0; } /* ??? Should this also swap msgh->name? */ @@ -899,7 +909,7 @@ { void *addr = alloca(addrlen); - target_to_host_sockaddr(addr, target_addr, addrlen); + if( copy_from_user_sockaddr(addr, target_addr, addrlen) ) return -EFAULT; return get_errno(bind(sockfd, addr, addrlen)); } @@ -908,7 +918,7 @@ { void *addr = alloca(addrlen); - target_to_host_sockaddr(addr, target_addr, addrlen); + if( copy_from_user_sockaddr(addr, target_addr, addrlen) ) return -EFAULT; return get_errno(connect(sockfd, addr, addrlen)); } @@ -922,12 +932,25 @@ struct iovec *vec; target_ulong target_vec; - lock_user_struct(msgp, target_msg, 1); + if( send ) { + if( lock_and_check_user_struct(&msgp,target_msg,sizeof(*msgp),1, + PAGE_READ) ) return -EFAULT; + } else { + if( lock_and_check_user_struct(&msgp,target_msg,sizeof(*msgp),1, + PAGE_WRITE) ) return -EFAULT; + } if (msgp->msg_name) { msg.msg_namelen = tswap32(msgp->msg_namelen); msg.msg_name = alloca(msg.msg_namelen); - target_to_host_sockaddr(msg.msg_name, tswapl(msgp->msg_name), - msg.msg_namelen); + if( send ) { + if( copy_from_user_sockaddr(msg.msg_name, tswapl(msgp->msg_name), + msg.msg_namelen) ) return -EFAULT; + } else { + /* FIXME + if( (ret=copy_from_user_sockaddr(msg.msg_name, tswapl(msgp->msg_name), + msg.msg_namelen,PAGE_WRITE)) != 0 ) return ret; + */ + } } else { msg.msg_name = NULL; msg.msg_namelen = 0; @@ -958,13 +981,15 @@ static long do_accept(int fd, target_ulong target_addr, target_ulong target_addrlen) { - socklen_t addrlen = tget32(target_addrlen); - void *addr = alloca(addrlen); + socklen_t addrlen; + void *addr; long ret; + addr = alloca(addrlen); + ret = get_errno(accept(fd, addr, &addrlen)); if (!is_error(ret)) { - host_to_target_sockaddr(target_addr, addr, addrlen); + if( copy_to_user_sockaddr(target_addr, addr, addrlen) ) return -EFAULT; tput32(target_addrlen, addrlen); } return ret; @@ -973,13 +998,18 @@ static long do_getpeername(int fd, target_ulong target_addr, target_ulong target_addrlen) { - socklen_t addrlen = tget32(target_addrlen); - void *addr = alloca(addrlen); - long ret; + socklen_t addrlen; + void *addr; + long ret,ret2; + + if( (ret=page_check_range(target_addrlen,sizeof(socklen_t),PAGE_READ)) != 0 ) return ret; + addrlen = tget32(target_addrlen); + addr = alloca(addrlen); ret = get_errno(getpeername(fd, addr, &addrlen)); if (!is_error(ret)) { - host_to_target_sockaddr(target_addr, addr, addrlen); + if( copy_to_user_sockaddr(target_addr, addr, addrlen) ) return -EFAULT; + if( (ret2=page_check_range(target_addrlen,sizeof(socklen_t),PAGE_WRITE)) != 0 ) return ret2; tput32(target_addrlen, addrlen); } return ret; @@ -988,13 +1018,17 @@ static long do_getsockname(int fd, target_ulong target_addr, target_ulong target_addrlen) { - socklen_t addrlen = tget32(target_addrlen); - void *addr = alloca(addrlen); + socklen_t addrlen; + void *addr; long ret; + if( (ret=page_check_range(target_addrlen,sizeof(socklen_t),PAGE_WRITE)) != 0 ) return ret; + addrlen = tget32(target_addrlen); + addr = alloca(addrlen); + ret = get_errno(getsockname(fd, addr, &addrlen)); if (!is_error(ret)) { - host_to_target_sockaddr(target_addr, addr, addrlen); + if( copy_to_user_sockaddr(target_addr, addr, addrlen) ) return -EFAULT; tput32(target_addrlen, addrlen); } return ret; @@ -1021,15 +1055,14 @@ void *host_msg; long ret; - host_msg = lock_user(msg, len, 1); + if (copy_from_user(&host_msg,msg,len) ) return -EFAULT; if (target_addr) { addr = alloca(addrlen); - target_to_host_sockaddr(addr, target_addr, addrlen); + if (copy_from_user_sockaddr(addr, target_addr, addrlen) ) return -EFAULT; ret = get_errno(sendto(fd, host_msg, len, flags, addr, addrlen)); } else { ret = get_errno(send(fd, host_msg, len, flags)); } - unlock_user(host_msg, msg, 0); return ret; } @@ -1041,7 +1074,7 @@ void *host_msg; long ret; - host_msg = lock_user(msg, len, 0); + if (lock_and_check_user_struct(&host_msg,msg,len,0,PAGE_WRITE) ) return -EFAULT; if (target_addr) { addrlen = tget32(target_addrlen); addr = alloca(addrlen); @@ -1052,7 +1085,7 @@ } if (!is_error(ret)) { if (target_addr) { - host_to_target_sockaddr(target_addr, addr, addrlen); + if (copy_to_user_sockaddr(target_addr, addr, addrlen) ) return -EFAULT; tput32(target_addrlen, addrlen); } unlock_user(host_msg, msg, len); @@ -1259,64 +1292,44 @@ target_ulong __unused4; }; -static inline void target_to_host_ipc_perm(struct ipc_perm *host_ip, - target_ulong target_addr) +static inline void copy_from_user_ipc_perm(struct ipc_perm *host_ip, + struct target_ipc_perm *target_ip) { - struct target_ipc_perm *target_ip; - struct target_semid_ds *target_sd; - - lock_user_struct(target_sd, target_addr, 1); - target_ip=&(target_sd->sem_perm); host_ip->__key = tswapl(target_ip->__key); host_ip->uid = tswapl(target_ip->uid); host_ip->gid = tswapl(target_ip->gid); host_ip->cuid = tswapl(target_ip->cuid); host_ip->cgid = tswapl(target_ip->cgid); host_ip->mode = tswapl(target_ip->mode); - unlock_user_struct(target_sd, target_addr, 0); } -static inline void host_to_target_ipc_perm(target_ulong target_addr, +static inline void copy_to_user_ipc_perm(struct target_ipc_perm *target_ip, struct ipc_perm *host_ip) { - struct target_ipc_perm *target_ip; - struct target_semid_ds *target_sd; - - lock_user_struct(target_sd, target_addr, 0); - target_ip = &(target_sd->sem_perm); target_ip->__key = tswapl(host_ip->__key); target_ip->uid = tswapl(host_ip->uid); target_ip->gid = tswapl(host_ip->gid); target_ip->cuid = tswapl(host_ip->cuid); target_ip->cgid = tswapl(host_ip->cgid); target_ip->mode = tswapl(host_ip->mode); - unlock_user_struct(target_sd, target_addr, 1); } -static inline void target_to_host_semid_ds(struct semid_ds *host_sd, - target_ulong target_addr) +static inline void copy_from_user_semid_ds(struct semid_ds *host_sd, + struct target_semid_ds *target_sd) { - struct target_semid_ds *target_sd; - - lock_user_struct(target_sd, target_addr, 1); - target_to_host_ipc_perm(&(host_sd->sem_perm),target_addr); + copy_from_user_ipc_perm(&(host_sd->sem_perm),&(target_sd->sem_perm)); 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); - unlock_user_struct(target_sd, target_addr, 0); } -static inline void host_to_target_semid_ds(target_ulong target_addr, +static inline void copy_to_user_semid_ds(struct target_semid_ds *target_sd, struct semid_ds *host_sd) { - struct target_semid_ds *target_sd; - - lock_user_struct(target_sd, target_addr, 0); - host_to_target_ipc_perm(target_addr,&(host_sd->sem_perm)); + copy_to_user_ipc_perm(&(target_sd->sem_perm),&(host_sd->sem_perm)); 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); - unlock_user_struct(target_sd, target_addr, 1); } union semun { @@ -1331,67 +1344,73 @@ unsigned short int *array; }; -static inline void target_to_host_semun(unsigned long cmd, +static inline long copy_from_user_semun(unsigned long cmd, union semun *host_su, target_ulong target_addr, - struct semid_ds *ds) + struct semid_ds *ds, + int pg_access) { union target_semun *target_su; switch( cmd ) { case IPC_STAT: case IPC_SET: - lock_user_struct(target_su, target_addr, 1); - target_to_host_semid_ds(ds,target_su->buf); + if (lock_and_check_user_struct(&target_su,target_addr,sizeof(*target_su),1,pg_access) ) return -1; + copy_from_user_semid_ds(ds,target_su->buf); host_su->buf = ds; unlock_user_struct(target_su, target_addr, 0); break; case GETVAL: case SETVAL: - lock_user_struct(target_su, target_addr, 1); + if (lock_and_check_user_struct(&target_su,target_addr,sizeof(*target_su),1,pg_access) ) return -1; host_su->val = tswapl(target_su->val); unlock_user_struct(target_su, target_addr, 0); break; case GETALL: case SETALL: - lock_user_struct(target_su, target_addr, 1); + if (lock_and_check_user_struct(&target_su,target_addr,sizeof(*target_su),1,pg_access) ) return -1; *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); } + + return 0; } -static inline void host_to_target_semun(unsigned long cmd, +static inline long copy_to_user_semun(unsigned long cmd, target_ulong target_addr, union semun *host_su, - struct semid_ds *ds) + struct semid_ds *ds, + int pg_access) { union target_semun *target_su; switch( cmd ) { case IPC_STAT: case IPC_SET: - lock_user_struct(target_su, target_addr, 0); - host_to_target_semid_ds(target_su->buf,ds); + if (lock_and_check_user_struct(&target_su,target_addr,sizeof(*target_su),0,pg_access) ) return -1; + copy_to_user_semid_ds(target_su->buf,ds); unlock_user_struct(target_su, target_addr, 1); break; case GETVAL: case SETVAL: - lock_user_struct(target_su, target_addr, 0); + if (lock_and_check_user_struct(&target_su,target_addr,sizeof(*target_su),0,pg_access) ) return -1; target_su->val = tswapl(host_su->val); unlock_user_struct(target_su, target_addr, 1); break; case GETALL: case SETALL: - lock_user_struct(target_su, target_addr, 0); + if (lock_and_check_user_struct(&target_su,target_addr,sizeof(*target_su),0,pg_access) ) return -1; *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); } + + return 0; } static inline long do_semctl(long first, long second, long third, long ptr) @@ -1403,34 +1422,34 @@ switch( cmd ) { case GETVAL: - target_to_host_semun(cmd,&arg,ptr,&dsarg); + if( copy_from_user_semun(cmd,&arg,ptr,&dsarg,PAGE_WRITE) ) return -EFAULT; ret = get_errno(semctl(first, second, cmd, arg)); - host_to_target_semun(cmd,ptr,&arg,&dsarg); + if( copy_to_user_semun(cmd,ptr,&arg,&dsarg,PAGE_WRITE) ) return -EFAULT; break; case SETVAL: - target_to_host_semun(cmd,&arg,ptr,&dsarg); + if( copy_from_user_semun(cmd,&arg,ptr,&dsarg,PAGE_READ) ) return -EFAULT; ret = get_errno(semctl(first, second, cmd, arg)); - host_to_target_semun(cmd,ptr,&arg,&dsarg); + if( copy_to_user_semun(cmd,ptr,&arg,&dsarg,PAGE_READ) ) return -EFAULT; break; case GETALL: - target_to_host_semun(cmd,&arg,ptr,&dsarg); + if( copy_from_user_semun(cmd,&arg,ptr,&dsarg,PAGE_WRITE) ) return -EFAULT; ret = get_errno(semctl(first, second, cmd, arg)); - host_to_target_semun(cmd,ptr,&arg,&dsarg); + if( copy_to_user_semun(cmd,ptr,&arg,&dsarg,PAGE_WRITE) ) return -EFAULT; break; case SETALL: - target_to_host_semun(cmd,&arg,ptr,&dsarg); + if( copy_from_user_semun(cmd,&arg,ptr,&dsarg,PAGE_READ) ) return -EFAULT; ret = get_errno(semctl(first, second, cmd, arg)); - host_to_target_semun(cmd,ptr,&arg,&dsarg); + if( copy_to_user_semun(cmd,ptr,&arg,&dsarg,PAGE_READ) ) return -EFAULT; break; case IPC_STAT: - target_to_host_semun(cmd,&arg,ptr,&dsarg); + if( copy_from_user_semun(cmd,&arg,ptr,&dsarg,PAGE_WRITE) ) return -EFAULT; ret = get_errno(semctl(first, second, cmd, arg)); - host_to_target_semun(cmd,ptr,&arg,&dsarg); + if( copy_to_user_semun(cmd,ptr,&arg,&dsarg,PAGE_READ) ) return -EFAULT; break; case IPC_SET: - target_to_host_semun(cmd,&arg,ptr,&dsarg); + if( copy_from_user_semun(cmd,&arg,ptr,&dsarg,PAGE_READ) ) return -EFAULT; ret = get_errno(semctl(first, second, cmd, arg)); - host_to_target_semun(cmd,ptr,&arg,&dsarg); + if( copy_to_user_semun(cmd,ptr,&arg,&dsarg,PAGE_READ) ) return -EFAULT; break; default: ret = get_errno(semctl(first, second, cmd, arg)); @@ -1457,13 +1476,14 @@ target_ulong __unused5; }; -static inline void target_to_host_msqid_ds(struct msqid_ds *host_md, - target_ulong target_addr) +static inline long target_to_host_msqid_ds(struct msqid_ds *host_md, + target_ulong target_addr, + int pg_access) { struct target_msqid_ds *target_md; - lock_user_struct(target_md, target_addr, 1); - target_to_host_ipc_perm(&(host_md->msg_perm),target_addr); + if( lock_and_check_user_struct(&target_md,target_addr,sizeof(*target_md),1,pg_access) ) return -1; + copy_from_user_ipc_perm(&(host_md->msg_perm),&(target_md->msg_perm)); host_md->msg_stime = tswapl(target_md->msg_stime); host_md->msg_rtime = tswapl(target_md->msg_rtime); host_md->msg_ctime = tswapl(target_md->msg_ctime); @@ -1473,15 +1493,18 @@ host_md->msg_lspid = tswapl(target_md->msg_lspid); host_md->msg_lrpid = tswapl(target_md->msg_lrpid); unlock_user_struct(target_md, target_addr, 0); + + return 0; } -static inline void host_to_target_msqid_ds(target_ulong target_addr, - struct msqid_ds *host_md) +static inline long host_to_target_msqid_ds(target_ulong target_addr, + struct msqid_ds *host_md, + int pg_access) { struct target_msqid_ds *target_md; - lock_user_struct(target_md, target_addr, 0); - host_to_target_ipc_perm(target_addr,&(host_md->msg_perm)); + if( lock_and_check_user_struct(&target_md,target_addr,sizeof(*target_md),0,pg_access) ) return -1; + copy_to_user_ipc_perm(&(target_md->msg_perm),&(host_md->msg_perm)); target_md->msg_stime = tswapl(host_md->msg_stime); target_md->msg_rtime = tswapl(host_md->msg_rtime); target_md->msg_ctime = tswapl(host_md->msg_ctime); @@ -1491,6 +1514,8 @@ target_md->msg_lspid = tswapl(host_md->msg_lspid); target_md->msg_lrpid = tswapl(host_md->msg_lrpid); unlock_user_struct(target_md, target_addr, 1); + + return 0; } static inline long do_msgctl(long first, long second, long ptr) @@ -1500,10 +1525,15 @@ long ret = 0; switch( cmd ) { case IPC_STAT: + if( (ret=target_to_host_msqid_ds(&dsarg,ptr,PAGE_WRITE)) != 0 ) return ret; + ret = get_errno(msgctl(first, cmd, &dsarg)); + host_to_target_msqid_ds(ptr,&dsarg,PAGE_WRITE); + break; case IPC_SET: - target_to_host_msqid_ds(&dsarg,ptr); + if( (ret=target_to_host_msqid_ds(&dsarg,ptr,PAGE_READ)) != 0 ) return ret; ret = get_errno(msgctl(first, cmd, &dsarg)); - host_to_target_msqid_ds(ptr,&dsarg); + host_to_target_msqid_ds(ptr,&dsarg,PAGE_WRITE); + break; default: ret = get_errno(msgctl(first, cmd, &dsarg)); } @@ -1521,7 +1551,7 @@ struct msgbuf *host_mb; long ret = 0; - lock_user_struct(target_mb,msgp,0); + if( lock_and_check_user_struct(&target_mb,msgp,sizeof(long)+msgsz,1,PAGE_READ) ) return -EFAULT; host_mb = malloc(msgsz+sizeof(long)); host_mb->mtype = tswapl(target_mb->mtype); memcpy(host_mb->mtext,target_mb->mtext,msgsz); @@ -1538,7 +1568,7 @@ struct msgbuf *host_mb; long ret = 0; - lock_user_struct(target_mb, msgp, 0); + if( lock_and_check_user_struct(&target_mb,msgp,sizeof(long)+msgsz,0,PAGE_WRITE) ) return -EFAULT; host_mb = malloc(msgsz+sizeof(long)); ret = get_errno(msgrcv(msqid, host_mb, msgsz, 1, msgflg)); if (ret > 0) @@ -2018,6 +2048,7 @@ static int write_ldt(CPUX86State *env, target_ulong ptr, unsigned long bytecount, int oldmode) { + long ret = 0; struct target_modify_ldt_ldt_s ldt_info; struct target_modify_ldt_ldt_s *target_ldt_info; int seg_32bit, contents, read_exec_only, limit_in_pages; @@ -2026,7 +2057,7 @@ if (bytecount != sizeof(ldt_info)) return -EINVAL; - lock_user_struct(target_ldt_info, ptr, 1); + if( lock_and_check_user_struct(&target_ldt_info,ptr,sizeof(struct target_modify_ldt_ldt_s),1,PAGE_READ) ) return -EFAULT; ldt_info.entry_number = tswap32(target_ldt_info->entry_number); ldt_info.base_addr = tswapl(target_ldt_info->base_addr); ldt_info.limit = tswap32(target_ldt_info->limit); @@ -2225,7 +2256,7 @@ switch(cmd) { case TARGET_F_GETLK: - lock_user_struct(target_fl, arg, 1); + if( lock_and_check_user_struct(&target_fl,arg,sizeof(struct target_flock),1,PAGE_READ) ) return -EFAULT; fl.l_type = tswap16(target_fl->l_type); fl.l_whence = tswap16(target_fl->l_whence); fl.l_start = tswapl(target_fl->l_start); @@ -2234,6 +2265,7 @@ unlock_user_struct(target_fl, arg, 0); ret = fcntl(fd, cmd, &fl); if (ret == 0) { + if( lock_and_check_user_struct(&target_fl,arg,sizeof(struct target_flock),0,PAGE_WRITE) ) return -EFAULT; lock_user_struct(target_fl, arg, 0); target_fl->l_type = tswap16(fl.l_type); target_fl->l_whence = tswap16(fl.l_whence); @@ -2246,7 +2278,7 @@ case TARGET_F_SETLK: case TARGET_F_SETLKW: - lock_user_struct(target_fl, arg, 1); + if( lock_and_check_user_struct(&target_fl,arg,sizeof(struct target_flock),1,PAGE_READ) ) return -EFAULT; fl.l_type = tswap16(target_fl->l_type); fl.l_whence = tswap16(target_fl->l_whence); fl.l_start = tswapl(target_fl->l_start); @@ -2257,7 +2289,7 @@ break; case TARGET_F_GETLK64: - lock_user_struct(target_fl64, arg, 1); + if( lock_and_check_user_struct(&target_fl64,arg,sizeof(struct target_flock64),1,PAGE_READ) ) return -EFAULT; fl64.l_type = tswap16(target_fl64->l_type) >> 1; fl64.l_whence = tswap16(target_fl64->l_whence); fl64.l_start = tswapl(target_fl64->l_start); @@ -2266,7 +2298,7 @@ unlock_user_struct(target_fl64, arg, 0); ret = fcntl(fd, cmd >> 1, &fl64); if (ret == 0) { - lock_user_struct(target_fl64, arg, 0); + if( lock_and_check_user_struct(&target_fl64,arg,sizeof(struct target_flock64),0,PAGE_WRITE) ) return -EFAULT; target_fl64->l_type = tswap16(fl64.l_type) >> 1; target_fl64->l_whence = tswap16(fl64.l_whence); target_fl64->l_start = tswapl(fl64.l_start); @@ -2277,7 +2309,7 @@ break; case TARGET_F_SETLK64: case TARGET_F_SETLKW64: - lock_user_struct(target_fl64, arg, 1); + if( lock_and_check_user_struct(&target_fl64,arg,sizeof(struct target_flock64),1,PAGE_READ) ) return -EFAULT; fl64.l_type = tswap16(target_fl64->l_type) >> 1; fl64.l_whence = tswap16(target_fl64->l_whence); fl64.l_start = tswapl(target_fl64->l_start); @@ -2419,32 +2451,37 @@ } #endif -static inline void target_to_host_timespec(struct timespec *host_ts, +static inline long target_to_host_timespec(struct timespec *host_ts, target_ulong target_addr) { struct target_timespec *target_ts; - lock_user_struct(target_ts, target_addr, 1); + if( lock_and_check_user_struct(&target_ts,target_addr,sizeof(struct target_timespec),1,PAGE_READ) ) return -1; host_ts->tv_sec = tswapl(target_ts->tv_sec); host_ts->tv_nsec = tswapl(target_ts->tv_nsec); unlock_user_struct(target_ts, target_addr, 0); + + return 0; } -static inline void host_to_target_timespec(target_ulong target_addr, +static inline long host_to_target_timespec(target_ulong target_addr, struct timespec *host_ts) { struct target_timespec *target_ts; lock_user_struct(target_ts, target_addr, 0); + if( lock_and_check_user_struct(&target_ts,target_addr,sizeof(struct target_timespec),0,PAGE_WRITE) ) return -1; target_ts->tv_sec = tswapl(host_ts->tv_sec); target_ts->tv_nsec = tswapl(host_ts->tv_nsec); unlock_user_struct(target_ts, target_addr, 1); + + return 0; } long do_syscall(void *cpu_env, int num, long arg1, long arg2, long arg3, long arg4, long arg5, long arg6) { - long ret; + long ret = 0; struct stat st; struct statfs stfs; void *p; @@ -2671,7 +2708,7 @@ struct utimbuf tbuf, *host_tbuf; struct target_utimbuf *target_tbuf; if (arg2) { - lock_user_struct(target_tbuf, arg2, 1); + if( lock_and_check_user_struct(&target_tbuf,arg2,sizeof(struct target_utimbuf),1,PAGE_READ) ) return -EFAULT; tbuf.actime = tswapl(target_tbuf->actime); tbuf.modtime = tswapl(target_tbuf->modtime); unlock_user_struct(target_tbuf, arg2, 0); @@ -2689,9 +2726,11 @@ { struct timeval *tvp, tv[2]; if (arg2) { - target_to_host_timeval(&tv[0], arg2); - target_to_host_timeval(&tv[1], - arg2 + sizeof (struct target_timeval)); + if( copy_from_user_timeval(&tv[0], arg2) ) + return -EFAULT; + if( copy_from_user_timeval(&tv[1], + arg2 + sizeof (struct target_timeval)) ) + return -EFAULT; tvp = tv; } else { tvp = NULL; @@ -2860,7 +2899,7 @@ struct target_old_sigaction *old_act; struct target_sigaction act, oact, *pact; if (arg2) { - lock_user_struct(old_act, arg2, 1); + if( lock_and_check_user_struct(&old_act,arg2,sizeof(*old_act),1,PAGE_READ) ) return -EFAULT; act._sa_handler = old_act->_sa_handler; target_siginitset(&act.sa_mask, old_act->sa_mask); act.sa_flags = old_act->sa_flags; @@ -2872,7 +2911,7 @@ } ret = get_errno(do_sigaction(arg1, pact, &oact)); if (!is_error(ret) && arg3) { - lock_user_struct(old_act, arg3, 0); + if( lock_and_check_user_struct(&old_act,arg3,sizeof(*old_act),0,PAGE_WRITE) ) return -EFAULT; old_act->_sa_handler = oact._sa_handler; old_act->sa_mask = oact.sa_mask.sig[0]; old_act->sa_flags = oact.sa_flags; @@ -2883,7 +2922,7 @@ struct target_sigaction act, oact, *pact, *old_act; if (arg2) { - lock_user_struct(old_act, arg2, 1); + if( lock_and_check_user_struct(&old_act,arg2,sizeof(*old_act),1,PAGE_READ) ) return -EFAULT; act._sa_handler = old_act->_sa_handler; target_siginitset(&act.sa_mask, old_act->sa_mask.sig[0]); act.sa_flags = old_act->sa_flags; @@ -2896,7 +2935,7 @@ ret = get_errno(do_sigaction(arg1, pact, &oact)); if (!is_error(ret) && arg3) { - lock_user_struct(old_act, arg3, 0); + if( lock_and_check_user_struct(&old_act,arg3,sizeof(*old_act),0,PAGE_WRITE) ) return -EFAULT; old_act->_sa_handler = oact._sa_handler; old_act->sa_flags = oact.sa_flags; old_act->sa_mask.sig[0] = oact.sa_mask.sig[0]; @@ -2914,12 +2953,14 @@ struct target_sigaction *act; struct target_sigaction *oact; - if (arg2) - lock_user_struct(act, arg2, 1); + if (arg2) { + if( lock_and_check_user_struct(&act,arg2,sizeof(*act),1,PAGE_READ) ) return -EFAULT; + } else act = NULL; - if (arg3) - lock_user_struct(oact, arg3, 0); + if (arg3) { + if( lock_and_check_user_struct(&oact,arg2,sizeof(*oact),0,PAGE_WRITE) ) return -EFAULT; + } else oact = NULL; ret = get_errno(do_sigaction(arg1, act, oact)); @@ -3046,7 +3087,7 @@ sigset_t set; ret = get_errno(sigpending(&set)); if (!is_error(ret)) { - p = lock_user(arg1, sizeof(target_sigset_t), 0); + if( lock_and_check_user_struct(&p,arg1,sizeof(target_sigset_t),0,PAGE_WRITE) ) return -EFAULT; host_to_target_sigset(p, &set); unlock_user(p, arg1, sizeof(target_sigset_t)); } @@ -3056,7 +3097,7 @@ case TARGET_NR_sigsuspend: { sigset_t set; - p = lock_user(arg1, sizeof(target_sigset_t), 1); + if( lock_and_check_user_struct(&p,arg1,sizeof(target_sigset_t),1,PAGE_READ) ) return -EFAULT; target_to_host_old_sigset(&set, p); unlock_user(p, arg1, 0); ret = get_errno(sigsuspend(&set)); @@ -3066,7 +3107,7 @@ case TARGET_NR_rt_sigsuspend: { sigset_t set; - p = lock_user(arg1, sizeof(target_sigset_t), 1); + if( lock_and_check_user_struct(&p,arg1,sizeof(target_sigset_t),1,PAGE_READ) ) return -EFAULT; target_to_host_sigset(&set, p); unlock_user(p, arg1, 0); ret = get_errno(sigsuspend(&set)); @@ -3078,18 +3119,18 @@ struct timespec uts, *puts; siginfo_t uinfo; - p = lock_user(arg1, sizeof(target_sigset_t), 1); + if( lock_and_check_user_struct(&p,arg1,sizeof(target_sigset_t),1,PAGE_READ) ) return -EFAULT; target_to_host_sigset(&set, p); unlock_user(p, arg1, 0); if (arg3) { puts = &uts; - target_to_host_timespec(puts, arg3); + if( (ret=target_to_host_timespec(puts, arg3)) != 0 ) return ret; } else { puts = NULL; } ret = get_errno(sigtimedwait(&set, &uinfo, puts)); if (!is_error(ret) && arg2) { - p = lock_user(arg2, sizeof(target_sigset_t), 0); + if( lock_and_check_user_struct(&p,arg2,sizeof(target_sigset_t),0,PAGE_WRITE) ) return -EFAULT; host_to_target_siginfo(p, &uinfo); unlock_user(p, arg2, sizeof(target_sigset_t)); } @@ -3098,7 +3139,7 @@ case TARGET_NR_rt_sigqueueinfo: { siginfo_t uinfo; - p = lock_user(arg3, sizeof(target_sigset_t), 1); + if( lock_and_check_user_struct(&p,arg3,sizeof(target_sigset_t),1,PAGE_READ) ) return -EFAULT; target_to_host_siginfo(&uinfo, p); unlock_user(p, arg1, 0); ret = get_errno(sys_rt_sigqueueinfo(arg1, arg2, &uinfo)); @@ -3153,7 +3194,7 @@ struct rusage rusage; ret = get_errno(getrusage(arg1, &rusage)); if (!is_error(ret)) { - host_to_target_rusage(arg2, &rusage); + if( copy_from_user_rusage(arg2, &rusage) ) return -EFAULT; } } break; @@ -3162,14 +3203,14 @@ struct timeval tv; ret = get_errno(gettimeofday(&tv, NULL)); if (!is_error(ret)) { - host_to_target_timeval(arg1, &tv); + if( copy_to_user_timeval(arg1, &tv) ) return -EFAULT; } } break; case TARGET_NR_settimeofday: { struct timeval tv; - target_to_host_timeval(&tv, arg1); + if( copy_from_user_timeval(&tv, arg1) ) return -EFAULT; ret = get_errno(settimeofday(&tv, NULL)); } break; @@ -3335,20 +3376,19 @@ unlock_user(p, arg1, 0); convert_statfs: if (!is_error(ret)) { - struct target_statfs *target_stfs; + struct target_statfs *target_stfs = (struct target_statfs *)arg2; - lock_user_struct(target_stfs, arg2, 0); - /* ??? put_user is probably wrong. */ - put_user(stfs.f_type, &target_stfs->f_type); - put_user(stfs.f_bsize, &target_stfs->f_bsize); - put_user(stfs.f_blocks, &target_stfs->f_blocks); - put_user(stfs.f_bfree, &target_stfs->f_bfree); - put_user(stfs.f_bavail, &target_stfs->f_bavail); - put_user(stfs.f_files, &target_stfs->f_files); - put_user(stfs.f_ffree, &target_stfs->f_ffree); - put_user(stfs.f_fsid.__val[0], &target_stfs->f_fsid); - put_user(stfs.f_namelen, &target_stfs->f_namelen); - unlock_user_struct(target_stfs, arg2, 1); + if( !access_ok(VERIFY_WRITE,target_stfs,sizeof(struct target_statfs)) ) return -EFAULT; + /* use __put_user since we just validated the entire bufffer. */ + __put_user(stfs.f_type, &target_stfs->f_type); + __put_user(stfs.f_bsize, &target_stfs->f_bsize); + __put_user(stfs.f_blocks, &target_stfs->f_blocks); + __put_user(stfs.f_bfree, &target_stfs->f_bfree); + __put_user(stfs.f_bavail, &target_stfs->f_bavail); + __put_user(stfs.f_files, &target_stfs->f_files); + __put_user(stfs.f_ffree, &target_stfs->f_ffree); + __put_user(stfs.f_fsid.__val[0], &target_stfs->f_fsid); + __put_user(stfs.f_namelen, &target_stfs->f_namelen); } break; case TARGET_NR_fstatfs: @@ -3361,20 +3401,19 @@ unlock_user(p, arg1, 0); convert_statfs64: if (!is_error(ret)) { - struct target_statfs64 *target_stfs; + struct target_statfs64 *target_stfs = (struct target_statfs64 *)arg3; - lock_user_struct(target_stfs, arg3, 0); - /* ??? put_user is probably wrong. */ - put_user(stfs.f_type, &target_stfs->f_type); - put_user(stfs.f_bsize, &target_stfs->f_bsize); - put_user(stfs.f_blocks, &target_stfs->f_blocks); - put_user(stfs.f_bfree, &target_stfs->f_bfree); - put_user(stfs.f_bavail, &target_stfs->f_bavail); - put_user(stfs.f_files, &target_stfs->f_files); - put_user(stfs.f_ffree, &target_stfs->f_ffree); - put_user(stfs.f_fsid.__val[0], &target_stfs->f_fsid); - put_user(stfs.f_namelen, &target_stfs->f_namelen); - unlock_user_struct(target_stfs, arg3, 0); + if( !access_ok(VERIFY_WRITE,target_stfs,sizeof(struct target_statfs64)) ) return -EFAULT; + /* use __put_user since we just validated the entire bufffer. */ + __put_user(stfs.f_type, &target_stfs->f_type); + __put_user(stfs.f_bsize, &target_stfs->f_bsize); + __put_user(stfs.f_blocks, &target_stfs->f_blocks); + __put_user(stfs.f_bfree, &target_stfs->f_bfree); + __put_user(stfs.f_bavail, &target_stfs->f_bavail); + __put_user(stfs.f_files, &target_stfs->f_files); + __put_user(stfs.f_ffree, &target_stfs->f_ffree); + __put_user(stfs.f_fsid.__val[0], &target_stfs->f_fsid); + __put_user(stfs.f_namelen, &target_stfs->f_namelen); } break; case TARGET_NR_fstatfs64: @@ -3488,19 +3527,20 @@ if (arg2) { pvalue = &value; - target_to_host_timeval(&pvalue->it_interval, - arg2); - target_to_host_timeval(&pvalue->it_value, - arg2 + sizeof(struct target_timeval)); + if( copy_from_user_timeval(&pvalue->it_interval, arg2) ) + return -EFAULT; + if( copy_from_user_timeval(&pvalue->it_value, + arg2 + sizeof(struct target_timeval)) ) + return -EFAULT; } else { pvalue = NULL; } ret = get_errno(setitimer(arg1, pvalue, &ovalue)); if (!is_error(ret) && arg3) { - host_to_target_timeval(arg3, - &ovalue.it_interval); - host_to_target_timeval(arg3 + sizeof(struct target_timeval), - &ovalue.it_value); + if( copy_to_user_timeval(arg3, + &ovalue.it_interval) ) return -EFAULT; + if( copy_to_user_timeval(arg3 + sizeof(struct target_timeval), + &ovalue.it_value) ) return -EFAULT; } } break; @@ -3510,10 +3550,10 @@ ret = get_errno(getitimer(arg1, &value)); if (!is_error(ret) && arg2) { - host_to_target_timeval(arg2, - &value.it_interval); - host_to_target_timeval(arg2 + sizeof(struct target_timeval), - &value.it_value); + if( copy_to_user_timeval(arg2, + &value.it_interval) ) return -EFAULT; + if( copy_to_user_timeval(arg2 + sizeof(struct target_timeval), + &value.it_value) ) return -EFAULT; } } break; @@ -3534,7 +3574,7 @@ if (!is_error(ret)) { struct target_stat *target_st; - lock_user_struct(target_st, arg2, 0); + if( lock_and_check_user_struct(&target_st,arg2,sizeof(*target_st),9,PAGE_WRITE) ) return -EFAULT; #if defined(TARGET_MIPS) target_st->st_dev = tswap32(st.st_dev); #else @@ -3603,7 +3643,8 @@ if (status_ptr) tputl(status_ptr, status); if (target_rusage) { - host_to_target_rusage(target_rusage, &rusage); + if( !copy_from_user_rusage(target_rusage, &rusage) ) + return -EFAULT; } } } @@ -3965,17 +4006,17 @@ struct timespec ts; ret = get_errno(sched_rr_get_interval(arg1, &ts)); if (!is_error(ret)) { - host_to_target_timespec(arg2, &ts); + if( (ret=host_to_target_timespec(arg2, &ts)) != 0 ) return ret; } } break; case TARGET_NR_nanosleep: { struct timespec req, rem; - target_to_host_timespec(&req, arg1); + if( (ret=target_to_host_timespec(&req, arg1)) != 0 ) return ret; ret = get_errno(nanosleep(&req, &rem)); if (is_error(ret) && arg2) { - host_to_target_timespec(arg2, &rem); + if( (ret=host_to_target_timespec(arg2, &rem)) != 0 ) return ret; } } break; @@ -4072,6 +4113,7 @@ #ifdef TARGET_NR_stat64 case TARGET_NR_stat64: p = lock_user_string(arg1); + if( ret=page_check_range(p,1,PAGE_READ) ) return ret; ret = get_errno(stat(path(p), &st)); unlock_user(p, arg1, 0); goto do_stat64; @@ -4079,6 +4121,7 @@ #ifdef TARGET_NR_lstat64 case TARGET_NR_lstat64: p = lock_user_string(arg1); + if( ret=page_check_range(p,1,PAGE_READ) ) return ret; ret = get_errno(lstat(path(p), &st)); unlock_user(p, arg1, 0); goto do_stat64; @@ -4091,53 +4134,51 @@ if (!is_error(ret)) { #ifdef TARGET_ARM if (((CPUARMState *)cpu_env)->eabi) { - struct target_eabi_stat64 *target_st; - lock_user_struct(target_st, arg2, 1); + struct target_eabi_stat64 *target_st = (struct target_eabi_stat64 *)arg2; + if( !access_ok(VERIFY_WRITE,target_st,sizeof(struct target_eabi_stat64)) ) return -EFAULT; memset(target_st, 0, sizeof(struct target_eabi_stat64)); - /* put_user is probably wrong. */ - put_user(st.st_dev, &target_st->st_dev); - put_user(st.st_ino, &target_st->st_ino); + /* use __put_user() since we just checked that the buffer is valid */ + __put_user(st.st_dev, &target_st->st_dev); + __put_user(st.st_ino, &target_st->st_ino); #ifdef TARGET_STAT64_HAS_BROKEN_ST_INO - put_user(st.st_ino, &target_st->__st_ino); + __put_user(st.st_ino, &target_st->__st_ino); #endif - put_user(st.st_mode, &target_st->st_mode); - put_user(st.st_nlink, &target_st->st_nlink); - put_user(st.st_uid, &target_st->st_uid); - put_user(st.st_gid, &target_st->st_gid); - put_user(st.st_rdev, &target_st->st_rdev); + __put_user(st.st_mode, &target_st->st_mode); + __put_user(st.st_nlink, &target_st->st_nlink); + __put_user(st.st_uid, &target_st->st_uid); + __put_user(st.st_gid, &target_st->st_gid); + __put_user(st.st_rdev, &target_st->st_rdev); /* XXX: better use of kernel struct */ - put_user(st.st_size, &target_st->st_size); - put_user(st.st_blksize, &target_st->st_blksize); - put_user(st.st_blocks, &target_st->st_blocks); - put_user(st.st_atime, &target_st->target_st_atime); - put_user(st.st_mtime, &target_st->target_st_mtime); - put_user(st.st_ctime, &target_st->target_st_ctime); - unlock_user_struct(target_st, arg2, 0); + __put_user(st.st_size, &target_st->st_size); + __put_user(st.st_blksize, &target_st->st_blksize); + __put_user(st.st_blocks, &target_st->st_blocks); + __put_user(st.st_atime, &target_st->target_st_atime); + __put_user(st.st_mtime, &target_st->target_st_mtime); + __put_user(st.st_ctime, &target_st->target_st_ctime); } else #endif { - struct target_stat64 *target_st; - lock_user_struct(target_st, arg2, 1); + struct target_stat64 *target_st = (struct target_stat64 *)arg2; + if( !access_ok(VERIFY_WRITE,target_st,sizeof(struct target_stat64)) ) return -EFAULT; memset(target_st, 0, sizeof(struct target_stat64)); - /* ??? put_user is probably wrong. */ - put_user(st.st_dev, &target_st->st_dev); - put_user(st.st_ino, &target_st->st_ino); + /* use __put_user() since we just checked that the buffer is valid */ + __put_user(st.st_dev, &target_st->st_dev); + __put_user(st.st_ino, &target_st->st_ino); #ifdef TARGET_STAT64_HAS_BROKEN_ST_INO - put_user(st.st_ino, &target_st->__st_ino); + __put_user(st.st_ino, &target_st->__st_ino); #endif - put_user(st.st_mode, &target_st->st_mode); - put_user(st.st_nlink, &target_st->st_nlink); - put_user(st.st_uid, &target_st->st_uid); - put_user(st.st_gid, &target_st->st_gid); - put_user(st.st_rdev, &target_st->st_rdev); + __put_user(st.st_mode, &target_st->st_mode); + __put_user(st.st_nlink, &target_st->st_nlink); + __put_user(st.st_uid, &target_st->st_uid); + __put_user(st.st_gid, &target_st->st_gid); + __put_user(st.st_rdev, &target_st->st_rdev); /* XXX: better use of kernel struct */ - put_user(st.st_size, &target_st->st_size); - put_user(st.st_blksize, &target_st->st_blksize); - put_user(st.st_blocks, &target_st->st_blocks); - put_user(st.st_atime, &target_st->target_st_atime); - put_user(st.st_mtime, &target_st->target_st_mtime); - put_user(st.st_ctime, &target_st->target_st_ctime); - unlock_user_struct(target_st, arg2, 0); + __put_user(st.st_size, &target_st->st_size); + __put_user(st.st_blksize, &target_st->st_blksize); + __put_user(st.st_blocks, &target_st->st_blocks); + __put_user(st.st_atime, &target_st->target_st_atime); + __put_user(st.st_mtime, &target_st->target_st_mtime); + __put_user(st.st_ctime, &target_st->target_st_ctime); } } } @@ -4583,7 +4624,7 @@ struct timespec ts; ret = get_errno(clock_gettime(arg1, &ts)); if (!is_error(ret)) { - host_to_target_timespec(arg2, &ts); + if( (ret=host_to_target_timespec(arg2, &ts)) != 0 ) return ret; } break; } @@ -4594,7 +4635,7 @@ struct timespec ts; ret = get_errno(clock_getres(arg1, &ts)); if (!is_error(ret)) { - host_to_target_timespec(arg2, &ts); + if( (ret=host_to_target_timespec(arg2, &ts)) != 0 ) return ret; } break; } Index: qemu/linux-user/qemu.h =================================================================== --- qemu.orig/linux-user/qemu.h 2007-07-09 13:18:56.000000000 -0400 +++ qemu/linux-user/qemu.h 2007-07-09 21:26:22.000000000 -0400 @@ -171,7 +171,8 @@ #define VERIFY_READ 0 #define VERIFY_WRITE 1 -#define access_ok(type,addr,size) (1) +#define access_ok(type,addr,size) \ + (page_check_range((target_ulong)addr,size,(type==VERIFY_READ)?PAGE_READ:PAGE_WRITE)==0) /* NOTE get_user and put_user use host addresses. */ #define __put_user(x,ptr)\ @@ -218,13 +219,14 @@ 0;\ }) +/* The abort() is just for debugging. Take it out after a while */ #define put_user(x,ptr)\ ({\ int __ret;\ if (access_ok(VERIFY_WRITE, ptr, sizeof(*ptr)))\ __ret = __put_user(x, ptr);\ else\ - __ret = -EFAULT;\ + {__ret = -EFAULT;abort();}\ __ret;\ }) @@ -234,10 +236,16 @@ if (access_ok(VERIFY_READ, ptr, sizeof(*ptr)))\ __ret = __get_user(x, ptr);\ else\ - __ret = -EFAULT;\ + {__ret = -EFAULT;abort();}\ __ret;\ }) +#define copy_from_user(hpptr, gptr, len) \ +({ \ + *(hpptr) = g2h(gptr); \ + access_ok(VERIFY_READ, gptr, len);\ +}) + /* Functions for accessing guest memory. The tget and tput functions read/write single values, byteswapping as neccessary. The lock_user gets a pointer to a contiguous area of guest memory, but does not perform @@ -295,6 +303,15 @@ #define unlock_user_struct(host_ptr, guest_addr, copy) \ unlock_user(host_ptr, guest_addr, (copy) ? sizeof(*host_ptr) : 0) +static inline long lock_and_check_user_struct(void *host_addr, target_ulong guest_addr, long len, int copy, int pg_access) +{ + long *haddr, ret = 0; + haddr = (long *)host_addr; + if( (ret=page_check_range(guest_addr,len,pg_access)) != 0 ) return ret; + *haddr = (long)lock_user(guest_addr, len, copy); + return 0; +} + #define tget8(addr) ldub(addr) #define tput8(addr, val) stb(addr, val) #define tget16(addr) lduw(addr) ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH] linux-user EFAULT implementation 2007-07-10 2:22 ` Stuart Anderson @ 2007-07-10 12:38 ` Paul Brook 2007-07-10 20:45 ` Fabrice Bellard 0 siblings, 1 reply; 13+ messages in thread From: Paul Brook @ 2007-07-10 12:38 UTC (permalink / raw) To: qemu-devel; +Cc: Stuart Anderson On Tuesday 10 July 2007, Stuart Anderson wrote: > On Mon, 9 Jul 2007, Fabrice Bellard wrote: > > No. Ideally you should use the same conventions as the Linux kernel and > > assume that you cannot access the user data directly. > > That's what I had already started doing today. > > > For the time being, I would suggest to minimize the number of changes and > > just extend lock_user()/unlock_user() as you began to do to handle > > -EFAULT. The rest is mostly a question of cosmetics. > > The attached patch is my in-progress work of the complete overhaul to use > the kernel conventions. It needs some more work to finish the > conversion, but enough should be done to see how it is going to turn > out. Overall, I think the converted code is easier to read, especially > if you are familiar with reading kernel code. I also think it will end > up being more correct both becasue of the additional time now spent on > reviewing each syscall, as well as the kernel conventions tend to make > you be more thorough and explicite. > { > struct target_rusage *target_rusage; > > - lock_user_struct(target_rusage, target_addr, 0); > + if( !access_ok(VERIFY_READ,target_addr,sizeof(*target_rusage)) ) > return -1; > + target_rusage = (struct target_rusage *)g2h(target_addr); Using g2h directly is bad. g2h is an implementation detail of one particular memory model. The whole point of the lock_user abstraction (or a similar copy_from_user abstraction) is that almost none of the code cares how "user" memory is accessed. One of the long-term goals of this abstraction is to allow the softmmu code to be used with userspace emulation. In this case a region may be split across multiple discontiguous host pages. The reason I used a locking paradigm rather than a copying one is that it allows a zero-copy implementation in the common case. I've no strong objections to a copying interface, however it must be implementation agnostic. Paul ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH] linux-user EFAULT implementation 2007-07-10 12:38 ` Paul Brook @ 2007-07-10 20:45 ` Fabrice Bellard 2007-07-10 21:09 ` Stuart Anderson 0 siblings, 1 reply; 13+ messages in thread From: Fabrice Bellard @ 2007-07-10 20:45 UTC (permalink / raw) To: qemu-devel Paul Brook wrote: (...] > Using g2h directly is bad. g2h is an implementation detail of one particular > memory model. > > The whole point of the lock_user abstraction (or a similar copy_from_user > abstraction) is that almost none of the code cares how "user" memory is > accessed. One of the long-term goals of this abstraction is to allow the > softmmu code to be used with userspace emulation. In this case a region may > be split across multiple discontiguous host pages. I confirm that I expected g2h() to be completely removed in case the Linux user access API is used. > The reason I used a locking paradigm rather than a copying one is that it > allows a zero-copy implementation in the common case. I've no strong > objections to a copying interface, however it must be implementation > agnostic. Since you agree for the copying interface, I suggest to convert all the code to it. The exact implementation of access_ok(), copy_to_user(), copy_from_user()... will be easily modifiable if one day softmmu user mode is needed. In the only specific case of potentially big unbounded memory areas (such as read/write buffers), a different API must be used where it is possible to get a pointer to user pages (e.g. ptr = lock_user_page(addr, rw_mode); if (!ptr) return -EFAULT; unlock_user_page(ptr)), so that no copy is performed while being able to handle any user page remapping. Regards, Fabrice. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH] linux-user EFAULT implementation 2007-07-10 20:45 ` Fabrice Bellard @ 2007-07-10 21:09 ` Stuart Anderson 2007-07-13 17:55 ` Stuart Anderson 0 siblings, 1 reply; 13+ messages in thread From: Stuart Anderson @ 2007-07-10 21:09 UTC (permalink / raw) To: qemu-devel On Tue, 10 Jul 2007, Fabrice Bellard wrote: > I confirm that I expected g2h() to be completely removed in case the Linux > user access API is used. I agree too that it should not be used. I'm testing ARM on x86_64, and used it in a few places to clean up some warnings. The real fix is to change what I was doing to not need this asignment anyway. I will take another look at how to bury this part in the copy* interfaces. Would it be OK to use g2h() inside the implementation of copy* ? > Since you agree for the copying interface, I suggest to convert all the code > to it. OK, I'll continue in this direction, after taking a fresh look at what I did yesterday. Thanks for the feedback. Stuart Stuart R. Anderson anderson@c2.net Solutions Architect http://www.c2.net/ 1024D/37A79149: 0791 D3B8 9A4C 2CDC A31F BD03 0A62 E534 37A7 9149 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH] linux-user EFAULT implementation 2007-07-10 21:09 ` Stuart Anderson @ 2007-07-13 17:55 ` Stuart Anderson 2007-07-20 20:16 ` Stuart Anderson 0 siblings, 1 reply; 13+ messages in thread From: Stuart Anderson @ 2007-07-13 17:55 UTC (permalink / raw) To: qemu-devel [-- Attachment #1: Type: TEXT/PLAIN, Size: 911 bytes --] On Tue, 10 Jul 2007, Stuart Anderson wrote: > OK, I'll continue in this direction, after taking a fresh look at what I > did yesterday. Attached is a snapshot of the work after a couple more evenings on it. I'm still doing a lot of testing, and I haven't finished converting everything, but there should be enough in this patch to be able to tell if this is heading in a good direction or not. There are even a couple of systems calls actually yield better test suite results as a result of the additional scrutiny of this process. Further comments would be appreciated. Stuart Stuart R. Anderson anderson@netsweng.com Network & Software Engineering http://www.netsweng.com/ 1024D/37A79149: 0791 D3B8 9A4C 2CDC A31F BD03 0A62 E534 37A7 9149 [-- Attachment #2: EFAULT patch --] [-- Type: TEXT/x-diff, Size: 57832 bytes --] Index: qemu/exec.c =================================================================== --- qemu.orig/exec.c 2007-07-13 07:10:58.000000000 -0400 +++ qemu/exec.c 2007-07-13 07:11:12.000000000 -0400 @@ -1862,6 +1862,29 @@ spin_unlock(&tb_lock); } +int page_check_range(target_ulong start, target_ulong len, int flags) +{ + PageDesc *p; + target_ulong end; + target_ulong addr; + + end = TARGET_PAGE_ALIGN(start+len); /* must do before we loose bits in the next step */ + start = start & TARGET_PAGE_MASK; + + if( end < start ) return -1; /* we've wrapped around */ + for(addr = start; addr < end; addr += TARGET_PAGE_SIZE) { + p = page_find(addr >> TARGET_PAGE_BITS); + if( !p ) return -1; + if( !(p->flags & PAGE_VALID) ) return -1; + + if (!(p->flags & PAGE_READ) && + (flags & PAGE_READ) ) return -1; + if (!(p->flags & PAGE_WRITE) && + (flags & PAGE_WRITE) ) return -1; + } + return 0; +} + /* called from signal handler: invalidate the code and unprotect the page. Return TRUE if the fault was succesfully handled. */ int page_unprotect(target_ulong address, unsigned long pc, void *puc) Index: qemu/cpu-all.h =================================================================== --- qemu.orig/cpu-all.h 2007-07-13 07:10:58.000000000 -0400 +++ qemu/cpu-all.h 2007-07-13 07:11:12.000000000 -0400 @@ -691,6 +691,7 @@ int page_get_flags(target_ulong address); void page_set_flags(target_ulong start, target_ulong end, int flags); void page_unprotect_range(target_ulong data, target_ulong data_size); +int page_check_range(target_ulong start, target_ulong len, int flags); CPUState *cpu_copy(CPUState *env); Index: qemu/linux-user/syscall.c =================================================================== --- qemu.orig/linux-user/syscall.c 2007-07-13 07:10:58.000000000 -0400 +++ qemu/linux-user/syscall.c 2007-07-13 07:12:56.000000000 -0400 @@ -404,12 +404,10 @@ #endif } -static inline void host_to_target_rusage(target_ulong target_addr, +static inline long copy_to_user_rusage(struct target_rusage *target_rusage, const struct rusage *rusage) { - struct target_rusage *target_rusage; - - lock_user_struct(target_rusage, target_addr, 0); + if( copy_to_user(target_rusage,rusage,sizeof(*target_rusage)) ) return -1; target_rusage->ru_utime.tv_sec = tswapl(rusage->ru_utime.tv_sec); target_rusage->ru_utime.tv_usec = tswapl(rusage->ru_utime.tv_usec); target_rusage->ru_stime.tv_sec = tswapl(rusage->ru_stime.tv_sec); @@ -428,29 +426,28 @@ target_rusage->ru_nsignals = tswapl(rusage->ru_nsignals); target_rusage->ru_nvcsw = tswapl(rusage->ru_nvcsw); target_rusage->ru_nivcsw = tswapl(rusage->ru_nivcsw); - unlock_user_struct(target_rusage, target_addr, 1); + + return 0; } -static inline void target_to_host_timeval(struct timeval *tv, - target_ulong target_addr) +static inline long copy_from_user_timeval(struct timeval *tv, + struct target_timeval *target_tv) { - struct target_timeval *target_tv; - - lock_user_struct(target_tv, target_addr, 1); + if( copy_from_user(tv,target_tv,sizeof(*target_tv)) ) return -1; tv->tv_sec = tswapl(target_tv->tv_sec); tv->tv_usec = tswapl(target_tv->tv_usec); - unlock_user_struct(target_tv, target_addr, 0); + + return 0; } -static inline void host_to_target_timeval(target_ulong target_addr, +static inline long copy_to_user_timeval(struct target_timeval *target_tv, const struct timeval *tv) { - struct target_timeval *target_tv; - - lock_user_struct(target_tv, target_addr, 0); + if( copy_to_user(target_tv,tv,sizeof(*target_tv)) ) return -1; target_tv->tv_sec = tswapl(tv->tv_sec); target_tv->tv_usec = tswapl(tv->tv_usec); - unlock_user_struct(target_tv, target_addr, 1); + + return 0; } @@ -466,21 +463,21 @@ int ok; if (rfd_p) { - target_rfds = lock_user(rfd_p, sizeof(target_long) * n, 1); + if( lock_and_check_user_struct(&target_rfds,rfd_p,sizeof(target_long)*n,1,PAGE_READ) ) return -EFAULT; rfds_ptr = target_to_host_fds(&rfds, target_rfds, n); } else { target_rfds = NULL; rfds_ptr = NULL; } if (wfd_p) { - target_wfds = lock_user(wfd_p, sizeof(target_long) * n, 1); + if( lock_and_check_user_struct(&target_wfds,wfd_p,sizeof(target_long)*n,1,PAGE_READ) ) return -EFAULT; wfds_ptr = target_to_host_fds(&wfds, target_wfds, n); } else { target_wfds = NULL; wfds_ptr = NULL; } if (efd_p) { - target_efds = lock_user(efd_p, sizeof(target_long) * n, 1); + if( lock_and_check_user_struct(&target_efds,efd_p,sizeof(target_long)*n,1,PAGE_READ) ) return -EFAULT; efds_ptr = target_to_host_fds(&efds, target_efds, n); } else { target_efds = NULL; @@ -488,7 +485,7 @@ } if (target_tv) { - target_to_host_timeval(&tv, target_tv); + if( copy_from_user_timeval(&tv, target_tv) ) return -EFAULT; tv_ptr = &tv; } else { tv_ptr = NULL; @@ -502,7 +499,7 @@ host_to_target_fds(target_efds, efds_ptr, n); if (target_tv) { - host_to_target_timeval(target_tv, &tv); + if( copy_to_user_timeval(target_tv, &tv) ) return -EFAULT; } } if (target_rfds) @@ -515,28 +512,24 @@ return ret; } -static inline void target_to_host_sockaddr(struct sockaddr *addr, - target_ulong target_addr, +static inline long copy_from_user_sockaddr(struct sockaddr *addr, + struct target_sockaddr *target_saddr, socklen_t len) { - struct target_sockaddr *target_saddr; - - target_saddr = lock_user(target_addr, len, 1); - memcpy(addr, target_saddr, len); + if( copy_from_user(addr,target_saddr,len)) return -1; addr->sa_family = tswap16(target_saddr->sa_family); - unlock_user(target_saddr, target_addr, 0); + + return 0; } -static inline void host_to_target_sockaddr(target_ulong target_addr, +static inline long copy_to_user_sockaddr(struct target_sockaddr *target_saddr, struct sockaddr *addr, socklen_t len) { - struct target_sockaddr *target_saddr; - - target_saddr = lock_user(target_addr, len, 0); - memcpy(target_saddr, addr, len); + if( copy_to_user(target_saddr,addr,len) ) return -1; target_saddr->sa_family = tswap16(addr->sa_family); - unlock_user(target_saddr, target_addr, len); + + return 0; } /* ??? Should this also swap msgh->name? */ @@ -628,7 +621,7 @@ msgh->msg_controllen = tswapl(space); } -static long do_setsockopt(int sockfd, int level, int optname, +static long do_setsockopt(int sockfd, int level, int optname, target_ulong optval, socklen_t optlen) { int val, ret; @@ -639,6 +632,8 @@ if (optlen < sizeof(uint32_t)) return -EINVAL; + if( !access_ok(VERIFY_READ, optval, optlen) ) + return -EFAULT; val = tget32(optval); ret = get_errno(setsockopt(sockfd, level, optname, &val, sizeof(val))); break; @@ -660,6 +655,8 @@ case IP_MULTICAST_TTL: case IP_MULTICAST_LOOP: val = 0; + if( !access_ok(VERIFY_READ, optval, optlen) ) + return -EFAULT; if (optlen >= sizeof(uint32_t)) { val = tget32(optval); } else if (optlen >= 1) { @@ -668,7 +665,7 @@ ret = get_errno(setsockopt(sockfd, level, optname, &val, sizeof(val))); break; default: - goto unimplemented; + goto unimplemented; } break; case TARGET_SOL_SOCKET: @@ -732,24 +729,26 @@ break; break; default: - goto unimplemented; + goto unimplemented; } if (optlen < sizeof(uint32_t)) - return -EINVAL; + return -EINVAL; + if( !access_ok(VERIFY_READ, optval, optlen) ) + return -EFAULT; val = tget32(optval); ret = get_errno(setsockopt(sockfd, SOL_SOCKET, optname, &val, sizeof(val))); break; default: unimplemented: - gemu_log("Unsupported setsockopt level=%d optname=%d \n", level, optname); - ret = -ENOSYS; + gemu_log("Unsupported setsockopt level=%d optname=%d\n", level, optname); + ret = -ENOPROTOOPT; } return ret; } -static long do_getsockopt(int sockfd, int level, int optname, - target_ulong optval, target_ulong optlen) +static long do_getsockopt(int sockfd, int level, int optname, + target_ulong optval, socklen_t *optlen) { int len, lv, val, ret; @@ -771,7 +770,10 @@ case SOL_TCP: /* TCP options all take an 'int' value. */ int_case: - len = tget32(optlen); + if( get_user(len,optlen) ) + return -EFAULT; + if( !access_ok(VERIFY_WRITE, optval,len) ) + return -EFAULT; if (len < 0) return -EINVAL; lv = sizeof(int); @@ -804,7 +806,10 @@ #endif case IP_MULTICAST_TTL: case IP_MULTICAST_LOOP: - len = tget32(optlen); + if( get_user(len,optlen) ) + return -EINVAL; + if( !access_ok(VERIFY_WRITE, optval,len) ) + return -EFAULT; if (len < 0) return -EINVAL; lv = sizeof(int); @@ -823,14 +828,15 @@ } break; default: - goto unimplemented; + ret = -ENOPROTOOPT; + break; } break; default: unimplemented: gemu_log("getsockopt level=%d optname=%d not yet supported\n", level, optname); - ret = -ENOSYS; + ret = -EOPNOTSUPP; break; } return ret; @@ -899,7 +905,7 @@ { void *addr = alloca(addrlen); - target_to_host_sockaddr(addr, target_addr, addrlen); + if( copy_from_user_sockaddr(addr, target_addr, addrlen) ) return -EFAULT; return get_errno(bind(sockfd, addr, addrlen)); } @@ -908,7 +914,7 @@ { void *addr = alloca(addrlen); - target_to_host_sockaddr(addr, target_addr, addrlen); + if( copy_from_user_sockaddr(addr, target_addr, addrlen) ) return -EFAULT; return get_errno(connect(sockfd, addr, addrlen)); } @@ -922,12 +928,25 @@ struct iovec *vec; target_ulong target_vec; - lock_user_struct(msgp, target_msg, 1); + if( send ) { + if( lock_and_check_user_struct(&msgp,target_msg,sizeof(*msgp),1, + PAGE_READ) ) return -EFAULT; + } else { + if( lock_and_check_user_struct(&msgp,target_msg,sizeof(*msgp),1, + PAGE_WRITE) ) return -EFAULT; + } if (msgp->msg_name) { msg.msg_namelen = tswap32(msgp->msg_namelen); msg.msg_name = alloca(msg.msg_namelen); - target_to_host_sockaddr(msg.msg_name, tswapl(msgp->msg_name), - msg.msg_namelen); + if( send ) { + if( copy_from_user_sockaddr(msg.msg_name, tswapl(msgp->msg_name), + msg.msg_namelen) ) return -EFAULT; + } else { + /* FIXME + if( (ret=copy_from_user_sockaddr(msg.msg_name, tswapl(msgp->msg_name), + msg.msg_namelen,PAGE_WRITE)) != 0 ) return ret; + */ + } } else { msg.msg_name = NULL; msg.msg_namelen = 0; @@ -956,46 +975,55 @@ } static long do_accept(int fd, target_ulong target_addr, - target_ulong target_addrlen) + socklen_t *target_addrlen) { - socklen_t addrlen = tget32(target_addrlen); - void *addr = alloca(addrlen); + socklen_t addrlen; + void *addr; long ret; + if( get_user(addrlen,target_addrlen) ) return -EINVAL; + addr = alloca(addrlen); + ret = get_errno(accept(fd, addr, &addrlen)); if (!is_error(ret)) { - host_to_target_sockaddr(target_addr, addr, addrlen); + if( copy_to_user_sockaddr(target_addr, addr, addrlen) ) return -EFAULT; tput32(target_addrlen, addrlen); } return ret; } static long do_getpeername(int fd, target_ulong target_addr, - target_ulong target_addrlen) + socklen_t *target_addrlen) { - socklen_t addrlen = tget32(target_addrlen); - void *addr = alloca(addrlen); + socklen_t addrlen; + void *addr; long ret; + if( get_user(addrlen,target_addrlen) ) return -EFAULT; + addr = alloca(addrlen); + ret = get_errno(getpeername(fd, addr, &addrlen)); if (!is_error(ret)) { - host_to_target_sockaddr(target_addr, addr, addrlen); - tput32(target_addrlen, addrlen); + if( copy_to_user_sockaddr(target_addr, addr, addrlen) ) return -EFAULT; + tput32(target_addrlen,addrlen); } return ret; } static long do_getsockname(int fd, target_ulong target_addr, - target_ulong target_addrlen) + socklen_t *target_addrlen) { - socklen_t addrlen = tget32(target_addrlen); - void *addr = alloca(addrlen); + socklen_t addrlen; + void *addr; long ret; + if( get_user(addrlen,target_addrlen) ) return -EFAULT; + addr = alloca(addrlen); + ret = get_errno(getsockname(fd, addr, &addrlen)); if (!is_error(ret)) { - host_to_target_sockaddr(target_addr, addr, addrlen); - tput32(target_addrlen, addrlen); + if( copy_to_user_sockaddr(target_addr, addr, addrlen) ) return -EFAULT; + tput32(target_addrlen,addrlen); } return ret; } @@ -1006,6 +1034,8 @@ int tab[2]; long ret; + if( !access_ok(VERIFY_WRITE,target_tab,2*sizeof(int)) ) + return -EFAULT; ret = get_errno(socketpair(domain, type, protocol, tab)); if (!is_error(ret)) { tput32(target_tab, tab[0]); @@ -1018,18 +1048,17 @@ target_ulong target_addr, socklen_t addrlen) { void *addr; - void *host_msg; + void *host_msg = alloca(len); long ret; - host_msg = lock_user(msg, len, 1); + if (copy_from_user(host_msg,msg,len) ) return -EFAULT; if (target_addr) { addr = alloca(addrlen); - target_to_host_sockaddr(addr, target_addr, addrlen); + if (copy_from_user_sockaddr(addr, target_addr, addrlen) ) return -EFAULT; ret = get_errno(sendto(fd, host_msg, len, flags, addr, addrlen)); } else { ret = get_errno(send(fd, host_msg, len, flags)); } - unlock_user(host_msg, msg, 0); return ret; } @@ -1038,10 +1067,9 @@ { socklen_t addrlen; void *addr; - void *host_msg; + void *host_msg = alloca(len); long ret; - host_msg = lock_user(msg, len, 0); if (target_addr) { addrlen = tget32(target_addrlen); addr = alloca(addrlen); @@ -1052,12 +1080,10 @@ } if (!is_error(ret)) { if (target_addr) { - host_to_target_sockaddr(target_addr, addr, addrlen); + if (copy_to_user_sockaddr(target_addr, addr, addrlen) ) return -EFAULT; tput32(target_addrlen, addrlen); } - unlock_user(host_msg, msg, len); - } else { - unlock_user(host_msg, msg, 0); + if( copy_to_user(msg, host_msg, len) ) return -EFAULT; } return ret; } @@ -1219,7 +1245,7 @@ break; default: gemu_log("Unsupported socketcall: %d\n", num); - ret = -ENOSYS; + ret = -EINVAL; break; } return ret; @@ -1259,64 +1285,44 @@ target_ulong __unused4; }; -static inline void target_to_host_ipc_perm(struct ipc_perm *host_ip, - target_ulong target_addr) +static inline void copy_from_user_ipc_perm(struct ipc_perm *host_ip, + struct target_ipc_perm *target_ip) { - struct target_ipc_perm *target_ip; - struct target_semid_ds *target_sd; - - lock_user_struct(target_sd, target_addr, 1); - target_ip=&(target_sd->sem_perm); host_ip->__key = tswapl(target_ip->__key); host_ip->uid = tswapl(target_ip->uid); host_ip->gid = tswapl(target_ip->gid); host_ip->cuid = tswapl(target_ip->cuid); host_ip->cgid = tswapl(target_ip->cgid); host_ip->mode = tswapl(target_ip->mode); - unlock_user_struct(target_sd, target_addr, 0); } -static inline void host_to_target_ipc_perm(target_ulong target_addr, +static inline void copy_to_user_ipc_perm(struct target_ipc_perm *target_ip, struct ipc_perm *host_ip) { - struct target_ipc_perm *target_ip; - struct target_semid_ds *target_sd; - - lock_user_struct(target_sd, target_addr, 0); - target_ip = &(target_sd->sem_perm); target_ip->__key = tswapl(host_ip->__key); target_ip->uid = tswapl(host_ip->uid); target_ip->gid = tswapl(host_ip->gid); target_ip->cuid = tswapl(host_ip->cuid); target_ip->cgid = tswapl(host_ip->cgid); target_ip->mode = tswapl(host_ip->mode); - unlock_user_struct(target_sd, target_addr, 1); } -static inline void target_to_host_semid_ds(struct semid_ds *host_sd, - target_ulong target_addr) +static inline void copy_from_user_semid_ds(struct semid_ds *host_sd, + struct target_semid_ds *target_sd) { - struct target_semid_ds *target_sd; - - lock_user_struct(target_sd, target_addr, 1); - target_to_host_ipc_perm(&(host_sd->sem_perm),target_addr); + copy_from_user_ipc_perm(&(host_sd->sem_perm),&(target_sd->sem_perm)); 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); - unlock_user_struct(target_sd, target_addr, 0); } -static inline void host_to_target_semid_ds(target_ulong target_addr, +static inline void copy_to_user_semid_ds(struct target_semid_ds *target_sd, struct semid_ds *host_sd) { - struct target_semid_ds *target_sd; - - lock_user_struct(target_sd, target_addr, 0); - host_to_target_ipc_perm(target_addr,&(host_sd->sem_perm)); + copy_to_user_ipc_perm(&(target_sd->sem_perm),&(host_sd->sem_perm)); 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); - unlock_user_struct(target_sd, target_addr, 1); } union semun { @@ -1328,109 +1334,122 @@ union target_semun { int val; target_long buf; - unsigned short int *array; + target_long array; }; -static inline void target_to_host_semun(unsigned long cmd, +/* static inline*/ long copy_from_user_semun(unsigned long cmd, union semun *host_su, - target_ulong target_addr, + union target_semun *target_su, struct semid_ds *ds) { - union target_semun *target_su; - switch( cmd ) { case IPC_STAT: case IPC_SET: - lock_user_struct(target_su, target_addr, 1); - target_to_host_semid_ds(ds,target_su->buf); + if( copy_from_user(ds,target_su->buf,sizeof(struct semid_ds)) ) return -1; + copy_from_user_semid_ds(ds,target_su->buf); host_su->buf = ds; - unlock_user_struct(target_su, target_addr, 0); break; case GETVAL: case SETVAL: - lock_user_struct(target_su, target_addr, 1); - host_su->val = tswapl(target_su->val); - unlock_user_struct(target_su, target_addr, 0); + if (get_user(host_su->val,&(target_su->val))) return -1; break; case GETALL: + host_su->array=(unsigned short *)ds; + break; case SETALL: - lock_user_struct(target_su, target_addr, 1); + host_su->array=(unsigned short *)ds; + if (copy_from_user(host_su->array,target_su->array,sizeof(unsigned short))) return -1; +/* + if (get_user(host_su->array[0],target_su->array)) return -1; + if (lock_and_check_user_struct(&target_su,,sizeof(*target_su),1,pg_access) ) return -1; *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); } + + return 0; } -static inline void host_to_target_semun(unsigned long cmd, - target_ulong target_addr, +/* static inline */ long copy_to_user_semun(unsigned long cmd, + union target_semun *target_su, union semun *host_su, struct semid_ds *ds) { - union target_semun *target_su; - switch( cmd ) { case IPC_STAT: case IPC_SET: - lock_user_struct(target_su, target_addr, 0); - host_to_target_semid_ds(target_su->buf,ds); - unlock_user_struct(target_su, target_addr, 1); + if( copy_to_user(target_su->buf,ds,sizeof(struct semid_ds)) ) return -1; + copy_to_user_semid_ds(target_su->buf,ds); break; case GETVAL: case SETVAL: - lock_user_struct(target_su, target_addr, 0); - target_su->val = tswapl(host_su->val); - unlock_user_struct(target_su, target_addr, 1); + if (put_user(host_su->val,&(target_su->val))) return -1; break; case GETALL: + if( copy_to_user(target_su->array,host_su->array,sizeof(unsigned short)) ) return -1; +/* + if (put_user(target_su->array[0],host_su->array)) return -1; + *target_su->array = tswap16(*host_su->array); +*/ + break; case SETALL: - lock_user_struct(target_su, target_addr, 0); +/* + if (put_user(*host_su->array,target_su->array)) return -1; + if (lock_and_check_user_struct(&target_su,target_addr,sizeof(*target_su),0,pg_access) ) return -1; *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); } + + return 0; } -static inline long do_semctl(long first, long second, long third, long ptr) +/* static inline */ long do_semctl(long first, long second, long third, long ptr) { union semun arg; struct semid_ds dsarg; int cmd = third&0xff; long ret = 0; + if( copy_from_user(&arg,ptr,sizeof(union semun)) ) return -EFAULT; switch( cmd ) { case GETVAL: - target_to_host_semun(cmd,&arg,ptr,&dsarg); + if( copy_from_user_semun(cmd,&arg,ptr,&dsarg) ) return -EFAULT; ret = get_errno(semctl(first, second, cmd, arg)); - host_to_target_semun(cmd,ptr,&arg,&dsarg); + if( copy_to_user_semun(cmd,ptr,&arg,&dsarg) ) return -EFAULT; break; case SETVAL: - target_to_host_semun(cmd,&arg,ptr,&dsarg); + if( copy_from_user_semun(cmd,&arg,ptr,&dsarg) ) return -EFAULT; ret = get_errno(semctl(first, second, cmd, arg)); - host_to_target_semun(cmd,ptr,&arg,&dsarg); + if( copy_to_user_semun(cmd,ptr,&arg,&dsarg) ) return -EFAULT; break; case GETALL: - target_to_host_semun(cmd,&arg,ptr,&dsarg); + if( copy_from_user_semun(cmd,&arg,ptr,&dsarg) ) return -EFAULT; ret = get_errno(semctl(first, second, cmd, arg)); - host_to_target_semun(cmd,ptr,&arg,&dsarg); + if (!is_error(ret)) { + if( copy_to_user_semun(cmd,ptr,&arg,&dsarg) ) return -EFAULT; + } break; case SETALL: - target_to_host_semun(cmd,&arg,ptr,&dsarg); + if( copy_from_user_semun(cmd,&arg,ptr,&dsarg) ) return -EFAULT; ret = get_errno(semctl(first, second, cmd, arg)); - host_to_target_semun(cmd,ptr,&arg,&dsarg); + if( copy_to_user_semun(cmd,ptr,&arg,&dsarg) ) return -EFAULT; break; case IPC_STAT: - target_to_host_semun(cmd,&arg,ptr,&dsarg); + if( copy_from_user_semun(cmd,&arg,ptr,&dsarg) ) return -EFAULT; ret = get_errno(semctl(first, second, cmd, arg)); - host_to_target_semun(cmd,ptr,&arg,&dsarg); + if( copy_to_user_semun(cmd,ptr,&arg,&dsarg) ) return -EFAULT; break; case IPC_SET: - target_to_host_semun(cmd,&arg,ptr,&dsarg); + if( copy_from_user_semun(cmd,&arg,ptr,&dsarg) ) return -EFAULT; ret = get_errno(semctl(first, second, cmd, arg)); - host_to_target_semun(cmd,ptr,&arg,&dsarg); + if( copy_to_user_semun(cmd,ptr,&arg,&dsarg) ) return -EFAULT; break; default: ret = get_errno(semctl(first, second, cmd, arg)); @@ -1457,13 +1476,10 @@ target_ulong __unused5; }; -static inline void target_to_host_msqid_ds(struct msqid_ds *host_md, - target_ulong target_addr) +static inline long copy_from_user_msqid_ds(struct msqid_ds *host_md, + struct target_msqid_ds *target_md) { - struct target_msqid_ds *target_md; - - lock_user_struct(target_md, target_addr, 1); - target_to_host_ipc_perm(&(host_md->msg_perm),target_addr); + copy_from_user_ipc_perm(&(host_md->msg_perm),&(target_md->msg_perm)); host_md->msg_stime = tswapl(target_md->msg_stime); host_md->msg_rtime = tswapl(target_md->msg_rtime); host_md->msg_ctime = tswapl(target_md->msg_ctime); @@ -1472,16 +1488,14 @@ host_md->msg_qbytes = tswapl(target_md->msg_qbytes); host_md->msg_lspid = tswapl(target_md->msg_lspid); host_md->msg_lrpid = tswapl(target_md->msg_lrpid); - unlock_user_struct(target_md, target_addr, 0); + + return 0; } -static inline void host_to_target_msqid_ds(target_ulong target_addr, - struct msqid_ds *host_md) +static inline long copy_to_usermsqid_ds(struct target_msqid_ds *target_md, + struct msqid_ds *host_md) { - struct target_msqid_ds *target_md; - - lock_user_struct(target_md, target_addr, 0); - host_to_target_ipc_perm(target_addr,&(host_md->msg_perm)); + copy_to_user_ipc_perm(&(target_md->msg_perm),&(host_md->msg_perm)); target_md->msg_stime = tswapl(host_md->msg_stime); target_md->msg_rtime = tswapl(host_md->msg_rtime); target_md->msg_ctime = tswapl(host_md->msg_ctime); @@ -1490,7 +1504,8 @@ target_md->msg_qbytes = tswapl(host_md->msg_qbytes); target_md->msg_lspid = tswapl(host_md->msg_lspid); target_md->msg_lrpid = tswapl(host_md->msg_lrpid); - unlock_user_struct(target_md, target_addr, 1); + + return 0; } static inline long do_msgctl(long first, long second, long ptr) @@ -1500,10 +1515,17 @@ long ret = 0; switch( cmd ) { case IPC_STAT: + if( copy_from_user(&dsarg,ptr,sizeof(struct msqid_ds)) ) return -EFAULT; + copy_from_user_msqid_ds(&dsarg,ptr); + ret = get_errno(msgctl(first, cmd, &dsarg)); + if( copy_to_usermsqid_ds(ptr,&dsarg) ) return -EFAULT; + break; case IPC_SET: - target_to_host_msqid_ds(&dsarg,ptr); + if( copy_from_user(&dsarg,ptr,sizeof(struct msqid_ds)) ) return -EFAULT; + copy_from_user_msqid_ds(&dsarg,ptr); ret = get_errno(msgctl(first, cmd, &dsarg)); - host_to_target_msqid_ds(ptr,&dsarg); + if( copy_to_usermsqid_ds(ptr,&dsarg) ) return -EFAULT; + break; default: ret = get_errno(msgctl(first, cmd, &dsarg)); } @@ -1515,19 +1537,20 @@ char mtext[1]; }; -static inline long do_msgsnd(long msqid, long msgp, long msgsz, long msgflg) +/* static inline */ long do_msgsnd(long msqid, long msgp, long msgsz, long msgflg) { - struct target_msgbuf *target_mb; - struct msgbuf *host_mb; + struct target_msgbuf *target_mb = (struct msgbuf *)msgp; + struct msgbuf *host_mb = alloca(sizeof(long)+msgsz); long ret = 0; - lock_user_struct(target_mb,msgp,0); + if( copy_from_user(host_mb,target_mb,sizeof(long)+msgsz) ) return -EFAULT; +/* host_mb = malloc(msgsz+sizeof(long)); - host_mb->mtype = tswapl(target_mb->mtype); memcpy(host_mb->mtext,target_mb->mtext,msgsz); - ret = get_errno(msgsnd(msqid, host_mb, msgsz, msgflg)); free(host_mb); - unlock_user_struct(target_mb, msgp, 0); +*/ + host_mb->mtype = tswapl(target_mb->mtype); + ret = get_errno(msgsnd(msqid, host_mb, msgsz, msgflg)); return ret; } @@ -1538,7 +1561,7 @@ struct msgbuf *host_mb; long ret = 0; - lock_user_struct(target_mb, msgp, 0); + if( lock_and_check_user_struct(&target_mb,msgp,sizeof(long)+msgsz,0,PAGE_WRITE) ) return -EFAULT; host_mb = malloc(msgsz+sizeof(long)); ret = get_errno(msgrcv(msqid, host_mb, msgsz, 1, msgflg)); if (ret > 0) @@ -2018,6 +2041,7 @@ static int write_ldt(CPUX86State *env, target_ulong ptr, unsigned long bytecount, int oldmode) { + long ret = 0; struct target_modify_ldt_ldt_s ldt_info; struct target_modify_ldt_ldt_s *target_ldt_info; int seg_32bit, contents, read_exec_only, limit_in_pages; @@ -2026,7 +2050,7 @@ if (bytecount != sizeof(ldt_info)) return -EINVAL; - lock_user_struct(target_ldt_info, ptr, 1); + if( lock_and_check_user_struct(&target_ldt_info,ptr,sizeof(struct target_modify_ldt_ldt_s),1,PAGE_READ) ) return -EFAULT; ldt_info.entry_number = tswap32(target_ldt_info->entry_number); ldt_info.base_addr = tswapl(target_ldt_info->base_addr); ldt_info.limit = tswap32(target_ldt_info->limit); @@ -2225,7 +2249,7 @@ switch(cmd) { case TARGET_F_GETLK: - lock_user_struct(target_fl, arg, 1); + if( lock_and_check_user_struct(&target_fl,arg,sizeof(struct target_flock),1,PAGE_READ) ) return -EFAULT; fl.l_type = tswap16(target_fl->l_type); fl.l_whence = tswap16(target_fl->l_whence); fl.l_start = tswapl(target_fl->l_start); @@ -2234,6 +2258,7 @@ unlock_user_struct(target_fl, arg, 0); ret = fcntl(fd, cmd, &fl); if (ret == 0) { + if( lock_and_check_user_struct(&target_fl,arg,sizeof(struct target_flock),0,PAGE_WRITE) ) return -EFAULT; lock_user_struct(target_fl, arg, 0); target_fl->l_type = tswap16(fl.l_type); target_fl->l_whence = tswap16(fl.l_whence); @@ -2246,7 +2271,7 @@ case TARGET_F_SETLK: case TARGET_F_SETLKW: - lock_user_struct(target_fl, arg, 1); + if( lock_and_check_user_struct(&target_fl,arg,sizeof(struct target_flock),1,PAGE_READ) ) return -EFAULT; fl.l_type = tswap16(target_fl->l_type); fl.l_whence = tswap16(target_fl->l_whence); fl.l_start = tswapl(target_fl->l_start); @@ -2257,7 +2282,7 @@ break; case TARGET_F_GETLK64: - lock_user_struct(target_fl64, arg, 1); + if( lock_and_check_user_struct(&target_fl64,arg,sizeof(struct target_flock64),1,PAGE_READ) ) return -EFAULT; fl64.l_type = tswap16(target_fl64->l_type) >> 1; fl64.l_whence = tswap16(target_fl64->l_whence); fl64.l_start = tswapl(target_fl64->l_start); @@ -2266,7 +2291,7 @@ unlock_user_struct(target_fl64, arg, 0); ret = fcntl(fd, cmd >> 1, &fl64); if (ret == 0) { - lock_user_struct(target_fl64, arg, 0); + if( lock_and_check_user_struct(&target_fl64,arg,sizeof(struct target_flock64),0,PAGE_WRITE) ) return -EFAULT; target_fl64->l_type = tswap16(fl64.l_type) >> 1; target_fl64->l_whence = tswap16(fl64.l_whence); target_fl64->l_start = tswapl(fl64.l_start); @@ -2277,7 +2302,7 @@ break; case TARGET_F_SETLK64: case TARGET_F_SETLKW64: - lock_user_struct(target_fl64, arg, 1); + if( lock_and_check_user_struct(&target_fl64,arg,sizeof(struct target_flock64),1,PAGE_READ) ) return -EFAULT; fl64.l_type = tswap16(target_fl64->l_type) >> 1; fl64.l_whence = tswap16(target_fl64->l_whence); fl64.l_start = tswapl(target_fl64->l_start); @@ -2419,32 +2444,30 @@ } #endif -static inline void target_to_host_timespec(struct timespec *host_ts, - target_ulong target_addr) +static inline long copy_from_user_timespec(struct timespec *host_ts, + struct target_timespec *target_ts) { - struct target_timespec *target_ts; - - lock_user_struct(target_ts, target_addr, 1); + if( copy_from_user(host_ts,target_ts,sizeof(*target_ts)) ) return -1; host_ts->tv_sec = tswapl(target_ts->tv_sec); host_ts->tv_nsec = tswapl(target_ts->tv_nsec); - unlock_user_struct(target_ts, target_addr, 0); + + return 0; } -static inline void host_to_target_timespec(target_ulong target_addr, +static inline long copy_to_user_timespec(struct target_timespec *target_ts, struct timespec *host_ts) { - struct target_timespec *target_ts; - - lock_user_struct(target_ts, target_addr, 0); + if( copy_to_user(target_ts, host_ts, sizeof(*target_ts)) ) return -1; target_ts->tv_sec = tswapl(host_ts->tv_sec); target_ts->tv_nsec = tswapl(host_ts->tv_nsec); - unlock_user_struct(target_ts, target_addr, 1); + + return 0; } long do_syscall(void *cpu_env, int num, long arg1, long arg2, long arg3, long arg4, long arg5, long arg6) { - long ret; + long ret = 0; struct stat st; struct statfs stfs; void *p; @@ -2671,7 +2694,7 @@ struct utimbuf tbuf, *host_tbuf; struct target_utimbuf *target_tbuf; if (arg2) { - lock_user_struct(target_tbuf, arg2, 1); + if( lock_and_check_user_struct(&target_tbuf,arg2,sizeof(struct target_utimbuf),1,PAGE_READ) ) return -EFAULT; tbuf.actime = tswapl(target_tbuf->actime); tbuf.modtime = tswapl(target_tbuf->modtime); unlock_user_struct(target_tbuf, arg2, 0); @@ -2689,9 +2712,11 @@ { struct timeval *tvp, tv[2]; if (arg2) { - target_to_host_timeval(&tv[0], arg2); - target_to_host_timeval(&tv[1], - arg2 + sizeof (struct target_timeval)); + if( copy_from_user_timeval(&tv[0], arg2) ) + return -EFAULT; + if( copy_from_user_timeval(&tv[1], + arg2 + sizeof (struct target_timeval)) ) + return -EFAULT; tvp = tv; } else { tvp = NULL; @@ -2860,7 +2885,7 @@ struct target_old_sigaction *old_act; struct target_sigaction act, oact, *pact; if (arg2) { - lock_user_struct(old_act, arg2, 1); + if( lock_and_check_user_struct(&old_act,arg2,sizeof(*old_act),1,PAGE_READ) ) return -EFAULT; act._sa_handler = old_act->_sa_handler; target_siginitset(&act.sa_mask, old_act->sa_mask); act.sa_flags = old_act->sa_flags; @@ -2872,7 +2897,7 @@ } ret = get_errno(do_sigaction(arg1, pact, &oact)); if (!is_error(ret) && arg3) { - lock_user_struct(old_act, arg3, 0); + if( lock_and_check_user_struct(&old_act,arg3,sizeof(*old_act),0,PAGE_WRITE) ) return -EFAULT; old_act->_sa_handler = oact._sa_handler; old_act->sa_mask = oact.sa_mask.sig[0]; old_act->sa_flags = oact.sa_flags; @@ -2883,7 +2908,7 @@ struct target_sigaction act, oact, *pact, *old_act; if (arg2) { - lock_user_struct(old_act, arg2, 1); + if( lock_and_check_user_struct(&old_act,arg2,sizeof(*old_act),1,PAGE_READ) ) return -EFAULT; act._sa_handler = old_act->_sa_handler; target_siginitset(&act.sa_mask, old_act->sa_mask.sig[0]); act.sa_flags = old_act->sa_flags; @@ -2896,7 +2921,7 @@ ret = get_errno(do_sigaction(arg1, pact, &oact)); if (!is_error(ret) && arg3) { - lock_user_struct(old_act, arg3, 0); + if( lock_and_check_user_struct(&old_act,arg3,sizeof(*old_act),0,PAGE_WRITE) ) return -EFAULT; old_act->_sa_handler = oact._sa_handler; old_act->sa_flags = oact.sa_flags; old_act->sa_mask.sig[0] = oact.sa_mask.sig[0]; @@ -2914,12 +2939,14 @@ struct target_sigaction *act; struct target_sigaction *oact; - if (arg2) - lock_user_struct(act, arg2, 1); + if (arg2) { + if( lock_and_check_user_struct(&act,arg2,sizeof(*act),1,PAGE_READ) ) return -EFAULT; + } else act = NULL; - if (arg3) - lock_user_struct(oact, arg3, 0); + if (arg3) { + if( lock_and_check_user_struct(&oact,arg2,sizeof(*oact),0,PAGE_WRITE) ) return -EFAULT; + } else oact = NULL; ret = get_errno(do_sigaction(arg1, act, oact)); @@ -3046,7 +3073,7 @@ sigset_t set; ret = get_errno(sigpending(&set)); if (!is_error(ret)) { - p = lock_user(arg1, sizeof(target_sigset_t), 0); + if( lock_and_check_user_struct(&p,arg1,sizeof(target_sigset_t),0,PAGE_WRITE) ) return -EFAULT; host_to_target_sigset(p, &set); unlock_user(p, arg1, sizeof(target_sigset_t)); } @@ -3056,7 +3083,7 @@ case TARGET_NR_sigsuspend: { sigset_t set; - p = lock_user(arg1, sizeof(target_sigset_t), 1); + if( lock_and_check_user_struct(&p,arg1,sizeof(target_sigset_t),1,PAGE_READ) ) return -EFAULT; target_to_host_old_sigset(&set, p); unlock_user(p, arg1, 0); ret = get_errno(sigsuspend(&set)); @@ -3066,7 +3093,7 @@ case TARGET_NR_rt_sigsuspend: { sigset_t set; - p = lock_user(arg1, sizeof(target_sigset_t), 1); + if( lock_and_check_user_struct(&p,arg1,sizeof(target_sigset_t),1,PAGE_READ) ) return -EFAULT; target_to_host_sigset(&set, p); unlock_user(p, arg1, 0); ret = get_errno(sigsuspend(&set)); @@ -3078,18 +3105,18 @@ struct timespec uts, *puts; siginfo_t uinfo; - p = lock_user(arg1, sizeof(target_sigset_t), 1); + if( lock_and_check_user_struct(&p,arg1,sizeof(target_sigset_t),1,PAGE_READ) ) return -EFAULT; target_to_host_sigset(&set, p); unlock_user(p, arg1, 0); if (arg3) { puts = &uts; - target_to_host_timespec(puts, arg3); + if( copy_from_user_timespec(puts, arg3) ) return -EFAULT; } else { puts = NULL; } ret = get_errno(sigtimedwait(&set, &uinfo, puts)); if (!is_error(ret) && arg2) { - p = lock_user(arg2, sizeof(target_sigset_t), 0); + if( lock_and_check_user_struct(&p,arg2,sizeof(target_sigset_t),0,PAGE_WRITE) ) return -EFAULT; host_to_target_siginfo(p, &uinfo); unlock_user(p, arg2, sizeof(target_sigset_t)); } @@ -3098,7 +3125,7 @@ case TARGET_NR_rt_sigqueueinfo: { siginfo_t uinfo; - p = lock_user(arg3, sizeof(target_sigset_t), 1); + if( lock_and_check_user_struct(&p,arg3,sizeof(target_sigset_t),1,PAGE_READ) ) return -EFAULT; target_to_host_siginfo(&uinfo, p); unlock_user(p, arg1, 0); ret = get_errno(sys_rt_sigqueueinfo(arg1, arg2, &uinfo)); @@ -3153,7 +3180,7 @@ struct rusage rusage; ret = get_errno(getrusage(arg1, &rusage)); if (!is_error(ret)) { - host_to_target_rusage(arg2, &rusage); + if( copy_to_user_rusage(arg2, &rusage) ) return -EFAULT; } } break; @@ -3162,14 +3189,14 @@ struct timeval tv; ret = get_errno(gettimeofday(&tv, NULL)); if (!is_error(ret)) { - host_to_target_timeval(arg1, &tv); + if( copy_to_user_timeval(arg1, &tv) ) return -EFAULT; } } break; case TARGET_NR_settimeofday: { struct timeval tv; - target_to_host_timeval(&tv, arg1); + if( copy_from_user_timeval(&tv, arg1) ) return -EFAULT; ret = get_errno(settimeofday(&tv, NULL)); } break; @@ -3335,20 +3362,19 @@ unlock_user(p, arg1, 0); convert_statfs: if (!is_error(ret)) { - struct target_statfs *target_stfs; + struct target_statfs *target_stfs = (struct target_statfs *)arg2; - lock_user_struct(target_stfs, arg2, 0); - /* ??? put_user is probably wrong. */ - put_user(stfs.f_type, &target_stfs->f_type); - put_user(stfs.f_bsize, &target_stfs->f_bsize); - put_user(stfs.f_blocks, &target_stfs->f_blocks); - put_user(stfs.f_bfree, &target_stfs->f_bfree); - put_user(stfs.f_bavail, &target_stfs->f_bavail); - put_user(stfs.f_files, &target_stfs->f_files); - put_user(stfs.f_ffree, &target_stfs->f_ffree); - put_user(stfs.f_fsid.__val[0], &target_stfs->f_fsid); - put_user(stfs.f_namelen, &target_stfs->f_namelen); - unlock_user_struct(target_stfs, arg2, 1); + if( !access_ok(VERIFY_WRITE,target_stfs,sizeof(struct target_statfs)) ) return -EFAULT; + /* use __put_user since we just validated the entire bufffer. */ + __put_user(stfs.f_type, &target_stfs->f_type); + __put_user(stfs.f_bsize, &target_stfs->f_bsize); + __put_user(stfs.f_blocks, &target_stfs->f_blocks); + __put_user(stfs.f_bfree, &target_stfs->f_bfree); + __put_user(stfs.f_bavail, &target_stfs->f_bavail); + __put_user(stfs.f_files, &target_stfs->f_files); + __put_user(stfs.f_ffree, &target_stfs->f_ffree); + __put_user(stfs.f_fsid.__val[0], &target_stfs->f_fsid); + __put_user(stfs.f_namelen, &target_stfs->f_namelen); } break; case TARGET_NR_fstatfs: @@ -3361,20 +3387,19 @@ unlock_user(p, arg1, 0); convert_statfs64: if (!is_error(ret)) { - struct target_statfs64 *target_stfs; + struct target_statfs64 *target_stfs = (struct target_statfs64 *)arg3; - lock_user_struct(target_stfs, arg3, 0); - /* ??? put_user is probably wrong. */ - put_user(stfs.f_type, &target_stfs->f_type); - put_user(stfs.f_bsize, &target_stfs->f_bsize); - put_user(stfs.f_blocks, &target_stfs->f_blocks); - put_user(stfs.f_bfree, &target_stfs->f_bfree); - put_user(stfs.f_bavail, &target_stfs->f_bavail); - put_user(stfs.f_files, &target_stfs->f_files); - put_user(stfs.f_ffree, &target_stfs->f_ffree); - put_user(stfs.f_fsid.__val[0], &target_stfs->f_fsid); - put_user(stfs.f_namelen, &target_stfs->f_namelen); - unlock_user_struct(target_stfs, arg3, 0); + if( !access_ok(VERIFY_WRITE,target_stfs,sizeof(struct target_statfs64)) ) return -EFAULT; + /* use __put_user since we just validated the entire bufffer. */ + __put_user(stfs.f_type, &target_stfs->f_type); + __put_user(stfs.f_bsize, &target_stfs->f_bsize); + __put_user(stfs.f_blocks, &target_stfs->f_blocks); + __put_user(stfs.f_bfree, &target_stfs->f_bfree); + __put_user(stfs.f_bavail, &target_stfs->f_bavail); + __put_user(stfs.f_files, &target_stfs->f_files); + __put_user(stfs.f_ffree, &target_stfs->f_ffree); + __put_user(stfs.f_fsid.__val[0], &target_stfs->f_fsid); + __put_user(stfs.f_namelen, &target_stfs->f_namelen); } break; case TARGET_NR_fstatfs64: @@ -3488,19 +3513,20 @@ if (arg2) { pvalue = &value; - target_to_host_timeval(&pvalue->it_interval, - arg2); - target_to_host_timeval(&pvalue->it_value, - arg2 + sizeof(struct target_timeval)); + if( copy_from_user_timeval(&pvalue->it_interval, arg2) ) + return -EFAULT; + if( copy_from_user_timeval(&pvalue->it_value, + arg2 + sizeof(struct target_timeval)) ) + return -EFAULT; } else { pvalue = NULL; } ret = get_errno(setitimer(arg1, pvalue, &ovalue)); if (!is_error(ret) && arg3) { - host_to_target_timeval(arg3, - &ovalue.it_interval); - host_to_target_timeval(arg3 + sizeof(struct target_timeval), - &ovalue.it_value); + if( copy_to_user_timeval(arg3, + &ovalue.it_interval) ) return -EFAULT; + if( copy_to_user_timeval(arg3 + sizeof(struct target_timeval), + &ovalue.it_value) ) return -EFAULT; } } break; @@ -3510,10 +3536,10 @@ ret = get_errno(getitimer(arg1, &value)); if (!is_error(ret) && arg2) { - host_to_target_timeval(arg2, - &value.it_interval); - host_to_target_timeval(arg2 + sizeof(struct target_timeval), - &value.it_value); + if( copy_to_user_timeval(arg2, + &value.it_interval) ) return -EFAULT; + if( copy_to_user_timeval(arg2 + sizeof(struct target_timeval), + &value.it_value) ) return -EFAULT; } } break; @@ -3534,7 +3560,7 @@ if (!is_error(ret)) { struct target_stat *target_st; - lock_user_struct(target_st, arg2, 0); + if( lock_and_check_user_struct(&target_st,arg2,sizeof(*target_st),9,PAGE_WRITE) ) return -EFAULT; #if defined(TARGET_MIPS) target_st->st_dev = tswap32(st.st_dev); #else @@ -3603,7 +3629,8 @@ if (status_ptr) tputl(status_ptr, status); if (target_rusage) { - host_to_target_rusage(target_rusage, &rusage); + if( !copy_to_user_rusage(target_rusage, &rusage) ) + return -EFAULT; } } } @@ -3965,17 +3992,17 @@ struct timespec ts; ret = get_errno(sched_rr_get_interval(arg1, &ts)); if (!is_error(ret)) { - host_to_target_timespec(arg2, &ts); + if( copy_to_user_timespec(arg2, &ts) ) return -EFAULT; } } break; case TARGET_NR_nanosleep: { struct timespec req, rem; - target_to_host_timespec(&req, arg1); + if( copy_from_user_timespec(&req, arg1) ) return -EFAULT; ret = get_errno(nanosleep(&req, &rem)); if (is_error(ret) && arg2) { - host_to_target_timespec(arg2, &rem); + if( copy_to_user_timespec(arg2, &rem) ) return -EFAULT; } } break; @@ -4072,6 +4099,7 @@ #ifdef TARGET_NR_stat64 case TARGET_NR_stat64: p = lock_user_string(arg1); + if( ret=page_check_range(p,1,PAGE_READ) ) return ret; ret = get_errno(stat(path(p), &st)); unlock_user(p, arg1, 0); goto do_stat64; @@ -4079,6 +4107,7 @@ #ifdef TARGET_NR_lstat64 case TARGET_NR_lstat64: p = lock_user_string(arg1); + if( ret=page_check_range(p,1,PAGE_READ) ) return ret; ret = get_errno(lstat(path(p), &st)); unlock_user(p, arg1, 0); goto do_stat64; @@ -4091,53 +4120,51 @@ if (!is_error(ret)) { #ifdef TARGET_ARM if (((CPUARMState *)cpu_env)->eabi) { - struct target_eabi_stat64 *target_st; - lock_user_struct(target_st, arg2, 1); + struct target_eabi_stat64 *target_st = (struct target_eabi_stat64 *)arg2; + if( !access_ok(VERIFY_WRITE,target_st,sizeof(struct target_eabi_stat64)) ) return -EFAULT; memset(target_st, 0, sizeof(struct target_eabi_stat64)); - /* put_user is probably wrong. */ - put_user(st.st_dev, &target_st->st_dev); - put_user(st.st_ino, &target_st->st_ino); + /* use __put_user() since we just checked that the buffer is valid */ + __put_user(st.st_dev, &target_st->st_dev); + __put_user(st.st_ino, &target_st->st_ino); #ifdef TARGET_STAT64_HAS_BROKEN_ST_INO - put_user(st.st_ino, &target_st->__st_ino); + __put_user(st.st_ino, &target_st->__st_ino); #endif - put_user(st.st_mode, &target_st->st_mode); - put_user(st.st_nlink, &target_st->st_nlink); - put_user(st.st_uid, &target_st->st_uid); - put_user(st.st_gid, &target_st->st_gid); - put_user(st.st_rdev, &target_st->st_rdev); + __put_user(st.st_mode, &target_st->st_mode); + __put_user(st.st_nlink, &target_st->st_nlink); + __put_user(st.st_uid, &target_st->st_uid); + __put_user(st.st_gid, &target_st->st_gid); + __put_user(st.st_rdev, &target_st->st_rdev); /* XXX: better use of kernel struct */ - put_user(st.st_size, &target_st->st_size); - put_user(st.st_blksize, &target_st->st_blksize); - put_user(st.st_blocks, &target_st->st_blocks); - put_user(st.st_atime, &target_st->target_st_atime); - put_user(st.st_mtime, &target_st->target_st_mtime); - put_user(st.st_ctime, &target_st->target_st_ctime); - unlock_user_struct(target_st, arg2, 0); + __put_user(st.st_size, &target_st->st_size); + __put_user(st.st_blksize, &target_st->st_blksize); + __put_user(st.st_blocks, &target_st->st_blocks); + __put_user(st.st_atime, &target_st->target_st_atime); + __put_user(st.st_mtime, &target_st->target_st_mtime); + __put_user(st.st_ctime, &target_st->target_st_ctime); } else #endif { - struct target_stat64 *target_st; - lock_user_struct(target_st, arg2, 1); + struct target_stat64 *target_st = (struct target_stat64 *)arg2; + if( !access_ok(VERIFY_WRITE,target_st,sizeof(struct target_stat64)) ) return -EFAULT; memset(target_st, 0, sizeof(struct target_stat64)); - /* ??? put_user is probably wrong. */ - put_user(st.st_dev, &target_st->st_dev); - put_user(st.st_ino, &target_st->st_ino); + /* use __put_user() since we just checked that the buffer is valid */ + __put_user(st.st_dev, &target_st->st_dev); + __put_user(st.st_ino, &target_st->st_ino); #ifdef TARGET_STAT64_HAS_BROKEN_ST_INO - put_user(st.st_ino, &target_st->__st_ino); + __put_user(st.st_ino, &target_st->__st_ino); #endif - put_user(st.st_mode, &target_st->st_mode); - put_user(st.st_nlink, &target_st->st_nlink); - put_user(st.st_uid, &target_st->st_uid); - put_user(st.st_gid, &target_st->st_gid); - put_user(st.st_rdev, &target_st->st_rdev); + __put_user(st.st_mode, &target_st->st_mode); + __put_user(st.st_nlink, &target_st->st_nlink); + __put_user(st.st_uid, &target_st->st_uid); + __put_user(st.st_gid, &target_st->st_gid); + __put_user(st.st_rdev, &target_st->st_rdev); /* XXX: better use of kernel struct */ - put_user(st.st_size, &target_st->st_size); - put_user(st.st_blksize, &target_st->st_blksize); - put_user(st.st_blocks, &target_st->st_blocks); - put_user(st.st_atime, &target_st->target_st_atime); - put_user(st.st_mtime, &target_st->target_st_mtime); - put_user(st.st_ctime, &target_st->target_st_ctime); - unlock_user_struct(target_st, arg2, 0); + __put_user(st.st_size, &target_st->st_size); + __put_user(st.st_blksize, &target_st->st_blksize); + __put_user(st.st_blocks, &target_st->st_blocks); + __put_user(st.st_atime, &target_st->target_st_atime); + __put_user(st.st_mtime, &target_st->target_st_mtime); + __put_user(st.st_ctime, &target_st->target_st_ctime); } } } @@ -4583,7 +4610,7 @@ struct timespec ts; ret = get_errno(clock_gettime(arg1, &ts)); if (!is_error(ret)) { - host_to_target_timespec(arg2, &ts); + if( copy_to_user_timespec(arg2, &ts)) return -EFAULT; } break; } @@ -4594,7 +4621,7 @@ struct timespec ts; ret = get_errno(clock_getres(arg1, &ts)); if (!is_error(ret)) { - host_to_target_timespec(arg2, &ts); + if( copy_to_user_timespec(arg2, &ts) ) return -EFAULT; } break; } Index: qemu/linux-user/qemu.h =================================================================== --- qemu.orig/linux-user/qemu.h 2007-07-13 07:10:58.000000000 -0400 +++ qemu/linux-user/qemu.h 2007-07-13 07:11:12.000000000 -0400 @@ -171,7 +171,8 @@ #define VERIFY_READ 0 #define VERIFY_WRITE 1 -#define access_ok(type,addr,size) (1) +#define access_ok(type,addr,size) \ + (page_check_range((target_ulong)addr,size,(type==VERIFY_READ)?PAGE_READ:PAGE_WRITE)==0) /* NOTE get_user and put_user use host addresses. */ #define __put_user(x,ptr)\ @@ -238,6 +239,26 @@ __ret;\ }) +#define copy_from_user(hptr, gptr, len) \ +({ \ + int ret=0;\ + if( access_ok(VERIFY_READ, gptr, (len) ) )\ + memcpy(hptr,gptr,(len)); \ + else \ + ret=1;\ + ret; \ +}) + +#define copy_to_user(gptr, hptr, len) \ +({ \ + int ret=0;\ + if( access_ok(VERIFY_WRITE, gptr, (len)) )\ + memcpy(gptr,hptr,(len)); \ + else \ + ret=1;\ + ret; \ +}) + /* Functions for accessing guest memory. The tget and tput functions read/write single values, byteswapping as neccessary. The lock_user gets a pointer to a contiguous area of guest memory, but does not perform @@ -295,6 +316,16 @@ #define unlock_user_struct(host_ptr, guest_addr, copy) \ unlock_user(host_ptr, guest_addr, (copy) ? sizeof(*host_ptr) : 0) +/* This is being phased ot in favor of the copy*user() interfaces */ +static inline long lock_and_check_user_struct(void *host_addr, target_ulong guest_addr, long len, int copy, int pg_access) +{ + long *haddr, ret = 0; + haddr = (long *)host_addr; + if( (ret=page_check_range(guest_addr,len,pg_access)) != 0 ) return ret; + *haddr = (long)lock_user(guest_addr, len, copy); + return 0; +} + #define tget8(addr) ldub(addr) #define tput8(addr, val) stb(addr, val) #define tget16(addr) lduw(addr) ^ permalink raw reply [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH] linux-user EFAULT implementation 2007-07-13 17:55 ` Stuart Anderson @ 2007-07-20 20:16 ` Stuart Anderson 0 siblings, 0 replies; 13+ messages in thread From: Stuart Anderson @ 2007-07-20 20:16 UTC (permalink / raw) To: qemu-devel [-- Attachment #1: Type: TEXT/PLAIN, Size: 993 bytes --] > On Tue, 10 Jul 2007, Stuart Anderson wrote: > > Further comments would be appreciated. Having heard no further comments, I have continued on in the process of coverting things to use the copy_{to|from}_user() interfaces. The attached patch does well with the LTP test suite, and is currently happily building an armel Debian glibc package on a x86_64 host. There are still a few uses of the lock*() interfaces remaining, but considering the size of this patch already, I'd like to suggest that if this patch is acceptable, to commit this patch, and the rest will be sent in as smaller, easier to digest patches, probably one or two syscalls at a time. Stuart Stuart R. Anderson anderson@netsweng.com Network & Software Engineering http://www.netsweng.com/ 1024D/37A79149: 0791 D3B8 9A4C 2CDC A31F BD03 0A62 E534 37A7 9149 [-- Attachment #2: EFAULT implementation --] [-- Type: TEXT/x-diff, Size: 62298 bytes --] Index: qemu/exec.c =================================================================== --- qemu.orig/exec.c 2007-07-20 13:43:58.000000000 -0400 +++ qemu/exec.c 2007-07-20 13:48:29.000000000 -0400 @@ -1862,6 +1862,29 @@ spin_unlock(&tb_lock); } +int page_check_range(target_ulong start, target_ulong len, int flags) +{ + PageDesc *p; + target_ulong end; + target_ulong addr; + + end = TARGET_PAGE_ALIGN(start+len); /* must do before we loose bits in the next step */ + start = start & TARGET_PAGE_MASK; + + if( end < start ) return -1; /* we've wrapped around */ + for(addr = start; addr < end; addr += TARGET_PAGE_SIZE) { + p = page_find(addr >> TARGET_PAGE_BITS); + if( !p ) return -1; + if( !(p->flags & PAGE_VALID) ) return -1; + + if (!(p->flags & PAGE_READ) && + (flags & PAGE_READ) ) return -1; + if (!(p->flags & PAGE_WRITE) && + (flags & PAGE_WRITE) ) return -1; + } + return 0; +} + /* called from signal handler: invalidate the code and unprotect the page. Return TRUE if the fault was succesfully handled. */ int page_unprotect(target_ulong address, unsigned long pc, void *puc) Index: qemu/cpu-all.h =================================================================== --- qemu.orig/cpu-all.h 2007-07-20 13:43:58.000000000 -0400 +++ qemu/cpu-all.h 2007-07-20 13:48:29.000000000 -0400 @@ -691,6 +691,7 @@ int page_get_flags(target_ulong address); void page_set_flags(target_ulong start, target_ulong end, int flags); void page_unprotect_range(target_ulong data, target_ulong data_size); +int page_check_range(target_ulong start, target_ulong len, int flags); CPUState *cpu_copy(CPUState *env); Index: qemu/linux-user/syscall.c =================================================================== --- qemu.orig/linux-user/syscall.c 2007-07-20 13:43:58.000000000 -0400 +++ qemu/linux-user/syscall.c 2007-07-20 15:38:13.000000000 -0400 @@ -404,12 +404,10 @@ #endif } -static inline void host_to_target_rusage(target_ulong target_addr, +static inline long copy_to_user_rusage(struct target_rusage *target_rusage, const struct rusage *rusage) { - struct target_rusage *target_rusage; - - lock_user_struct(target_rusage, target_addr, 0); + if( copy_to_user(target_rusage,rusage,sizeof(*target_rusage)) ) return -1; target_rusage->ru_utime.tv_sec = tswapl(rusage->ru_utime.tv_sec); target_rusage->ru_utime.tv_usec = tswapl(rusage->ru_utime.tv_usec); target_rusage->ru_stime.tv_sec = tswapl(rusage->ru_stime.tv_sec); @@ -428,29 +426,28 @@ target_rusage->ru_nsignals = tswapl(rusage->ru_nsignals); target_rusage->ru_nvcsw = tswapl(rusage->ru_nvcsw); target_rusage->ru_nivcsw = tswapl(rusage->ru_nivcsw); - unlock_user_struct(target_rusage, target_addr, 1); + + return 0; } -static inline void target_to_host_timeval(struct timeval *tv, - target_ulong target_addr) +static inline long copy_from_user_timeval(struct timeval *tv, + struct target_timeval *target_tv) { - struct target_timeval *target_tv; - - lock_user_struct(target_tv, target_addr, 1); + if( copy_from_user(tv,target_tv,sizeof(*target_tv)) ) return -1; tv->tv_sec = tswapl(target_tv->tv_sec); tv->tv_usec = tswapl(target_tv->tv_usec); - unlock_user_struct(target_tv, target_addr, 0); + + return 0; } -static inline void host_to_target_timeval(target_ulong target_addr, +static inline long copy_to_user_timeval(struct target_timeval *target_tv, const struct timeval *tv) { - struct target_timeval *target_tv; - - lock_user_struct(target_tv, target_addr, 0); + if( copy_to_user(target_tv,tv,sizeof(*target_tv)) ) return -1; target_tv->tv_sec = tswapl(tv->tv_sec); target_tv->tv_usec = tswapl(tv->tv_usec); - unlock_user_struct(target_tv, target_addr, 1); + + return 0; } @@ -466,21 +463,21 @@ int ok; if (rfd_p) { - target_rfds = lock_user(rfd_p, sizeof(target_long) * n, 1); + if( lock_and_check_user_struct(&target_rfds,rfd_p,sizeof(target_long)*n,1,PAGE_READ) ) return -EFAULT; rfds_ptr = target_to_host_fds(&rfds, target_rfds, n); } else { target_rfds = NULL; rfds_ptr = NULL; } if (wfd_p) { - target_wfds = lock_user(wfd_p, sizeof(target_long) * n, 1); + if( lock_and_check_user_struct(&target_wfds,wfd_p,sizeof(target_long)*n,1,PAGE_READ) ) return -EFAULT; wfds_ptr = target_to_host_fds(&wfds, target_wfds, n); } else { target_wfds = NULL; wfds_ptr = NULL; } if (efd_p) { - target_efds = lock_user(efd_p, sizeof(target_long) * n, 1); + if( lock_and_check_user_struct(&target_efds,efd_p,sizeof(target_long)*n,1,PAGE_READ) ) return -EFAULT; efds_ptr = target_to_host_fds(&efds, target_efds, n); } else { target_efds = NULL; @@ -488,7 +485,7 @@ } if (target_tv) { - target_to_host_timeval(&tv, target_tv); + if( copy_from_user_timeval(&tv, (struct target_timeval *)target_tv) ) return -EFAULT; tv_ptr = &tv; } else { tv_ptr = NULL; @@ -502,7 +499,7 @@ host_to_target_fds(target_efds, efds_ptr, n); if (target_tv) { - host_to_target_timeval(target_tv, &tv); + if( copy_to_user_timeval((struct target_timeval *)target_tv, &tv) ) return -EFAULT; } } if (target_rfds) @@ -515,28 +512,24 @@ return ret; } -static inline void target_to_host_sockaddr(struct sockaddr *addr, - target_ulong target_addr, +static inline long copy_from_user_sockaddr(struct sockaddr *addr, + struct target_sockaddr *target_saddr, socklen_t len) { - struct target_sockaddr *target_saddr; - - target_saddr = lock_user(target_addr, len, 1); - memcpy(addr, target_saddr, len); + if( copy_from_user(addr,target_saddr,len)) return -1; addr->sa_family = tswap16(target_saddr->sa_family); - unlock_user(target_saddr, target_addr, 0); + + return 0; } -static inline void host_to_target_sockaddr(target_ulong target_addr, +static inline long copy_to_user_sockaddr(struct target_sockaddr *target_saddr, struct sockaddr *addr, socklen_t len) { - struct target_sockaddr *target_saddr; - - target_saddr = lock_user(target_addr, len, 0); - memcpy(target_saddr, addr, len); + if( copy_to_user(target_saddr,addr,len) ) return -1; target_saddr->sa_family = tswap16(addr->sa_family); - unlock_user(target_saddr, target_addr, len); + + return 0; } /* ??? Should this also swap msgh->name? */ @@ -628,7 +621,7 @@ msgh->msg_controllen = tswapl(space); } -static long do_setsockopt(int sockfd, int level, int optname, +static long do_setsockopt(int sockfd, int level, int optname, target_ulong optval, socklen_t optlen) { int val, ret; @@ -639,6 +632,8 @@ if (optlen < sizeof(uint32_t)) return -EINVAL; + if( !access_ok(VERIFY_READ, optval, optlen) ) + return -EFAULT; val = tget32(optval); ret = get_errno(setsockopt(sockfd, level, optname, &val, sizeof(val))); break; @@ -660,6 +655,8 @@ case IP_MULTICAST_TTL: case IP_MULTICAST_LOOP: val = 0; + if( !access_ok(VERIFY_READ, optval, optlen) ) + return -EFAULT; if (optlen >= sizeof(uint32_t)) { val = tget32(optval); } else if (optlen >= 1) { @@ -668,7 +665,7 @@ ret = get_errno(setsockopt(sockfd, level, optname, &val, sizeof(val))); break; default: - goto unimplemented; + goto unimplemented; } break; case TARGET_SOL_SOCKET: @@ -732,24 +729,26 @@ break; break; default: - goto unimplemented; + goto unimplemented; } if (optlen < sizeof(uint32_t)) - return -EINVAL; + return -EINVAL; + if( !access_ok(VERIFY_READ, optval, optlen) ) + return -EFAULT; val = tget32(optval); ret = get_errno(setsockopt(sockfd, SOL_SOCKET, optname, &val, sizeof(val))); break; default: unimplemented: - gemu_log("Unsupported setsockopt level=%d optname=%d \n", level, optname); - ret = -ENOSYS; + gemu_log("Unsupported setsockopt level=%d optname=%d\n", level, optname); + ret = -ENOPROTOOPT; } return ret; } -static long do_getsockopt(int sockfd, int level, int optname, - target_ulong optval, target_ulong optlen) +static long do_getsockopt(int sockfd, int level, int optname, + target_ulong optval, socklen_t *optlen) { int len, lv, val, ret; @@ -771,7 +770,10 @@ case SOL_TCP: /* TCP options all take an 'int' value. */ int_case: - len = tget32(optlen); + if( get_user(len,optlen) ) + return -EFAULT; + if( !access_ok(VERIFY_WRITE, optval,len) ) + return -EFAULT; if (len < 0) return -EINVAL; lv = sizeof(int); @@ -804,7 +806,10 @@ #endif case IP_MULTICAST_TTL: case IP_MULTICAST_LOOP: - len = tget32(optlen); + if( get_user(len,optlen) ) + return -EINVAL; + if( !access_ok(VERIFY_WRITE, optval,len) ) + return -EFAULT; if (len < 0) return -EINVAL; lv = sizeof(int); @@ -823,14 +828,15 @@ } break; default: - goto unimplemented; + ret = -ENOPROTOOPT; + break; } break; default: unimplemented: gemu_log("getsockopt level=%d optname=%d not yet supported\n", level, optname); - ret = -ENOSYS; + ret = -EOPNOTSUPP; break; } return ret; @@ -899,7 +905,7 @@ { void *addr = alloca(addrlen); - target_to_host_sockaddr(addr, target_addr, addrlen); + if( copy_from_user_sockaddr(addr, (struct target_sockaddr *)target_addr, addrlen) ) return -EFAULT; return get_errno(bind(sockfd, addr, addrlen)); } @@ -908,7 +914,7 @@ { void *addr = alloca(addrlen); - target_to_host_sockaddr(addr, target_addr, addrlen); + if( copy_from_user_sockaddr(addr, (struct target_sockaddr *)target_addr, addrlen) ) return -EFAULT; return get_errno(connect(sockfd, addr, addrlen)); } @@ -926,8 +932,8 @@ if (msgp->msg_name) { msg.msg_namelen = tswap32(msgp->msg_namelen); msg.msg_name = alloca(msg.msg_namelen); - target_to_host_sockaddr(msg.msg_name, tswapl(msgp->msg_name), - msg.msg_namelen); + if( copy_from_user_sockaddr(msg.msg_name, (struct target_sockaddr *)msgp->msg_name, + msg.msg_namelen) ) return -EFAULT; } else { msg.msg_name = NULL; msg.msg_namelen = 0; @@ -956,46 +962,55 @@ } static long do_accept(int fd, target_ulong target_addr, - target_ulong target_addrlen) + socklen_t *target_addrlen) { - socklen_t addrlen = tget32(target_addrlen); - void *addr = alloca(addrlen); + socklen_t addrlen; + void *addr; long ret; + if( get_user(addrlen,target_addrlen) ) return -EINVAL; + addr = alloca(addrlen); + ret = get_errno(accept(fd, addr, &addrlen)); if (!is_error(ret)) { - host_to_target_sockaddr(target_addr, addr, addrlen); + if( copy_to_user_sockaddr((struct target_sockaddr *)target_addr, addr, addrlen) ) return -EFAULT; tput32(target_addrlen, addrlen); } return ret; } static long do_getpeername(int fd, target_ulong target_addr, - target_ulong target_addrlen) + socklen_t *target_addrlen) { - socklen_t addrlen = tget32(target_addrlen); - void *addr = alloca(addrlen); + socklen_t addrlen; + void *addr; long ret; + if( get_user(addrlen,target_addrlen) ) return -EFAULT; + addr = alloca(addrlen); + ret = get_errno(getpeername(fd, addr, &addrlen)); if (!is_error(ret)) { - host_to_target_sockaddr(target_addr, addr, addrlen); - tput32(target_addrlen, addrlen); + if( copy_to_user_sockaddr((struct target_sockaddr *)target_addr, addr, addrlen) ) return -EFAULT; + tput32(target_addrlen,addrlen); } return ret; } static long do_getsockname(int fd, target_ulong target_addr, - target_ulong target_addrlen) + socklen_t *target_addrlen) { - socklen_t addrlen = tget32(target_addrlen); - void *addr = alloca(addrlen); + socklen_t addrlen; + void *addr; long ret; + if( get_user(addrlen,target_addrlen) ) return -EFAULT; + addr = alloca(addrlen); + ret = get_errno(getsockname(fd, addr, &addrlen)); if (!is_error(ret)) { - host_to_target_sockaddr(target_addr, addr, addrlen); - tput32(target_addrlen, addrlen); + if( copy_to_user_sockaddr((struct target_sockaddr *)target_addr, addr, addrlen) ) return -EFAULT; + tput32(target_addrlen,addrlen); } return ret; } @@ -1006,6 +1021,8 @@ int tab[2]; long ret; + if( !access_ok(VERIFY_WRITE,target_tab,2*sizeof(int)) ) + return -EFAULT; ret = get_errno(socketpair(domain, type, protocol, tab)); if (!is_error(ret)) { tput32(target_tab, tab[0]); @@ -1018,18 +1035,18 @@ target_ulong target_addr, socklen_t addrlen) { void *addr; - void *host_msg; + void *target_msg = (void *)msg; + void *host_msg = alloca(len); long ret; - host_msg = lock_user(msg, len, 1); + if (copy_from_user(host_msg,target_msg,len) ) return -EFAULT; if (target_addr) { addr = alloca(addrlen); - target_to_host_sockaddr(addr, target_addr, addrlen); + if (copy_from_user_sockaddr(addr, (struct target_sockaddr *)target_addr, addrlen) ) return -EFAULT; ret = get_errno(sendto(fd, host_msg, len, flags, addr, addrlen)); } else { ret = get_errno(send(fd, host_msg, len, flags)); } - unlock_user(host_msg, msg, 0); return ret; } @@ -1038,10 +1055,10 @@ { socklen_t addrlen; void *addr; - void *host_msg; + void *target_msg = (void *)msg; + void *host_msg = alloca(len); long ret; - host_msg = lock_user(msg, len, 0); if (target_addr) { addrlen = tget32(target_addrlen); addr = alloca(addrlen); @@ -1052,12 +1069,10 @@ } if (!is_error(ret)) { if (target_addr) { - host_to_target_sockaddr(target_addr, addr, addrlen); + if (copy_to_user_sockaddr((struct target_sockaddr *)target_addr, addr, addrlen) ) return -EFAULT; tput32(target_addrlen, addrlen); } - unlock_user(host_msg, msg, len); - } else { - unlock_user(host_msg, msg, 0); + if( copy_to_user(target_msg, host_msg, len) ) return -EFAULT; } return ret; } @@ -1104,7 +1119,7 @@ int sockfd = tgetl(vptr); target_ulong target_addr = tgetl(vptr + n); target_ulong target_addrlen = tgetl(vptr + 2 * n); - ret = do_accept(sockfd, target_addr, target_addrlen); + ret = do_accept(sockfd, target_addr, (socklen_t *)target_addrlen); } break; case SOCKOP_getsockname: @@ -1112,7 +1127,7 @@ int sockfd = tgetl(vptr); target_ulong target_addr = tgetl(vptr + n); target_ulong target_addrlen = tgetl(vptr + 2 * n); - ret = do_getsockname(sockfd, target_addr, target_addrlen); + ret = do_getsockname(sockfd, target_addr, (socklen_t *)target_addrlen); } break; case SOCKOP_getpeername: @@ -1120,7 +1135,7 @@ int sockfd = tgetl(vptr); target_ulong target_addr = tgetl(vptr + n); target_ulong target_addrlen = tgetl(vptr + 2 * n); - ret = do_getpeername(sockfd, target_addr, target_addrlen); + ret = do_getpeername(sockfd, target_addr, (socklen_t *)target_addrlen); } break; case SOCKOP_socketpair: @@ -1214,12 +1229,12 @@ target_ulong optval = tgetl(vptr + 3 * n); target_ulong poptlen = tgetl(vptr + 4 * n); - ret = do_getsockopt(sockfd, level, optname, optval, poptlen); + ret = do_getsockopt(sockfd, level, optname, optval, (socklen_t *)poptlen); } break; default: gemu_log("Unsupported socketcall: %d\n", num); - ret = -ENOSYS; + ret = -EINVAL; break; } return ret; @@ -1259,64 +1274,46 @@ target_ulong __unused4; }; -static inline void target_to_host_ipc_perm(struct ipc_perm *host_ip, - target_ulong target_addr) +static inline void copy_from_user_ipc_perm(struct ipc_perm *host_ip, + struct target_ipc_perm *target_ip) { - struct target_ipc_perm *target_ip; - struct target_semid_ds *target_sd; - - lock_user_struct(target_sd, target_addr, 1); - target_ip=&(target_sd->sem_perm); host_ip->__key = tswapl(target_ip->__key); host_ip->uid = tswapl(target_ip->uid); host_ip->gid = tswapl(target_ip->gid); host_ip->cuid = tswapl(target_ip->cuid); host_ip->cgid = tswapl(target_ip->cgid); - host_ip->mode = tswapl(target_ip->mode); - unlock_user_struct(target_sd, target_addr, 0); + host_ip->mode = tswap16(target_ip->mode); + host_ip->__seq = tswap16(target_ip->__seq); } -static inline void host_to_target_ipc_perm(target_ulong target_addr, +static inline void copy_to_user_ipc_perm(struct target_ipc_perm *target_ip, struct ipc_perm *host_ip) { - struct target_ipc_perm *target_ip; - struct target_semid_ds *target_sd; - - lock_user_struct(target_sd, target_addr, 0); - target_ip = &(target_sd->sem_perm); target_ip->__key = tswapl(host_ip->__key); target_ip->uid = tswapl(host_ip->uid); target_ip->gid = tswapl(host_ip->gid); target_ip->cuid = tswapl(host_ip->cuid); target_ip->cgid = tswapl(host_ip->cgid); - target_ip->mode = tswapl(host_ip->mode); - unlock_user_struct(target_sd, target_addr, 1); + target_ip->mode = tswap16(host_ip->mode); + target_ip->__seq = tswap16(host_ip->__seq); } -static inline void target_to_host_semid_ds(struct semid_ds *host_sd, - target_ulong target_addr) +static inline void copy_from_user_semid_ds(struct semid_ds *host_sd, + struct target_semid_ds *target_sd) { - struct target_semid_ds *target_sd; - - lock_user_struct(target_sd, target_addr, 1); - target_to_host_ipc_perm(&(host_sd->sem_perm),target_addr); + copy_from_user_ipc_perm(&(host_sd->sem_perm),&(target_sd->sem_perm)); 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); - unlock_user_struct(target_sd, target_addr, 0); } -static inline void host_to_target_semid_ds(target_ulong target_addr, +static inline void copy_to_user_semid_ds(struct target_semid_ds *target_sd, struct semid_ds *host_sd) { - struct target_semid_ds *target_sd; - - lock_user_struct(target_sd, target_addr, 0); - host_to_target_ipc_perm(target_addr,&(host_sd->sem_perm)); + copy_to_user_ipc_perm(&(target_sd->sem_perm),&(host_sd->sem_perm)); 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); - unlock_user_struct(target_sd, target_addr, 1); } union semun { @@ -1327,113 +1324,107 @@ union target_semun { int val; - target_long buf; - unsigned short int *array; + struct target_semid_ds *buf; + unsigned short *array; + long host_ptr; }; -static inline void target_to_host_semun(unsigned long cmd, - union semun *host_su, - target_ulong target_addr, +static inline long copy_from_user_semun(unsigned long cmd, + union semun host_su, + union target_semun target_su, struct semid_ds *ds) { - union target_semun *target_su; - switch( cmd ) { - case IPC_STAT: case IPC_SET: - lock_user_struct(target_su, target_addr, 1); - 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: - lock_user_struct(target_su, target_addr, 1); - host_su->val = tswapl(target_su->val); - unlock_user_struct(target_su, target_addr, 0); + copy_from_user_semid_ds(ds,target_su.buf); + host_su.buf = ds; break; - case GETALL: case SETALL: - lock_user_struct(target_su, target_addr, 1); - *host_su->array = tswap16(*target_su->array); - unlock_user_struct(target_su, target_addr, 0); + { + int i; + for(i=0;i<ds->sem_nsems;i++) + if (get_user(host_su.array[i],&(target_su.array[i]))) return -1; break; + } default: gemu_log("semun operation not fully supported: %d\n", (int)cmd); } + + return 0; } -static inline void host_to_target_semun(unsigned long cmd, - target_ulong target_addr, - union semun *host_su, +static inline long copy_to_user_semun(unsigned long cmd, + union target_semun target_su, + union semun host_su, struct semid_ds *ds) { - union target_semun *target_su; - switch( cmd ) { case IPC_STAT: - case IPC_SET: - lock_user_struct(target_su, target_addr, 0); - host_to_target_semid_ds(target_su->buf,ds); - unlock_user_struct(target_su, target_addr, 1); - break; - case GETVAL: - case SETVAL: - lock_user_struct(target_su, target_addr, 0); - target_su->val = tswapl(host_su->val); - unlock_user_struct(target_su, target_addr, 1); + if( !access_ok(VERIFY_WRITE,(target_ulong)target_su.buf,sizeof(struct target_semid_ds)) ) return -1; + copy_to_user_semid_ds(target_su.buf,ds); break; case GETALL: - case SETALL: - lock_user_struct(target_su, target_addr, 0); - *target_su->array = tswap16(*host_su->array); - unlock_user_struct(target_su, target_addr, 1); + { + int i; + for(i=0;i<ds->sem_nsems;i++) + if (put_user(host_su.array[i],&(target_su.array[i]))) return -1; break; + } default: gemu_log("semun operation not fully supported: %d\n", (int)cmd); } + + return 0; } static inline long do_semctl(long first, long second, long third, long ptr) { union semun arg; + union target_semun targ = (union target_semun)ptr; struct semid_ds dsarg; int cmd = third&0xff; long ret = 0; + arg.buf = &dsarg; + 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); + ret = get_errno(semctl(first, second, cmd, targ)); 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); + ret = get_errno(semctl(first, second, cmd, targ.val)); break; case GETALL: - target_to_host_semun(cmd,&arg,ptr,&dsarg); + /* Use IPC_STAT to figure out how many semaphores are in + * the set, and allocate buffer space for them. + */ + semctl(first,second, IPC_STAT,arg); + arg.array = alloca(sizeof(unsigned short)*dsarg.sem_nsems); ret = get_errno(semctl(first, second, cmd, arg)); - host_to_target_semun(cmd,ptr,&arg,&dsarg); + if (!is_error(ret)) { + if( copy_to_user_semun(cmd,targ,arg,&dsarg) ) return -EFAULT; + } break; case SETALL: - target_to_host_semun(cmd,&arg,ptr,&dsarg); + /* Use IPC_STAT to figure out how many semaphores are in + * the set, and allocate buffer space for them. + */ + semctl(first,second, IPC_STAT,arg); + arg.array = alloca(sizeof(unsigned short)*dsarg.sem_nsems); + if( copy_from_user_semun(cmd,arg,targ,&dsarg) ) return -EFAULT; ret = get_errno(semctl(first, second, cmd, arg)); - host_to_target_semun(cmd,ptr,&arg,&dsarg); 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); + if( copy_to_user_semun(cmd,targ,arg,&dsarg) ) return -EFAULT; break; case IPC_SET: - target_to_host_semun(cmd,&arg,ptr,&dsarg); + if( copy_from_user(arg.buf,targ.buf,sizeof(union target_semun)) ) return -EFAULT; + if( copy_from_user_semun(cmd,arg,targ,&dsarg) ) return -EFAULT; ret = get_errno(semctl(first, second, cmd, arg)); - host_to_target_semun(cmd,ptr,&arg,&dsarg); break; default: - ret = get_errno(semctl(first, second, cmd, arg)); + ret = get_errno(semctl(first, second, cmd, ptr)); } return ret; @@ -1457,13 +1448,10 @@ target_ulong __unused5; }; -static inline void target_to_host_msqid_ds(struct msqid_ds *host_md, - target_ulong target_addr) +static inline long copy_from_user_msqid_ds(struct msqid_ds *host_md, + struct target_msqid_ds *target_md) { - struct target_msqid_ds *target_md; - - lock_user_struct(target_md, target_addr, 1); - target_to_host_ipc_perm(&(host_md->msg_perm),target_addr); + copy_from_user_ipc_perm(&(host_md->msg_perm),&(target_md->msg_perm)); host_md->msg_stime = tswapl(target_md->msg_stime); host_md->msg_rtime = tswapl(target_md->msg_rtime); host_md->msg_ctime = tswapl(target_md->msg_ctime); @@ -1472,16 +1460,14 @@ host_md->msg_qbytes = tswapl(target_md->msg_qbytes); host_md->msg_lspid = tswapl(target_md->msg_lspid); host_md->msg_lrpid = tswapl(target_md->msg_lrpid); - unlock_user_struct(target_md, target_addr, 0); + + return 0; } -static inline void host_to_target_msqid_ds(target_ulong target_addr, - struct msqid_ds *host_md) +static inline long copy_to_usermsqid_ds(struct target_msqid_ds *target_md, + struct msqid_ds *host_md) { - struct target_msqid_ds *target_md; - - lock_user_struct(target_md, target_addr, 0); - host_to_target_ipc_perm(target_addr,&(host_md->msg_perm)); + copy_to_user_ipc_perm(&(target_md->msg_perm),&(host_md->msg_perm)); target_md->msg_stime = tswapl(host_md->msg_stime); target_md->msg_rtime = tswapl(host_md->msg_rtime); target_md->msg_ctime = tswapl(host_md->msg_ctime); @@ -1490,20 +1476,29 @@ target_md->msg_qbytes = tswapl(host_md->msg_qbytes); target_md->msg_lspid = tswapl(host_md->msg_lspid); target_md->msg_lrpid = tswapl(host_md->msg_lrpid); - unlock_user_struct(target_md, target_addr, 1); + + return 0; } static inline long do_msgctl(long first, long second, long ptr) { struct msqid_ds dsarg; + struct target_msqid_ds *target_ds = (struct target_msqid_ds *)ptr; int cmd = second&0xff; long ret = 0; switch( cmd ) { case IPC_STAT: + if( copy_from_user(&dsarg,target_ds,sizeof(struct msqid_ds)) ) return -EFAULT; + copy_from_user_msqid_ds(&dsarg,target_ds); + ret = get_errno(msgctl(first, cmd, &dsarg)); + if( copy_to_usermsqid_ds(target_ds,&dsarg) ) return -EFAULT; + break; case IPC_SET: - target_to_host_msqid_ds(&dsarg,ptr); + if( copy_from_user(&dsarg,target_ds,sizeof(struct msqid_ds)) ) return -EFAULT; + copy_from_user_msqid_ds(&dsarg,target_ds); ret = get_errno(msgctl(first, cmd, &dsarg)); - host_to_target_msqid_ds(ptr,&dsarg); + if( copy_to_usermsqid_ds(target_ds,&dsarg) ) return -EFAULT; + break; default: ret = get_errno(msgctl(first, cmd, &dsarg)); } @@ -1511,41 +1506,44 @@ } struct target_msgbuf { - target_ulong mtype; + target_long mtype; char mtext[1]; }; static inline long do_msgsnd(long msqid, long msgp, long msgsz, long msgflg) { - struct target_msgbuf *target_mb; + struct target_msgbuf *target_mb = (struct target_msgbuf *)msgp; + target_long tmsgsz = tswap32(msgsz); struct msgbuf *host_mb; long ret = 0; - lock_user_struct(target_mb,msgp,0); - host_mb = malloc(msgsz+sizeof(long)); + if( tmsgsz < 0 ) return -EINVAL; + if( !(target_mb->mtype > 0) ) return -EINVAL; + + host_mb = alloca(sizeof(long)+msgsz); + + if( copy_from_user(host_mb->mtext,target_mb->mtext,msgsz) ) return -EFAULT; host_mb->mtype = tswapl(target_mb->mtype); - memcpy(host_mb->mtext,target_mb->mtext,msgsz); ret = get_errno(msgsnd(msqid, host_mb, msgsz, msgflg)); - free(host_mb); - unlock_user_struct(target_mb, msgp, 0); return ret; } static inline long do_msgrcv(long msqid, long msgp, long msgsz, long msgtype, long msgflg) { - struct target_msgbuf *target_mb; + struct target_msgbuf *target_mb = (struct target_msgbuf *)msgp; + target_long tmsgsz = tswap32(msgsz); struct msgbuf *host_mb; long ret = 0; - lock_user_struct(target_mb, msgp, 0); - host_mb = malloc(msgsz+sizeof(long)); - ret = get_errno(msgrcv(msqid, host_mb, msgsz, 1, msgflg)); + if( tmsgsz < 0 ) return -EINVAL; + + host_mb = alloca(sizeof(long)+msgsz); + + ret = get_errno(msgrcv(msqid, host_mb, msgsz, msgtype, msgflg)); if (ret > 0) - memcpy(target_mb->mtext, host_mb->mtext, ret); + if( copy_to_user(target_mb->mtext,host_mb->mtext,ret) ) return -EFAULT; target_mb->mtype = tswapl(host_mb->mtype); - free(host_mb); - unlock_user_struct(target_mb, msgp, 0); return ret; } @@ -2018,6 +2016,7 @@ static int write_ldt(CPUX86State *env, target_ulong ptr, unsigned long bytecount, int oldmode) { + long ret = 0; struct target_modify_ldt_ldt_s ldt_info; struct target_modify_ldt_ldt_s *target_ldt_info; int seg_32bit, contents, read_exec_only, limit_in_pages; @@ -2026,7 +2025,7 @@ if (bytecount != sizeof(ldt_info)) return -EINVAL; - lock_user_struct(target_ldt_info, ptr, 1); + if( lock_and_check_user_struct(&target_ldt_info,ptr,sizeof(struct target_modify_ldt_ldt_s),1,PAGE_READ) ) return -EFAULT; ldt_info.entry_number = tswap32(target_ldt_info->entry_number); ldt_info.base_addr = tswapl(target_ldt_info->base_addr); ldt_info.limit = tswap32(target_ldt_info->limit); @@ -2225,7 +2224,7 @@ switch(cmd) { case TARGET_F_GETLK: - lock_user_struct(target_fl, arg, 1); + if( lock_and_check_user_struct(&target_fl,arg,sizeof(struct target_flock),1,PAGE_READ) ) return -EFAULT; fl.l_type = tswap16(target_fl->l_type); fl.l_whence = tswap16(target_fl->l_whence); fl.l_start = tswapl(target_fl->l_start); @@ -2234,6 +2233,7 @@ unlock_user_struct(target_fl, arg, 0); ret = fcntl(fd, cmd, &fl); if (ret == 0) { + if( lock_and_check_user_struct(&target_fl,arg,sizeof(struct target_flock),0,PAGE_WRITE) ) return -EFAULT; lock_user_struct(target_fl, arg, 0); target_fl->l_type = tswap16(fl.l_type); target_fl->l_whence = tswap16(fl.l_whence); @@ -2246,7 +2246,7 @@ case TARGET_F_SETLK: case TARGET_F_SETLKW: - lock_user_struct(target_fl, arg, 1); + if( lock_and_check_user_struct(&target_fl,arg,sizeof(struct target_flock),1,PAGE_READ) ) return -EFAULT; fl.l_type = tswap16(target_fl->l_type); fl.l_whence = tswap16(target_fl->l_whence); fl.l_start = tswapl(target_fl->l_start); @@ -2257,7 +2257,7 @@ break; case TARGET_F_GETLK64: - lock_user_struct(target_fl64, arg, 1); + if( lock_and_check_user_struct(&target_fl64,arg,sizeof(struct target_flock64),1,PAGE_READ) ) return -EFAULT; fl64.l_type = tswap16(target_fl64->l_type) >> 1; fl64.l_whence = tswap16(target_fl64->l_whence); fl64.l_start = tswapl(target_fl64->l_start); @@ -2266,7 +2266,7 @@ unlock_user_struct(target_fl64, arg, 0); ret = fcntl(fd, cmd >> 1, &fl64); if (ret == 0) { - lock_user_struct(target_fl64, arg, 0); + if( lock_and_check_user_struct(&target_fl64,arg,sizeof(struct target_flock64),0,PAGE_WRITE) ) return -EFAULT; target_fl64->l_type = tswap16(fl64.l_type) >> 1; target_fl64->l_whence = tswap16(fl64.l_whence); target_fl64->l_start = tswapl(fl64.l_start); @@ -2277,7 +2277,7 @@ break; case TARGET_F_SETLK64: case TARGET_F_SETLKW64: - lock_user_struct(target_fl64, arg, 1); + if( lock_and_check_user_struct(&target_fl64,arg,sizeof(struct target_flock64),1,PAGE_READ) ) return -EFAULT; fl64.l_type = tswap16(target_fl64->l_type) >> 1; fl64.l_whence = tswap16(target_fl64->l_whence); fl64.l_start = tswapl(target_fl64->l_start); @@ -2419,32 +2419,30 @@ } #endif -static inline void target_to_host_timespec(struct timespec *host_ts, - target_ulong target_addr) +static inline long copy_from_user_timespec(struct timespec *host_ts, + struct target_timespec *target_ts) { - struct target_timespec *target_ts; - - lock_user_struct(target_ts, target_addr, 1); + if( copy_from_user(host_ts,target_ts,sizeof(*target_ts)) ) return -1; host_ts->tv_sec = tswapl(target_ts->tv_sec); host_ts->tv_nsec = tswapl(target_ts->tv_nsec); - unlock_user_struct(target_ts, target_addr, 0); + + return 0; } -static inline void host_to_target_timespec(target_ulong target_addr, +static inline long copy_to_user_timespec(struct target_timespec *target_ts, struct timespec *host_ts) { - struct target_timespec *target_ts; - - lock_user_struct(target_ts, target_addr, 0); + if( copy_to_user(target_ts, host_ts, sizeof(*target_ts)) ) return -1; target_ts->tv_sec = tswapl(host_ts->tv_sec); target_ts->tv_nsec = tswapl(host_ts->tv_nsec); - unlock_user_struct(target_ts, target_addr, 1); + + return 0; } long do_syscall(void *cpu_env, int num, long arg1, long arg2, long arg3, long arg4, long arg5, long arg6) { - long ret; + long ret = 0; struct stat st; struct statfs stfs; void *p; @@ -2671,7 +2669,7 @@ struct utimbuf tbuf, *host_tbuf; struct target_utimbuf *target_tbuf; if (arg2) { - lock_user_struct(target_tbuf, arg2, 1); + if( lock_and_check_user_struct(&target_tbuf,arg2,sizeof(struct target_utimbuf),1,PAGE_READ) ) return -EFAULT; tbuf.actime = tswapl(target_tbuf->actime); tbuf.modtime = tswapl(target_tbuf->modtime); unlock_user_struct(target_tbuf, arg2, 0); @@ -2689,9 +2687,11 @@ { struct timeval *tvp, tv[2]; if (arg2) { - target_to_host_timeval(&tv[0], arg2); - target_to_host_timeval(&tv[1], - arg2 + sizeof (struct target_timeval)); + if( copy_from_user_timeval(&tv[0], (struct target_timeval *)arg2) ) + return -EFAULT; + if( copy_from_user_timeval(&tv[1], + (struct target_timeval *)(arg2 + sizeof (struct target_timeval))) ) + return -EFAULT; tvp = tv; } else { tvp = NULL; @@ -2860,7 +2860,7 @@ struct target_old_sigaction *old_act; struct target_sigaction act, oact, *pact; if (arg2) { - lock_user_struct(old_act, arg2, 1); + if( lock_and_check_user_struct(&old_act,arg2,sizeof(*old_act),1,PAGE_READ) ) return -EFAULT; act._sa_handler = old_act->_sa_handler; target_siginitset(&act.sa_mask, old_act->sa_mask); act.sa_flags = old_act->sa_flags; @@ -2872,7 +2872,7 @@ } ret = get_errno(do_sigaction(arg1, pact, &oact)); if (!is_error(ret) && arg3) { - lock_user_struct(old_act, arg3, 0); + if( lock_and_check_user_struct(&old_act,arg3,sizeof(*old_act),0,PAGE_WRITE) ) return -EFAULT; old_act->_sa_handler = oact._sa_handler; old_act->sa_mask = oact.sa_mask.sig[0]; old_act->sa_flags = oact.sa_flags; @@ -2883,7 +2883,7 @@ struct target_sigaction act, oact, *pact, *old_act; if (arg2) { - lock_user_struct(old_act, arg2, 1); + if( lock_and_check_user_struct(&old_act,arg2,sizeof(*old_act),1,PAGE_READ) ) return -EFAULT; act._sa_handler = old_act->_sa_handler; target_siginitset(&act.sa_mask, old_act->sa_mask.sig[0]); act.sa_flags = old_act->sa_flags; @@ -2896,7 +2896,7 @@ ret = get_errno(do_sigaction(arg1, pact, &oact)); if (!is_error(ret) && arg3) { - lock_user_struct(old_act, arg3, 0); + if( lock_and_check_user_struct(&old_act,arg3,sizeof(*old_act),0,PAGE_WRITE) ) return -EFAULT; old_act->_sa_handler = oact._sa_handler; old_act->sa_flags = oact.sa_flags; old_act->sa_mask.sig[0] = oact.sa_mask.sig[0]; @@ -2914,12 +2914,14 @@ struct target_sigaction *act; struct target_sigaction *oact; - if (arg2) - lock_user_struct(act, arg2, 1); + if (arg2) { + if( lock_and_check_user_struct(&act,arg2,sizeof(*act),1,PAGE_READ) ) return -EFAULT; + } else act = NULL; - if (arg3) - lock_user_struct(oact, arg3, 0); + if (arg3) { + if( lock_and_check_user_struct(&oact,arg2,sizeof(*oact),0,PAGE_WRITE) ) return -EFAULT; + } else oact = NULL; ret = get_errno(do_sigaction(arg1, act, oact)); @@ -3046,7 +3048,7 @@ sigset_t set; ret = get_errno(sigpending(&set)); if (!is_error(ret)) { - p = lock_user(arg1, sizeof(target_sigset_t), 0); + if( lock_and_check_user_struct(&p,arg1,sizeof(target_sigset_t),0,PAGE_WRITE) ) return -EFAULT; host_to_target_sigset(p, &set); unlock_user(p, arg1, sizeof(target_sigset_t)); } @@ -3056,7 +3058,7 @@ case TARGET_NR_sigsuspend: { sigset_t set; - p = lock_user(arg1, sizeof(target_sigset_t), 1); + if( lock_and_check_user_struct(&p,arg1,sizeof(target_sigset_t),1,PAGE_READ) ) return -EFAULT; target_to_host_old_sigset(&set, p); unlock_user(p, arg1, 0); ret = get_errno(sigsuspend(&set)); @@ -3066,7 +3068,7 @@ case TARGET_NR_rt_sigsuspend: { sigset_t set; - p = lock_user(arg1, sizeof(target_sigset_t), 1); + if( lock_and_check_user_struct(&p,arg1,sizeof(target_sigset_t),1,PAGE_READ) ) return -EFAULT; target_to_host_sigset(&set, p); unlock_user(p, arg1, 0); ret = get_errno(sigsuspend(&set)); @@ -3078,18 +3080,18 @@ struct timespec uts, *puts; siginfo_t uinfo; - p = lock_user(arg1, sizeof(target_sigset_t), 1); + if( lock_and_check_user_struct(&p,arg1,sizeof(target_sigset_t),1,PAGE_READ) ) return -EFAULT; target_to_host_sigset(&set, p); unlock_user(p, arg1, 0); if (arg3) { puts = &uts; - target_to_host_timespec(puts, arg3); + if( copy_from_user_timespec(puts, (struct target_timespec *)arg3) ) return -EFAULT; } else { puts = NULL; } ret = get_errno(sigtimedwait(&set, &uinfo, puts)); if (!is_error(ret) && arg2) { - p = lock_user(arg2, sizeof(target_sigset_t), 0); + if( lock_and_check_user_struct(&p,arg2,sizeof(target_sigset_t),0,PAGE_WRITE) ) return -EFAULT; host_to_target_siginfo(p, &uinfo); unlock_user(p, arg2, sizeof(target_sigset_t)); } @@ -3098,7 +3100,7 @@ case TARGET_NR_rt_sigqueueinfo: { siginfo_t uinfo; - p = lock_user(arg3, sizeof(target_sigset_t), 1); + if( lock_and_check_user_struct(&p,arg3,sizeof(target_sigset_t),1,PAGE_READ) ) return -EFAULT; target_to_host_siginfo(&uinfo, p); unlock_user(p, arg1, 0); ret = get_errno(sys_rt_sigqueueinfo(arg1, arg2, &uinfo)); @@ -3153,7 +3155,7 @@ struct rusage rusage; ret = get_errno(getrusage(arg1, &rusage)); if (!is_error(ret)) { - host_to_target_rusage(arg2, &rusage); + if( copy_to_user_rusage((struct target_rusage *)arg2, &rusage) ) return -EFAULT; } } break; @@ -3162,14 +3164,14 @@ struct timeval tv; ret = get_errno(gettimeofday(&tv, NULL)); if (!is_error(ret)) { - host_to_target_timeval(arg1, &tv); + if( copy_to_user_timeval((struct target_timeval *)arg1, &tv) ) return -EFAULT; } } break; case TARGET_NR_settimeofday: { struct timeval tv; - target_to_host_timeval(&tv, arg1); + if( copy_from_user_timeval(&tv, (struct target_timeval *)arg1) ) return -EFAULT; ret = get_errno(settimeofday(&tv, NULL)); } break; @@ -3331,24 +3333,24 @@ #endif case TARGET_NR_statfs: p = lock_user_string(arg1); + if( !access_ok(VERIFY_READ,p,1) ) return -EFAULT; ret = get_errno(statfs(path(p), &stfs)); unlock_user(p, arg1, 0); convert_statfs: if (!is_error(ret)) { - struct target_statfs *target_stfs; + struct target_statfs *target_stfs = (struct target_statfs *)arg2; - lock_user_struct(target_stfs, arg2, 0); - /* ??? put_user is probably wrong. */ - put_user(stfs.f_type, &target_stfs->f_type); - put_user(stfs.f_bsize, &target_stfs->f_bsize); - put_user(stfs.f_blocks, &target_stfs->f_blocks); - put_user(stfs.f_bfree, &target_stfs->f_bfree); - put_user(stfs.f_bavail, &target_stfs->f_bavail); - put_user(stfs.f_files, &target_stfs->f_files); - put_user(stfs.f_ffree, &target_stfs->f_ffree); - put_user(stfs.f_fsid.__val[0], &target_stfs->f_fsid); - put_user(stfs.f_namelen, &target_stfs->f_namelen); - unlock_user_struct(target_stfs, arg2, 1); + if( !access_ok(VERIFY_WRITE,target_stfs,sizeof(struct target_statfs)) ) return -EFAULT; + /* use __put_user since we just validated the entire bufffer. */ + __put_user(stfs.f_type, &target_stfs->f_type); + __put_user(stfs.f_bsize, &target_stfs->f_bsize); + __put_user(stfs.f_blocks, &target_stfs->f_blocks); + __put_user(stfs.f_bfree, &target_stfs->f_bfree); + __put_user(stfs.f_bavail, &target_stfs->f_bavail); + __put_user(stfs.f_files, &target_stfs->f_files); + __put_user(stfs.f_ffree, &target_stfs->f_ffree); + __put_user(stfs.f_fsid.__val[0], &target_stfs->f_fsid); + __put_user(stfs.f_namelen, &target_stfs->f_namelen); } break; case TARGET_NR_fstatfs: @@ -3357,24 +3359,24 @@ #ifdef TARGET_NR_statfs64 case TARGET_NR_statfs64: p = lock_user_string(arg1); + if( !access_ok(VERIFY_READ,p,1) ) return -EFAULT; ret = get_errno(statfs(path(p), &stfs)); unlock_user(p, arg1, 0); convert_statfs64: if (!is_error(ret)) { - struct target_statfs64 *target_stfs; + struct target_statfs64 *target_stfs = (struct target_statfs64 *)arg3; - lock_user_struct(target_stfs, arg3, 0); - /* ??? put_user is probably wrong. */ - put_user(stfs.f_type, &target_stfs->f_type); - put_user(stfs.f_bsize, &target_stfs->f_bsize); - put_user(stfs.f_blocks, &target_stfs->f_blocks); - put_user(stfs.f_bfree, &target_stfs->f_bfree); - put_user(stfs.f_bavail, &target_stfs->f_bavail); - put_user(stfs.f_files, &target_stfs->f_files); - put_user(stfs.f_ffree, &target_stfs->f_ffree); - put_user(stfs.f_fsid.__val[0], &target_stfs->f_fsid); - put_user(stfs.f_namelen, &target_stfs->f_namelen); - unlock_user_struct(target_stfs, arg3, 0); + if( !access_ok(VERIFY_WRITE,target_stfs,sizeof(struct target_statfs64)) ) return -EFAULT; + /* use __put_user since we just validated the entire bufffer. */ + __put_user(stfs.f_type, &target_stfs->f_type); + __put_user(stfs.f_bsize, &target_stfs->f_bsize); + __put_user(stfs.f_blocks, &target_stfs->f_blocks); + __put_user(stfs.f_bfree, &target_stfs->f_bfree); + __put_user(stfs.f_bavail, &target_stfs->f_bavail); + __put_user(stfs.f_files, &target_stfs->f_files); + __put_user(stfs.f_ffree, &target_stfs->f_ffree); + __put_user(stfs.f_fsid.__val[0], &target_stfs->f_fsid); + __put_user(stfs.f_namelen, &target_stfs->f_namelen); } break; case TARGET_NR_fstatfs64: @@ -3392,7 +3394,7 @@ #endif #ifdef TARGET_NR_accept case TARGET_NR_accept: - ret = do_accept(arg1, arg2, arg3); + ret = do_accept(arg1, arg2, (socklen_t *)arg3); break; #endif #ifdef TARGET_NR_bind @@ -3407,17 +3409,17 @@ #endif #ifdef TARGET_NR_getpeername case TARGET_NR_getpeername: - ret = do_getpeername(arg1, arg2, arg3); + ret = do_getpeername(arg1, arg2, (socklen_t *)arg3); break; #endif #ifdef TARGET_NR_getsockname case TARGET_NR_getsockname: - ret = do_getsockname(arg1, arg2, arg3); + ret = do_getsockname(arg1, arg2, (socklen_t *)arg3); break; #endif #ifdef TARGET_NR_getsockopt case TARGET_NR_getsockopt: - ret = do_getsockopt(arg1, arg2, arg3, arg4, arg5); + ret = do_getsockopt(arg1, arg2, arg3, arg4, (socklen_t *)arg5); break; #endif #ifdef TARGET_NR_listen @@ -3488,19 +3490,20 @@ if (arg2) { pvalue = &value; - target_to_host_timeval(&pvalue->it_interval, - arg2); - target_to_host_timeval(&pvalue->it_value, - arg2 + sizeof(struct target_timeval)); + if( copy_from_user_timeval(&pvalue->it_interval, (struct target_timeval *)arg2) ) + return -EFAULT; + if( copy_from_user_timeval(&pvalue->it_value, + (struct target_timeval *)(arg2 + sizeof(struct target_timeval))) ) + return -EFAULT; } else { pvalue = NULL; } ret = get_errno(setitimer(arg1, pvalue, &ovalue)); if (!is_error(ret) && arg3) { - host_to_target_timeval(arg3, - &ovalue.it_interval); - host_to_target_timeval(arg3 + sizeof(struct target_timeval), - &ovalue.it_value); + if( copy_to_user_timeval((struct target_timeval *)arg3, + &ovalue.it_interval) ) return -EFAULT; + if( copy_to_user_timeval((struct target_timeval *)(arg3 + sizeof(struct target_timeval)), + &ovalue.it_value) ) return -EFAULT; } } break; @@ -3510,20 +3513,22 @@ ret = get_errno(getitimer(arg1, &value)); if (!is_error(ret) && arg2) { - host_to_target_timeval(arg2, - &value.it_interval); - host_to_target_timeval(arg2 + sizeof(struct target_timeval), - &value.it_value); + if( copy_to_user_timeval((struct target_timeval *)arg2, + &value.it_interval) ) return -EFAULT; + if( copy_to_user_timeval((struct target_timeval *)(arg2 + sizeof(struct target_timeval)), + &value.it_value) ) return -EFAULT; } } break; case TARGET_NR_stat: p = lock_user_string(arg1); + if( !access_ok(VERIFY_READ,p,1) ) return -EFAULT; ret = get_errno(stat(path(p), &st)); unlock_user(p, arg1, 0); goto do_stat; case TARGET_NR_lstat: p = lock_user_string(arg1); + if( !access_ok(VERIFY_READ,p,1) ) return -EFAULT; ret = get_errno(lstat(path(p), &st)); unlock_user(p, arg1, 0); goto do_stat; @@ -3534,7 +3539,7 @@ if (!is_error(ret)) { struct target_stat *target_st; - lock_user_struct(target_st, arg2, 0); + if( lock_and_check_user_struct(&target_st,arg2,sizeof(*target_st),9,PAGE_WRITE) ) return -EFAULT; #if defined(TARGET_MIPS) target_st->st_dev = tswap32(st.st_dev); #else @@ -3593,7 +3598,7 @@ int status; target_long status_ptr = arg2; struct rusage rusage, *rusage_ptr; - target_ulong target_rusage = arg4; + struct target_rusage *target_rusage = (struct target_rusage *)arg4; if (target_rusage) rusage_ptr = &rusage; else @@ -3603,7 +3608,8 @@ if (status_ptr) tputl(status_ptr, status); if (target_rusage) { - host_to_target_rusage(target_rusage, &rusage); + if( !copy_to_user_rusage(target_rusage, &rusage) ) + return -EFAULT; } } } @@ -3965,17 +3971,17 @@ struct timespec ts; ret = get_errno(sched_rr_get_interval(arg1, &ts)); if (!is_error(ret)) { - host_to_target_timespec(arg2, &ts); + if( copy_to_user_timespec((struct target_timespec *)arg2, &ts) ) return -EFAULT; } } break; case TARGET_NR_nanosleep: { struct timespec req, rem; - target_to_host_timespec(&req, arg1); + if( copy_from_user_timespec(&req, (struct target_timespec *)arg1) ) return -EFAULT; ret = get_errno(nanosleep(&req, &rem)); if (is_error(ret) && arg2) { - host_to_target_timespec(arg2, &rem); + if( copy_to_user_timespec((struct target_timespec *)arg2, &rem) ) return -EFAULT; } } break; @@ -4072,6 +4078,7 @@ #ifdef TARGET_NR_stat64 case TARGET_NR_stat64: p = lock_user_string(arg1); + if( !access_ok(VERIFY_READ,p,1) ) return -EFAULT; ret = get_errno(stat(path(p), &st)); unlock_user(p, arg1, 0); goto do_stat64; @@ -4079,6 +4086,7 @@ #ifdef TARGET_NR_lstat64 case TARGET_NR_lstat64: p = lock_user_string(arg1); + if( !access_ok(VERIFY_READ,p,1) ) return -EFAULT; ret = get_errno(lstat(path(p), &st)); unlock_user(p, arg1, 0); goto do_stat64; @@ -4091,53 +4099,51 @@ if (!is_error(ret)) { #ifdef TARGET_ARM if (((CPUARMState *)cpu_env)->eabi) { - struct target_eabi_stat64 *target_st; - lock_user_struct(target_st, arg2, 1); + struct target_eabi_stat64 *target_st = (struct target_eabi_stat64 *)arg2; + if( !access_ok(VERIFY_WRITE,target_st,sizeof(struct target_eabi_stat64)) ) return -EFAULT; memset(target_st, 0, sizeof(struct target_eabi_stat64)); - /* put_user is probably wrong. */ - put_user(st.st_dev, &target_st->st_dev); - put_user(st.st_ino, &target_st->st_ino); + /* use __put_user() since we just checked that the buffer is valid */ + __put_user(st.st_dev, &target_st->st_dev); + __put_user(st.st_ino, &target_st->st_ino); #ifdef TARGET_STAT64_HAS_BROKEN_ST_INO - put_user(st.st_ino, &target_st->__st_ino); + __put_user(st.st_ino, &target_st->__st_ino); #endif - put_user(st.st_mode, &target_st->st_mode); - put_user(st.st_nlink, &target_st->st_nlink); - put_user(st.st_uid, &target_st->st_uid); - put_user(st.st_gid, &target_st->st_gid); - put_user(st.st_rdev, &target_st->st_rdev); + __put_user(st.st_mode, &target_st->st_mode); + __put_user(st.st_nlink, &target_st->st_nlink); + __put_user(st.st_uid, &target_st->st_uid); + __put_user(st.st_gid, &target_st->st_gid); + __put_user(st.st_rdev, &target_st->st_rdev); /* XXX: better use of kernel struct */ - put_user(st.st_size, &target_st->st_size); - put_user(st.st_blksize, &target_st->st_blksize); - put_user(st.st_blocks, &target_st->st_blocks); - put_user(st.st_atime, &target_st->target_st_atime); - put_user(st.st_mtime, &target_st->target_st_mtime); - put_user(st.st_ctime, &target_st->target_st_ctime); - unlock_user_struct(target_st, arg2, 0); + __put_user(st.st_size, &target_st->st_size); + __put_user(st.st_blksize, &target_st->st_blksize); + __put_user(st.st_blocks, &target_st->st_blocks); + __put_user(st.st_atime, &target_st->target_st_atime); + __put_user(st.st_mtime, &target_st->target_st_mtime); + __put_user(st.st_ctime, &target_st->target_st_ctime); } else #endif { - struct target_stat64 *target_st; - lock_user_struct(target_st, arg2, 1); + struct target_stat64 *target_st = (struct target_stat64 *)arg2; + if( !access_ok(VERIFY_WRITE,target_st,sizeof(struct target_stat64)) ) return -EFAULT; memset(target_st, 0, sizeof(struct target_stat64)); - /* ??? put_user is probably wrong. */ - put_user(st.st_dev, &target_st->st_dev); - put_user(st.st_ino, &target_st->st_ino); + /* use __put_user() since we just checked that the buffer is valid */ + __put_user(st.st_dev, &target_st->st_dev); + __put_user(st.st_ino, &target_st->st_ino); #ifdef TARGET_STAT64_HAS_BROKEN_ST_INO - put_user(st.st_ino, &target_st->__st_ino); + __put_user(st.st_ino, &target_st->__st_ino); #endif - put_user(st.st_mode, &target_st->st_mode); - put_user(st.st_nlink, &target_st->st_nlink); - put_user(st.st_uid, &target_st->st_uid); - put_user(st.st_gid, &target_st->st_gid); - put_user(st.st_rdev, &target_st->st_rdev); + __put_user(st.st_mode, &target_st->st_mode); + __put_user(st.st_nlink, &target_st->st_nlink); + __put_user(st.st_uid, &target_st->st_uid); + __put_user(st.st_gid, &target_st->st_gid); + __put_user(st.st_rdev, &target_st->st_rdev); /* XXX: better use of kernel struct */ - put_user(st.st_size, &target_st->st_size); - put_user(st.st_blksize, &target_st->st_blksize); - put_user(st.st_blocks, &target_st->st_blocks); - put_user(st.st_atime, &target_st->target_st_atime); - put_user(st.st_mtime, &target_st->target_st_mtime); - put_user(st.st_ctime, &target_st->target_st_ctime); - unlock_user_struct(target_st, arg2, 0); + __put_user(st.st_size, &target_st->st_size); + __put_user(st.st_blksize, &target_st->st_blksize); + __put_user(st.st_blocks, &target_st->st_blocks); + __put_user(st.st_atime, &target_st->target_st_atime); + __put_user(st.st_mtime, &target_st->target_st_mtime); + __put_user(st.st_ctime, &target_st->target_st_ctime); } } } @@ -4583,7 +4589,7 @@ struct timespec ts; ret = get_errno(clock_gettime(arg1, &ts)); if (!is_error(ret)) { - host_to_target_timespec(arg2, &ts); + if( copy_to_user_timespec((struct target_timespec *)arg2, &ts)) return -EFAULT; } break; } @@ -4594,7 +4600,7 @@ struct timespec ts; ret = get_errno(clock_getres(arg1, &ts)); if (!is_error(ret)) { - host_to_target_timespec(arg2, &ts); + if( copy_to_user_timespec((struct target_timespec *)arg2, &ts) ) return -EFAULT; } break; } Index: qemu/linux-user/qemu.h =================================================================== --- qemu.orig/linux-user/qemu.h 2007-07-20 13:43:58.000000000 -0400 +++ qemu/linux-user/qemu.h 2007-07-20 14:52:49.000000000 -0400 @@ -171,7 +171,8 @@ #define VERIFY_READ 0 #define VERIFY_WRITE 1 -#define access_ok(type,addr,size) (1) +#define access_ok(type,addr,size) \ + (page_check_range((target_ulong)addr,size,(type==VERIFY_READ)?PAGE_READ:PAGE_WRITE)==0) /* NOTE get_user and put_user use host addresses. */ #define __put_user(x,ptr)\ @@ -238,6 +239,26 @@ __ret;\ }) +#define copy_from_user(hptr, gptr, len) \ +({ \ + int __cfu_ret=0;\ + if( access_ok(VERIFY_READ, gptr, (len) ) )\ + memcpy(hptr,gptr,(len)); \ + else \ + __cfu_ret=1;\ + __cfu_ret; \ +}) + +#define copy_to_user(gptr, hptr, len) \ +({ \ + int __ctu_ret=0;\ + if( access_ok(VERIFY_WRITE, gptr, (len)) )\ + memcpy(gptr,hptr,(len)); \ + else \ + __ctu_ret=1;\ + __ctu_ret; \ +}) + /* Functions for accessing guest memory. The tget and tput functions read/write single values, byteswapping as neccessary. The lock_user gets a pointer to a contiguous area of guest memory, but does not perform @@ -295,6 +316,16 @@ #define unlock_user_struct(host_ptr, guest_addr, copy) \ unlock_user(host_ptr, guest_addr, (copy) ? sizeof(*host_ptr) : 0) +/* This is being phased out in favor of the copy*user() interfaces */ +static inline long lock_and_check_user_struct(void *host_addr, target_ulong guest_addr, long len, int copy, int pg_access) +{ + long *haddr, ret = 0; + haddr = (long *)host_addr; + if( (ret=page_check_range(guest_addr,len,pg_access)) != 0 ) return ret; + *haddr = (long)lock_user(guest_addr, len, copy); + return 0; +} + #define tget8(addr) ldub(addr) #define tput8(addr, val) stb(addr, val) #define tget16(addr) lduw(addr) ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2007-07-20 20:16 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2007-07-06 15:15 [Qemu-devel] [PATCH] linux-user EFAULT implementation Stuart Anderson 2007-07-06 16:08 ` Fabrice Bellard 2007-07-06 16:55 ` Stuart Anderson 2007-07-06 18:45 ` Stuart Anderson 2007-07-09 11:41 ` Stuart Anderson 2007-07-09 12:02 ` Stuart Anderson 2007-07-09 21:15 ` Fabrice Bellard 2007-07-10 2:22 ` Stuart Anderson 2007-07-10 12:38 ` Paul Brook 2007-07-10 20:45 ` Fabrice Bellard 2007-07-10 21:09 ` Stuart Anderson 2007-07-13 17:55 ` Stuart Anderson 2007-07-20 20:16 ` Stuart Anderson
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).