From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1Io61v-0007xK-MX for qemu-devel@nongnu.org; Fri, 02 Nov 2007 19:34:23 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1Io61u-0007ws-V7 for qemu-devel@nongnu.org; Fri, 02 Nov 2007 19:34:23 -0400 Received: from [199.232.76.173] (helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Io61u-0007wj-Fg for qemu-devel@nongnu.org; Fri, 02 Nov 2007 19:34:22 -0400 Received: from owa.c2.net ([207.235.78.2] helo=email.c2.net) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1Io61u-0003RP-1I for qemu-devel@nongnu.org; Fri, 02 Nov 2007 19:34:22 -0400 From: Thayne Harbaugh In-Reply-To: <1193870631.19343.51.camel@phantasm.home.enterpriseandprosperity.com> References: <1193869827.19343.38.camel@phantasm.home.enterpriseandprosperity.com> <1193870136.19343.43.camel@phantasm.home.enterpriseandprosperity.com> <1193870631.19343.51.camel@phantasm.home.enterpriseandprosperity.com> Content-Type: multipart/mixed; boundary="=-m0DGUgd1UEpyrKkHntg2" Date: Fri, 02 Nov 2007 17:26:23 -0600 Message-Id: <1194045983.2168.17.camel@phantasm.home.enterpriseandprosperity.com> Mime-Version: 1.0 Subject: [Qemu-devel] Re: [PATCH] efault - add data type to put_user()/get_user() Reply-To: thayne@c2.net, qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel --=-m0DGUgd1UEpyrKkHntg2 Content-Type: text/plain Content-Transfer-Encoding: 7bit On Wed, 2007-10-31 at 16:44 -0600, Thayne Harbaugh wrote: > This patch updates get_user() and put_user() to take a third argument of > data type. get_user() and put_user() use target address which are > target_ulong and don't reflect the data type pointed to in target > memory. > > Simply casting the target_ulong to a type before passing to > get/put_user() is poor because target_ulong isn't always a simple cast > to a host type (consider 32 bit on 64 bit where address are either > extended or truncate). Also, simple casting of the argument to > get/put_user() results in several warnings when target and long pointer > sizes don't match. > > This patch has additional updates to fix places where get/put_user() are > already used. This is an updated patch that doesn't conflict with the abi_long/abi_ulong changes from a couple weeks ago. --=-m0DGUgd1UEpyrKkHntg2 Content-Disposition: attachment; filename=06_efault.patch.1.3 Content-Type: text/x-patch; name=06_efault.patch.1.3; charset=utf-8 Content-Transfer-Encoding: 7bit Index: qemu/linux-user/qemu.h =================================================================== --- qemu.orig/linux-user/qemu.h 2007-10-30 07:56:57.000000000 -0600 +++ qemu/linux-user/qemu.h 2007-10-30 17:55:57.000000000 -0600 @@ -230,26 +231,47 @@ 0;\ }) -#define put_user(x,ptr)\ +/* put_user()/get_user() take a guest address and check access */ +#define put_user(x, gaddr, target_type)\ ({\ int __ret;\ - if (access_ok(VERIFY_WRITE, ptr, sizeof(*ptr)))\ - __ret = __put_user(x, ptr);\ + if (access_ok(VERIFY_WRITE, gaddr, sizeof(target_type)))\ + __ret = __put_user(x, (target_type)g2h(gaddr)); \ else\ - __ret = -EFAULT;\ + __ret = -TARGET_EFAULT;\ __ret;\ }) -#define get_user(x,ptr)\ +#define get_user(x, gaddr, target_type)\ ({\ int __ret;\ - if (access_ok(VERIFY_READ, ptr, sizeof(*ptr)))\ - __ret = __get_user(x, ptr);\ + if (access_ok(VERIFY_READ, gaddr, sizeof(target_type)))\ + __ret = __get_user(x, (target_type)g2h(gaddr)); \ else\ - __ret = -EFAULT;\ + __ret = -TARGET_EFAULT;\ __ret;\ }) +#define copy_from_user(hptr, gaddr, len) \ +({ \ + int __cfu_ret=0; \ + if (access_ok(VERIFY_READ, gaddr, (len))) \ + memcpy(hptr, g2h(gaddr), (len)); \ + else \ + __cfu_ret = -TARGET_EFAULT; \ + __cfu_ret; \ +}) + +#define copy_to_user(gaddr, hptr, len) \ +({ \ + int __ctu_ret=0; \ + if (access_ok(VERIFY_WRITE, gaddr, (len))) \ + memcpy(g2h(gaddr), hptr, (len)); \ + else \ + __ctu_ret = -TARGET_EFAULT; \ + __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 Index: qemu/linux-user/syscall.c =================================================================== --- qemu.orig/linux-user/syscall.c 2007-10-30 07:56:57.000000000 -0600 +++ qemu/linux-user/syscall.c 2007-10-30 17:56:06.000000000 -0600 @@ -1757,7 +1757,7 @@ break; } } - if (put_user(raddr, (abi_ulong *)third)) + if (put_user(raddr, third, abi_ulong *)) return -TARGET_EFAULT; ret = 0; break; @@ -3689,17 +3689,16 @@ struct target_statfs *target_stfs; 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.val[0]); - put_user(stfs.f_fsid.__val[1], &target_stfs->f_fsid.val[1]); - put_user(stfs.f_namelen, &target_stfs->f_namelen); + __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.val[0]); + __put_user(stfs.f_fsid.__val[1], &target_stfs->f_fsid.val[1]); + __put_user(stfs.f_namelen, &target_stfs->f_namelen); unlock_user_struct(target_stfs, arg2, 1); } break; @@ -3716,17 +3715,16 @@ struct target_statfs64 *target_stfs; 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.val[0]); - put_user(stfs.f_fsid.__val[1], &target_stfs->f_fsid.val[1]); - put_user(stfs.f_namelen, &target_stfs->f_namelen); + __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.val[0]); + __put_user(stfs.f_fsid.__val[1], &target_stfs->f_fsid.val[1]); + __put_user(stfs.f_namelen, &target_stfs->f_namelen); unlock_user_struct(target_stfs, arg3, 0); } break; @@ -4465,24 +4463,22 @@ struct target_eabi_stat64 *target_st; lock_user_struct(target_st, arg2, 1); 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); + __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); - /* 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); + __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_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); } else #endif @@ -4490,24 +4486,23 @@ struct target_stat64 *target_st; lock_user_struct(target_st, arg2, 1); 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); + __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); + __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); } } --=-m0DGUgd1UEpyrKkHntg2--