From: Thayne Harbaugh <thayne@c2.net>
To: qemu-devel <qemu-devel@nongnu.org>
Subject: [Qemu-devel] Re: [PATCH] efault - add data type to put_user()/get_user()
Date: Fri, 02 Nov 2007 17:26:23 -0600 [thread overview]
Message-ID: <1194045983.2168.17.camel@phantasm.home.enterpriseandprosperity.com> (raw)
In-Reply-To: <1193870631.19343.51.camel@phantasm.home.enterpriseandprosperity.com>
[-- Attachment #1: Type: text/plain, Size: 852 bytes --]
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.
[-- Attachment #2: 06_efault.patch.1.3 --]
[-- Type: text/x-patch, Size: 10443 bytes --]
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);
}
}
next prev parent reply other threads:[~2007-11-02 23:34 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2007-10-31 22:30 [Qemu-devel] [PATCH] efault - verify pages are in cache and are read/write Thayne Harbaugh
2007-10-31 22:35 ` [Qemu-devel] Re: [PATCH] efault - update __get_user() __put_user() Thayne Harbaugh
2007-10-31 22:43 ` [Qemu-devel] Re: [PATCH] efault - add data type to put_user()/get_user() Thayne Harbaugh
2007-11-02 23:26 ` Thayne Harbaugh [this message]
2007-11-03 15:56 ` Thiemo Seufer
2007-11-03 19:05 ` Fabrice Bellard
2007-11-05 20:22 ` Thayne Harbaugh
2007-11-05 20:43 ` Thayne Harbaugh
2007-11-05 21:42 ` Fabrice Bellard
2007-11-05 23:00 ` Thayne Harbaugh
2007-11-05 23:33 ` Fabrice Bellard
2007-10-31 22:49 ` [Qemu-devel] Re: [PATCH] efault Thayne Harbaugh
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1194045983.2168.17.camel@phantasm.home.enterpriseandprosperity.com \
--to=thayne@c2.net \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).