* [Qemu-devel] [PATCH v2 0/3] Use safe_syscall wrapper for fcntl()
@ 2016-06-09 14:09 Peter Maydell
2016-06-09 14:09 ` [Qemu-devel] [PATCH v2 1/3] linux-user: Avoid possible misalignment in host_to_target_siginfo() Peter Maydell
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Peter Maydell @ 2016-06-09 14:09 UTC (permalink / raw)
To: qemu-devel; +Cc: patches, Laurent Vivier, Riku Voipio
This is a fixed respin of the three patches which didn't make
it through review from my last set of linux-user patches.
Changes v1->v2:
* add a memset() to host_to_target_siginfo_noswap() to suppress
bogus gcc warnings about struct fields being maybe used while
uninitialized
* pass the error returned from copy_to/from_user_flock() etc out to
the guest rather than assuming it's always -TARGET_EFAULT
* added a note to the commit message about where the bogus shift
operations in conversion of l_type were from
thanks
-- PMM
Peter Maydell (3):
linux-user: Avoid possible misalignment in host_to_target_siginfo()
linux-user: Use __get_user() and __put_user() to handle structs in
do_fcntl()
linux-user: Use safe_syscall wrapper for fcntl
linux-user/signal.c | 13 +-
linux-user/syscall.c | 328 +++++++++++++++++++++++++++++----------------------
2 files changed, 198 insertions(+), 143 deletions(-)
--
1.9.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Qemu-devel] [PATCH v2 1/3] linux-user: Avoid possible misalignment in host_to_target_siginfo()
2016-06-09 14:09 [Qemu-devel] [PATCH v2 0/3] Use safe_syscall wrapper for fcntl() Peter Maydell
@ 2016-06-09 14:09 ` Peter Maydell
2016-06-09 19:27 ` Laurent Vivier
2016-06-09 14:09 ` [Qemu-devel] [PATCH v2 2/3] linux-user: Use __get_user() and __put_user() to handle structs in do_fcntl() Peter Maydell
2016-06-09 14:09 ` [Qemu-devel] [PATCH v2 3/3] linux-user: Use safe_syscall wrapper for fcntl Peter Maydell
2 siblings, 1 reply; 7+ messages in thread
From: Peter Maydell @ 2016-06-09 14:09 UTC (permalink / raw)
To: qemu-devel; +Cc: patches, Laurent Vivier, Riku Voipio
host_to_target_siginfo() is implemented by a combination of
host_to_target_siginfo_noswap() followed by tswap_siginfo().
The first of these two functions assumes that the target_siginfo_t
it is writing to is correctly aligned, but the pointer passed
into host_to_target_siginfo() is directly from the guest and
might be misaligned. Use a local variable to avoid this problem.
(tswap_siginfo() does now correctly handle a misaligned destination.)
We have to add a memset() to host_to_target_siginfo_noswap()
to avoid some false positive "may be used uninitialized" warnings
from gcc about subfields of the _sifields union if it chooses to
inline both tswap_siginfo() and host_to_target_siginfo_noswap()
into host_to_target_siginfo().
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
linux-user/signal.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/linux-user/signal.c b/linux-user/signal.c
index 61c1145..37fb60f 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -281,6 +281,14 @@ static inline void host_to_target_siginfo_noswap(target_siginfo_t *tinfo,
tinfo->si_errno = 0;
tinfo->si_code = info->si_code;
+ /* This memset serves two purposes:
+ * (1) ensure we don't leak random junk to the guest later
+ * (2) placate false positives from gcc about fields
+ * being used uninitialized if it chooses to inline both this
+ * function and tswap_siginfo() into host_to_target_siginfo().
+ */
+ memset(tinfo->_sifields._pad, 0, sizeof(tinfo->_sifields._pad));
+
/* This is awkward, because we have to use a combination of
* the si_code and si_signo to figure out which of the union's
* members are valid. (Within the host kernel it is always possible
@@ -400,8 +408,9 @@ static void tswap_siginfo(target_siginfo_t *tinfo,
void host_to_target_siginfo(target_siginfo_t *tinfo, const siginfo_t *info)
{
- host_to_target_siginfo_noswap(tinfo, info);
- tswap_siginfo(tinfo, tinfo);
+ target_siginfo_t tgt_tmp;
+ host_to_target_siginfo_noswap(&tgt_tmp, info);
+ tswap_siginfo(tinfo, &tgt_tmp);
}
/* XXX: we support only POSIX RT signals are used. */
--
1.9.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [Qemu-devel] [PATCH v2 2/3] linux-user: Use __get_user() and __put_user() to handle structs in do_fcntl()
2016-06-09 14:09 [Qemu-devel] [PATCH v2 0/3] Use safe_syscall wrapper for fcntl() Peter Maydell
2016-06-09 14:09 ` [Qemu-devel] [PATCH v2 1/3] linux-user: Avoid possible misalignment in host_to_target_siginfo() Peter Maydell
@ 2016-06-09 14:09 ` Peter Maydell
2016-06-09 19:27 ` Laurent Vivier
2016-06-09 14:09 ` [Qemu-devel] [PATCH v2 3/3] linux-user: Use safe_syscall wrapper for fcntl Peter Maydell
2 siblings, 1 reply; 7+ messages in thread
From: Peter Maydell @ 2016-06-09 14:09 UTC (permalink / raw)
To: qemu-devel; +Cc: patches, Laurent Vivier, Riku Voipio
Use the __get_user() and __put_user() to handle reading and writing the
guest structures in do_ioctl(). This has two benefits:
* avoids possible errors due to misaligned guest pointers
* correctly sign extends signed fields (like l_start in struct flock)
which might be different sizes between guest and host
To do this we abstract out into copy_from/to_user functions. We
also standardize on always using host flock64 and the F_GETLK64
etc flock commands, as this means we always have 64 bit offsets
whether the host is 64-bit or 32-bit and we don't need to support
conversion to both host struct flock and struct flock64.
In passing we fix errors in converting l_type from the host to
the target (where we were doing a byteswap of the host value
before trying to do the convert-bitmasks operation rather than
otherwise, and inexplicably shifting left by 1); these were
accidentally left over when the original simple "just shift by 1"
arm<->x86 conversion of commit 43f238d was changed to the more
general scheme of using target_to_host_bitmask() functions in 2ba7f73.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
linux-user/syscall.c | 298 ++++++++++++++++++++++++++++-----------------------
1 file changed, 166 insertions(+), 132 deletions(-)
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 71ccbd9..78c36dd 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -5542,11 +5542,11 @@ static int target_to_host_fcntl_cmd(int cmd)
case TARGET_F_SETFL:
return cmd;
case TARGET_F_GETLK:
- return F_GETLK;
- case TARGET_F_SETLK:
- return F_SETLK;
- case TARGET_F_SETLKW:
- return F_SETLKW;
+ return F_GETLK64;
+ case TARGET_F_SETLK:
+ return F_SETLK64;
+ case TARGET_F_SETLKW:
+ return F_SETLKW64;
case TARGET_F_GETOWN:
return F_GETOWN;
case TARGET_F_SETOWN:
@@ -5597,12 +5597,134 @@ static const bitmask_transtbl flock_tbl[] = {
{ 0, 0, 0, 0 }
};
-static abi_long do_fcntl(int fd, int cmd, abi_ulong arg)
+static inline abi_long copy_from_user_flock(struct flock64 *fl,
+ abi_ulong target_flock_addr)
{
- struct flock fl;
struct target_flock *target_fl;
+ short l_type;
+
+ if (!lock_user_struct(VERIFY_READ, target_fl, target_flock_addr, 1)) {
+ return -TARGET_EFAULT;
+ }
+
+ __get_user(l_type, &target_fl->l_type);
+ fl->l_type = target_to_host_bitmask(l_type, flock_tbl);
+ __get_user(fl->l_whence, &target_fl->l_whence);
+ __get_user(fl->l_start, &target_fl->l_start);
+ __get_user(fl->l_len, &target_fl->l_len);
+ __get_user(fl->l_pid, &target_fl->l_pid);
+ unlock_user_struct(target_fl, target_flock_addr, 0);
+ return 0;
+}
+
+static inline abi_long copy_to_user_flock(abi_ulong target_flock_addr,
+ const struct flock64 *fl)
+{
+ struct target_flock *target_fl;
+ short l_type;
+
+ if (!lock_user_struct(VERIFY_WRITE, target_fl, target_flock_addr, 0)) {
+ return -TARGET_EFAULT;
+ }
+
+ l_type = host_to_target_bitmask(fl->l_type, flock_tbl);
+ __put_user(l_type, &target_fl->l_type);
+ __put_user(fl->l_whence, &target_fl->l_whence);
+ __put_user(fl->l_start, &target_fl->l_start);
+ __put_user(fl->l_len, &target_fl->l_len);
+ __put_user(fl->l_pid, &target_fl->l_pid);
+ unlock_user_struct(target_fl, target_flock_addr, 1);
+ return 0;
+}
+
+typedef abi_long from_flock64_fn(struct flock64 *fl, abi_ulong target_addr);
+typedef abi_long to_flock64_fn(abi_ulong target_addr, const struct flock64 *fl);
+
+#ifdef TARGET_ARM
+static inline abi_long copy_from_user_eabi_flock64(struct flock64 *fl,
+ abi_ulong target_flock_addr)
+{
+ struct target_eabi_flock64 *target_fl;
+ short l_type;
+
+ if (!lock_user_struct(VERIFY_READ, target_fl, target_flock_addr, 1)) {
+ return -TARGET_EFAULT;
+ }
+
+ __get_user(l_type, &target_fl->l_type);
+ fl->l_type = target_to_host_bitmask(l_type, flock_tbl);
+ __get_user(fl->l_whence, &target_fl->l_whence);
+ __get_user(fl->l_start, &target_fl->l_start);
+ __get_user(fl->l_len, &target_fl->l_len);
+ __get_user(fl->l_pid, &target_fl->l_pid);
+ unlock_user_struct(target_fl, target_flock_addr, 0);
+ return 0;
+}
+
+static inline abi_long copy_to_user_eabi_flock64(abi_ulong target_flock_addr,
+ const struct flock64 *fl)
+{
+ struct target_eabi_flock64 *target_fl;
+ short l_type;
+
+ if (!lock_user_struct(VERIFY_WRITE, target_fl, target_flock_addr, 0)) {
+ return -TARGET_EFAULT;
+ }
+
+ l_type = host_to_target_bitmask(fl->l_type, flock_tbl);
+ __put_user(l_type, &target_fl->l_type);
+ __put_user(fl->l_whence, &target_fl->l_whence);
+ __put_user(fl->l_start, &target_fl->l_start);
+ __put_user(fl->l_len, &target_fl->l_len);
+ __put_user(fl->l_pid, &target_fl->l_pid);
+ unlock_user_struct(target_fl, target_flock_addr, 1);
+ return 0;
+}
+#endif
+
+static inline abi_long copy_from_user_flock64(struct flock64 *fl,
+ abi_ulong target_flock_addr)
+{
+ struct target_flock64 *target_fl;
+ short l_type;
+
+ if (!lock_user_struct(VERIFY_READ, target_fl, target_flock_addr, 1)) {
+ return -TARGET_EFAULT;
+ }
+
+ __get_user(l_type, &target_fl->l_type);
+ fl->l_type = target_to_host_bitmask(l_type, flock_tbl);
+ __get_user(fl->l_whence, &target_fl->l_whence);
+ __get_user(fl->l_start, &target_fl->l_start);
+ __get_user(fl->l_len, &target_fl->l_len);
+ __get_user(fl->l_pid, &target_fl->l_pid);
+ unlock_user_struct(target_fl, target_flock_addr, 0);
+ return 0;
+}
+
+static inline abi_long copy_to_user_flock64(abi_ulong target_flock_addr,
+ const struct flock64 *fl)
+{
+ struct target_flock64 *target_fl;
+ short l_type;
+
+ if (!lock_user_struct(VERIFY_WRITE, target_fl, target_flock_addr, 0)) {
+ return -TARGET_EFAULT;
+ }
+
+ l_type = host_to_target_bitmask(fl->l_type, flock_tbl);
+ __put_user(l_type, &target_fl->l_type);
+ __put_user(fl->l_whence, &target_fl->l_whence);
+ __put_user(fl->l_start, &target_fl->l_start);
+ __put_user(fl->l_len, &target_fl->l_len);
+ __put_user(fl->l_pid, &target_fl->l_pid);
+ unlock_user_struct(target_fl, target_flock_addr, 1);
+ return 0;
+}
+
+static abi_long do_fcntl(int fd, int cmd, abi_ulong arg)
+{
struct flock64 fl64;
- struct target_flock64 *target_fl64;
#ifdef F_GETOWN_EX
struct f_owner_ex fox;
struct target_f_owner_ex *target_fox;
@@ -5615,77 +5737,41 @@ static abi_long do_fcntl(int fd, int cmd, abi_ulong arg)
switch(cmd) {
case TARGET_F_GETLK:
- if (!lock_user_struct(VERIFY_READ, target_fl, arg, 1))
- return -TARGET_EFAULT;
- fl.l_type =
- target_to_host_bitmask(tswap16(target_fl->l_type), flock_tbl);
- fl.l_whence = tswap16(target_fl->l_whence);
- fl.l_start = tswapal(target_fl->l_start);
- fl.l_len = tswapal(target_fl->l_len);
- fl.l_pid = tswap32(target_fl->l_pid);
- unlock_user_struct(target_fl, arg, 0);
- ret = get_errno(fcntl(fd, host_cmd, &fl));
+ ret = copy_from_user_flock(&fl64, arg);
+ if (ret) {
+ return ret;
+ }
+ ret = get_errno(fcntl(fd, host_cmd, &fl64));
if (ret == 0) {
- if (!lock_user_struct(VERIFY_WRITE, target_fl, arg, 0))
- return -TARGET_EFAULT;
- target_fl->l_type =
- host_to_target_bitmask(tswap16(fl.l_type), flock_tbl);
- target_fl->l_whence = tswap16(fl.l_whence);
- target_fl->l_start = tswapal(fl.l_start);
- target_fl->l_len = tswapal(fl.l_len);
- target_fl->l_pid = tswap32(fl.l_pid);
- unlock_user_struct(target_fl, arg, 1);
+ ret = copy_to_user_flock(arg, &fl64);
}
break;
case TARGET_F_SETLK:
case TARGET_F_SETLKW:
- if (!lock_user_struct(VERIFY_READ, target_fl, arg, 1))
- return -TARGET_EFAULT;
- fl.l_type =
- target_to_host_bitmask(tswap16(target_fl->l_type), flock_tbl);
- fl.l_whence = tswap16(target_fl->l_whence);
- fl.l_start = tswapal(target_fl->l_start);
- fl.l_len = tswapal(target_fl->l_len);
- fl.l_pid = tswap32(target_fl->l_pid);
- unlock_user_struct(target_fl, arg, 0);
- ret = get_errno(fcntl(fd, host_cmd, &fl));
+ ret = copy_from_user_flock(&fl64, arg);
+ if (ret) {
+ return ret;
+ }
+ ret = get_errno(fcntl(fd, host_cmd, &fl64));
break;
case TARGET_F_GETLK64:
- if (!lock_user_struct(VERIFY_READ, target_fl64, arg, 1))
- return -TARGET_EFAULT;
- fl64.l_type =
- target_to_host_bitmask(tswap16(target_fl64->l_type), flock_tbl) >> 1;
- fl64.l_whence = tswap16(target_fl64->l_whence);
- fl64.l_start = tswap64(target_fl64->l_start);
- fl64.l_len = tswap64(target_fl64->l_len);
- fl64.l_pid = tswap32(target_fl64->l_pid);
- unlock_user_struct(target_fl64, arg, 0);
+ ret = copy_from_user_flock64(&fl64, arg);
+ if (ret) {
+ return ret;
+ }
ret = get_errno(fcntl(fd, host_cmd, &fl64));
if (ret == 0) {
- if (!lock_user_struct(VERIFY_WRITE, target_fl64, arg, 0))
- return -TARGET_EFAULT;
- target_fl64->l_type =
- host_to_target_bitmask(tswap16(fl64.l_type), flock_tbl) >> 1;
- target_fl64->l_whence = tswap16(fl64.l_whence);
- target_fl64->l_start = tswap64(fl64.l_start);
- target_fl64->l_len = tswap64(fl64.l_len);
- target_fl64->l_pid = tswap32(fl64.l_pid);
- unlock_user_struct(target_fl64, arg, 1);
+ ret = copy_to_user_flock64(arg, &fl64);
}
break;
case TARGET_F_SETLK64:
case TARGET_F_SETLKW64:
- if (!lock_user_struct(VERIFY_READ, target_fl64, arg, 1))
- return -TARGET_EFAULT;
- fl64.l_type =
- target_to_host_bitmask(tswap16(target_fl64->l_type), flock_tbl) >> 1;
- fl64.l_whence = tswap16(target_fl64->l_whence);
- fl64.l_start = tswap64(target_fl64->l_start);
- fl64.l_len = tswap64(target_fl64->l_len);
- fl64.l_pid = tswap32(target_fl64->l_pid);
- unlock_user_struct(target_fl64, arg, 0);
+ ret = copy_from_user_flock64(&fl64, arg);
+ if (ret) {
+ return ret;
+ }
ret = get_errno(fcntl(fd, host_cmd, &fl64));
break;
@@ -10133,9 +10219,14 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
{
int cmd;
struct flock64 fl;
- struct target_flock64 *target_fl;
+ from_flock64_fn *copyfrom = copy_from_user_flock64;
+ to_flock64_fn *copyto = copy_to_user_flock64;
+
#ifdef TARGET_ARM
- struct target_eabi_flock64 *target_efl;
+ if (((CPUARMState *)cpu_env)->eabi) {
+ copyfrom = copy_from_user_eabi_flock64;
+ copyto = copy_to_user_eabi_flock64;
+ }
#endif
cmd = target_to_host_fcntl_cmd(arg2);
@@ -10146,78 +10237,21 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
switch(arg2) {
case TARGET_F_GETLK64:
-#ifdef TARGET_ARM
- if (((CPUARMState *)cpu_env)->eabi) {
- if (!lock_user_struct(VERIFY_READ, target_efl, arg3, 1))
- goto efault;
- fl.l_type = tswap16(target_efl->l_type);
- fl.l_whence = tswap16(target_efl->l_whence);
- fl.l_start = tswap64(target_efl->l_start);
- fl.l_len = tswap64(target_efl->l_len);
- fl.l_pid = tswap32(target_efl->l_pid);
- unlock_user_struct(target_efl, arg3, 0);
- } else
-#endif
- {
- if (!lock_user_struct(VERIFY_READ, target_fl, arg3, 1))
- goto efault;
- fl.l_type = tswap16(target_fl->l_type);
- fl.l_whence = tswap16(target_fl->l_whence);
- fl.l_start = tswap64(target_fl->l_start);
- fl.l_len = tswap64(target_fl->l_len);
- fl.l_pid = tswap32(target_fl->l_pid);
- unlock_user_struct(target_fl, arg3, 0);
+ ret = copyfrom(&fl, arg3);
+ if (ret) {
+ break;
}
ret = get_errno(fcntl(arg1, cmd, &fl));
- if (ret == 0) {
-#ifdef TARGET_ARM
- if (((CPUARMState *)cpu_env)->eabi) {
- if (!lock_user_struct(VERIFY_WRITE, target_efl, arg3, 0))
- goto efault;
- target_efl->l_type = tswap16(fl.l_type);
- target_efl->l_whence = tswap16(fl.l_whence);
- target_efl->l_start = tswap64(fl.l_start);
- target_efl->l_len = tswap64(fl.l_len);
- target_efl->l_pid = tswap32(fl.l_pid);
- unlock_user_struct(target_efl, arg3, 1);
- } else
-#endif
- {
- if (!lock_user_struct(VERIFY_WRITE, target_fl, arg3, 0))
- goto efault;
- target_fl->l_type = tswap16(fl.l_type);
- target_fl->l_whence = tswap16(fl.l_whence);
- target_fl->l_start = tswap64(fl.l_start);
- target_fl->l_len = tswap64(fl.l_len);
- target_fl->l_pid = tswap32(fl.l_pid);
- unlock_user_struct(target_fl, arg3, 1);
- }
- }
+ if (ret == 0) {
+ ret = copyto(arg3, &fl);
+ }
break;
case TARGET_F_SETLK64:
case TARGET_F_SETLKW64:
-#ifdef TARGET_ARM
- if (((CPUARMState *)cpu_env)->eabi) {
- if (!lock_user_struct(VERIFY_READ, target_efl, arg3, 1))
- goto efault;
- fl.l_type = tswap16(target_efl->l_type);
- fl.l_whence = tswap16(target_efl->l_whence);
- fl.l_start = tswap64(target_efl->l_start);
- fl.l_len = tswap64(target_efl->l_len);
- fl.l_pid = tswap32(target_efl->l_pid);
- unlock_user_struct(target_efl, arg3, 0);
- } else
-#endif
- {
- if (!lock_user_struct(VERIFY_READ, target_fl, arg3, 1))
- goto efault;
- fl.l_type = tswap16(target_fl->l_type);
- fl.l_whence = tswap16(target_fl->l_whence);
- fl.l_start = tswap64(target_fl->l_start);
- fl.l_len = tswap64(target_fl->l_len);
- fl.l_pid = tswap32(target_fl->l_pid);
- unlock_user_struct(target_fl, arg3, 0);
+ ret = copyfrom(&fl, arg3);
+ if (ret) {
+ break;
}
ret = get_errno(fcntl(arg1, cmd, &fl));
break;
--
1.9.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [Qemu-devel] [PATCH v2 3/3] linux-user: Use safe_syscall wrapper for fcntl
2016-06-09 14:09 [Qemu-devel] [PATCH v2 0/3] Use safe_syscall wrapper for fcntl() Peter Maydell
2016-06-09 14:09 ` [Qemu-devel] [PATCH v2 1/3] linux-user: Avoid possible misalignment in host_to_target_siginfo() Peter Maydell
2016-06-09 14:09 ` [Qemu-devel] [PATCH v2 2/3] linux-user: Use __get_user() and __put_user() to handle structs in do_fcntl() Peter Maydell
@ 2016-06-09 14:09 ` Peter Maydell
2016-06-09 19:28 ` Laurent Vivier
2 siblings, 1 reply; 7+ messages in thread
From: Peter Maydell @ 2016-06-09 14:09 UTC (permalink / raw)
To: qemu-devel; +Cc: patches, Laurent Vivier, Riku Voipio
Use the safe_syscall wrapper for fcntl. This is straightforward now
that we always use 'struct fcntl64' on the host, as we don't need
to select whether to call the host's fcntl64 or fcntl syscall
(a detail that the libc previously hid for us).
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
linux-user/syscall.c | 34 +++++++++++++++++++++++-----------
1 file changed, 23 insertions(+), 11 deletions(-)
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 78c36dd..121b89f 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -784,6 +784,16 @@ safe_syscall5(int, mq_timedreceive, int, mqdes, char *, msg_ptr,
* the libc function.
*/
#define safe_ioctl(...) safe_syscall(__NR_ioctl, __VA_ARGS__)
+/* Similarly for fcntl. Note that callers must always:
+ * pass the F_GETLK64 etc constants rather than the unsuffixed F_GETLK
+ * use the flock64 struct rather than unsuffixed flock
+ * This will then work and use a 64-bit offset for both 32-bit and 64-bit hosts.
+ */
+#ifdef __NR_fcntl64
+#define safe_fcntl(...) safe_syscall(__NR_fcntl64, __VA_ARGS__)
+#else
+#define safe_fcntl(...) safe_syscall(__NR_fcntl, __VA_ARGS__)
+#endif
static inline int host_to_target_sock_type(int host_type)
{
@@ -5741,7 +5751,7 @@ static abi_long do_fcntl(int fd, int cmd, abi_ulong arg)
if (ret) {
return ret;
}
- ret = get_errno(fcntl(fd, host_cmd, &fl64));
+ ret = get_errno(safe_fcntl(fd, host_cmd, &fl64));
if (ret == 0) {
ret = copy_to_user_flock(arg, &fl64);
}
@@ -5753,7 +5763,7 @@ static abi_long do_fcntl(int fd, int cmd, abi_ulong arg)
if (ret) {
return ret;
}
- ret = get_errno(fcntl(fd, host_cmd, &fl64));
+ ret = get_errno(safe_fcntl(fd, host_cmd, &fl64));
break;
case TARGET_F_GETLK64:
@@ -5761,7 +5771,7 @@ static abi_long do_fcntl(int fd, int cmd, abi_ulong arg)
if (ret) {
return ret;
}
- ret = get_errno(fcntl(fd, host_cmd, &fl64));
+ ret = get_errno(safe_fcntl(fd, host_cmd, &fl64));
if (ret == 0) {
ret = copy_to_user_flock64(arg, &fl64);
}
@@ -5772,23 +5782,25 @@ static abi_long do_fcntl(int fd, int cmd, abi_ulong arg)
if (ret) {
return ret;
}
- ret = get_errno(fcntl(fd, host_cmd, &fl64));
+ ret = get_errno(safe_fcntl(fd, host_cmd, &fl64));
break;
case TARGET_F_GETFL:
- ret = get_errno(fcntl(fd, host_cmd, arg));
+ ret = get_errno(safe_fcntl(fd, host_cmd, arg));
if (ret >= 0) {
ret = host_to_target_bitmask(ret, fcntl_flags_tbl);
}
break;
case TARGET_F_SETFL:
- ret = get_errno(fcntl(fd, host_cmd, target_to_host_bitmask(arg, fcntl_flags_tbl)));
+ ret = get_errno(safe_fcntl(fd, host_cmd,
+ target_to_host_bitmask(arg,
+ fcntl_flags_tbl)));
break;
#ifdef F_GETOWN_EX
case TARGET_F_GETOWN_EX:
- ret = get_errno(fcntl(fd, host_cmd, &fox));
+ ret = get_errno(safe_fcntl(fd, host_cmd, &fox));
if (ret >= 0) {
if (!lock_user_struct(VERIFY_WRITE, target_fox, arg, 0))
return -TARGET_EFAULT;
@@ -5806,7 +5818,7 @@ static abi_long do_fcntl(int fd, int cmd, abi_ulong arg)
fox.type = tswap32(target_fox->type);
fox.pid = tswap32(target_fox->pid);
unlock_user_struct(target_fox, arg, 0);
- ret = get_errno(fcntl(fd, host_cmd, &fox));
+ ret = get_errno(safe_fcntl(fd, host_cmd, &fox));
break;
#endif
@@ -5816,11 +5828,11 @@ static abi_long do_fcntl(int fd, int cmd, abi_ulong arg)
case TARGET_F_GETSIG:
case TARGET_F_SETLEASE:
case TARGET_F_GETLEASE:
- ret = get_errno(fcntl(fd, host_cmd, arg));
+ ret = get_errno(safe_fcntl(fd, host_cmd, arg));
break;
default:
- ret = get_errno(fcntl(fd, cmd, arg));
+ ret = get_errno(safe_fcntl(fd, cmd, arg));
break;
}
return ret;
@@ -10253,7 +10265,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
if (ret) {
break;
}
- ret = get_errno(fcntl(arg1, cmd, &fl));
+ ret = get_errno(safe_fcntl(arg1, cmd, &fl));
break;
default:
ret = do_fcntl(arg1, arg2, arg3);
--
1.9.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/3] linux-user: Avoid possible misalignment in host_to_target_siginfo()
2016-06-09 14:09 ` [Qemu-devel] [PATCH v2 1/3] linux-user: Avoid possible misalignment in host_to_target_siginfo() Peter Maydell
@ 2016-06-09 19:27 ` Laurent Vivier
0 siblings, 0 replies; 7+ messages in thread
From: Laurent Vivier @ 2016-06-09 19:27 UTC (permalink / raw)
To: Peter Maydell, qemu-devel; +Cc: patches, Riku Voipio
Le 09/06/2016 à 16:09, Peter Maydell a écrit :
> host_to_target_siginfo() is implemented by a combination of
> host_to_target_siginfo_noswap() followed by tswap_siginfo().
> The first of these two functions assumes that the target_siginfo_t
> it is writing to is correctly aligned, but the pointer passed
> into host_to_target_siginfo() is directly from the guest and
> might be misaligned. Use a local variable to avoid this problem.
> (tswap_siginfo() does now correctly handle a misaligned destination.)
>
> We have to add a memset() to host_to_target_siginfo_noswap()
> to avoid some false positive "may be used uninitialized" warnings
> from gcc about subfields of the _sifields union if it chooses to
> inline both tswap_siginfo() and host_to_target_siginfo_noswap()
> into host_to_target_siginfo().
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
> ---
> linux-user/signal.c | 13 +++++++++++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/linux-user/signal.c b/linux-user/signal.c
> index 61c1145..37fb60f 100644
> --- a/linux-user/signal.c
> +++ b/linux-user/signal.c
> @@ -281,6 +281,14 @@ static inline void host_to_target_siginfo_noswap(target_siginfo_t *tinfo,
> tinfo->si_errno = 0;
> tinfo->si_code = info->si_code;
>
> + /* This memset serves two purposes:
> + * (1) ensure we don't leak random junk to the guest later
> + * (2) placate false positives from gcc about fields
> + * being used uninitialized if it chooses to inline both this
> + * function and tswap_siginfo() into host_to_target_siginfo().
> + */
> + memset(tinfo->_sifields._pad, 0, sizeof(tinfo->_sifields._pad));
> +
> /* This is awkward, because we have to use a combination of
> * the si_code and si_signo to figure out which of the union's
> * members are valid. (Within the host kernel it is always possible
> @@ -400,8 +408,9 @@ static void tswap_siginfo(target_siginfo_t *tinfo,
>
> void host_to_target_siginfo(target_siginfo_t *tinfo, const siginfo_t *info)
> {
> - host_to_target_siginfo_noswap(tinfo, info);
> - tswap_siginfo(tinfo, tinfo);
> + target_siginfo_t tgt_tmp;
> + host_to_target_siginfo_noswap(&tgt_tmp, info);
> + tswap_siginfo(tinfo, &tgt_tmp);
> }
>
> /* XXX: we support only POSIX RT signals are used. */
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH v2 2/3] linux-user: Use __get_user() and __put_user() to handle structs in do_fcntl()
2016-06-09 14:09 ` [Qemu-devel] [PATCH v2 2/3] linux-user: Use __get_user() and __put_user() to handle structs in do_fcntl() Peter Maydell
@ 2016-06-09 19:27 ` Laurent Vivier
0 siblings, 0 replies; 7+ messages in thread
From: Laurent Vivier @ 2016-06-09 19:27 UTC (permalink / raw)
To: Peter Maydell, qemu-devel; +Cc: patches, Riku Voipio
Le 09/06/2016 à 16:09, Peter Maydell a écrit :
> Use the __get_user() and __put_user() to handle reading and writing the
> guest structures in do_ioctl(). This has two benefits:
> * avoids possible errors due to misaligned guest pointers
> * correctly sign extends signed fields (like l_start in struct flock)
> which might be different sizes between guest and host
>
> To do this we abstract out into copy_from/to_user functions. We
> also standardize on always using host flock64 and the F_GETLK64
> etc flock commands, as this means we always have 64 bit offsets
> whether the host is 64-bit or 32-bit and we don't need to support
> conversion to both host struct flock and struct flock64.
>
> In passing we fix errors in converting l_type from the host to
> the target (where we were doing a byteswap of the host value
> before trying to do the convert-bitmasks operation rather than
> otherwise, and inexplicably shifting left by 1); these were
> accidentally left over when the original simple "just shift by 1"
> arm<->x86 conversion of commit 43f238d was changed to the more
> general scheme of using target_to_host_bitmask() functions in 2ba7f73.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
> ---
> linux-user/syscall.c | 298 ++++++++++++++++++++++++++++-----------------------
> 1 file changed, 166 insertions(+), 132 deletions(-)
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 71ccbd9..78c36dd 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -5542,11 +5542,11 @@ static int target_to_host_fcntl_cmd(int cmd)
> case TARGET_F_SETFL:
> return cmd;
> case TARGET_F_GETLK:
> - return F_GETLK;
> - case TARGET_F_SETLK:
> - return F_SETLK;
> - case TARGET_F_SETLKW:
> - return F_SETLKW;
> + return F_GETLK64;
> + case TARGET_F_SETLK:
> + return F_SETLK64;
> + case TARGET_F_SETLKW:
> + return F_SETLKW64;
> case TARGET_F_GETOWN:
> return F_GETOWN;
> case TARGET_F_SETOWN:
> @@ -5597,12 +5597,134 @@ static const bitmask_transtbl flock_tbl[] = {
> { 0, 0, 0, 0 }
> };
>
> -static abi_long do_fcntl(int fd, int cmd, abi_ulong arg)
> +static inline abi_long copy_from_user_flock(struct flock64 *fl,
> + abi_ulong target_flock_addr)
> {
> - struct flock fl;
> struct target_flock *target_fl;
> + short l_type;
> +
> + if (!lock_user_struct(VERIFY_READ, target_fl, target_flock_addr, 1)) {
> + return -TARGET_EFAULT;
> + }
> +
> + __get_user(l_type, &target_fl->l_type);
> + fl->l_type = target_to_host_bitmask(l_type, flock_tbl);
> + __get_user(fl->l_whence, &target_fl->l_whence);
> + __get_user(fl->l_start, &target_fl->l_start);
> + __get_user(fl->l_len, &target_fl->l_len);
> + __get_user(fl->l_pid, &target_fl->l_pid);
> + unlock_user_struct(target_fl, target_flock_addr, 0);
> + return 0;
> +}
> +
> +static inline abi_long copy_to_user_flock(abi_ulong target_flock_addr,
> + const struct flock64 *fl)
> +{
> + struct target_flock *target_fl;
> + short l_type;
> +
> + if (!lock_user_struct(VERIFY_WRITE, target_fl, target_flock_addr, 0)) {
> + return -TARGET_EFAULT;
> + }
> +
> + l_type = host_to_target_bitmask(fl->l_type, flock_tbl);
> + __put_user(l_type, &target_fl->l_type);
> + __put_user(fl->l_whence, &target_fl->l_whence);
> + __put_user(fl->l_start, &target_fl->l_start);
> + __put_user(fl->l_len, &target_fl->l_len);
> + __put_user(fl->l_pid, &target_fl->l_pid);
> + unlock_user_struct(target_fl, target_flock_addr, 1);
> + return 0;
> +}
> +
> +typedef abi_long from_flock64_fn(struct flock64 *fl, abi_ulong target_addr);
> +typedef abi_long to_flock64_fn(abi_ulong target_addr, const struct flock64 *fl);
> +
> +#ifdef TARGET_ARM
> +static inline abi_long copy_from_user_eabi_flock64(struct flock64 *fl,
> + abi_ulong target_flock_addr)
> +{
> + struct target_eabi_flock64 *target_fl;
> + short l_type;
> +
> + if (!lock_user_struct(VERIFY_READ, target_fl, target_flock_addr, 1)) {
> + return -TARGET_EFAULT;
> + }
> +
> + __get_user(l_type, &target_fl->l_type);
> + fl->l_type = target_to_host_bitmask(l_type, flock_tbl);
> + __get_user(fl->l_whence, &target_fl->l_whence);
> + __get_user(fl->l_start, &target_fl->l_start);
> + __get_user(fl->l_len, &target_fl->l_len);
> + __get_user(fl->l_pid, &target_fl->l_pid);
> + unlock_user_struct(target_fl, target_flock_addr, 0);
> + return 0;
> +}
> +
> +static inline abi_long copy_to_user_eabi_flock64(abi_ulong target_flock_addr,
> + const struct flock64 *fl)
> +{
> + struct target_eabi_flock64 *target_fl;
> + short l_type;
> +
> + if (!lock_user_struct(VERIFY_WRITE, target_fl, target_flock_addr, 0)) {
> + return -TARGET_EFAULT;
> + }
> +
> + l_type = host_to_target_bitmask(fl->l_type, flock_tbl);
> + __put_user(l_type, &target_fl->l_type);
> + __put_user(fl->l_whence, &target_fl->l_whence);
> + __put_user(fl->l_start, &target_fl->l_start);
> + __put_user(fl->l_len, &target_fl->l_len);
> + __put_user(fl->l_pid, &target_fl->l_pid);
> + unlock_user_struct(target_fl, target_flock_addr, 1);
> + return 0;
> +}
> +#endif
> +
> +static inline abi_long copy_from_user_flock64(struct flock64 *fl,
> + abi_ulong target_flock_addr)
> +{
> + struct target_flock64 *target_fl;
> + short l_type;
> +
> + if (!lock_user_struct(VERIFY_READ, target_fl, target_flock_addr, 1)) {
> + return -TARGET_EFAULT;
> + }
> +
> + __get_user(l_type, &target_fl->l_type);
> + fl->l_type = target_to_host_bitmask(l_type, flock_tbl);
> + __get_user(fl->l_whence, &target_fl->l_whence);
> + __get_user(fl->l_start, &target_fl->l_start);
> + __get_user(fl->l_len, &target_fl->l_len);
> + __get_user(fl->l_pid, &target_fl->l_pid);
> + unlock_user_struct(target_fl, target_flock_addr, 0);
> + return 0;
> +}
> +
> +static inline abi_long copy_to_user_flock64(abi_ulong target_flock_addr,
> + const struct flock64 *fl)
> +{
> + struct target_flock64 *target_fl;
> + short l_type;
> +
> + if (!lock_user_struct(VERIFY_WRITE, target_fl, target_flock_addr, 0)) {
> + return -TARGET_EFAULT;
> + }
> +
> + l_type = host_to_target_bitmask(fl->l_type, flock_tbl);
> + __put_user(l_type, &target_fl->l_type);
> + __put_user(fl->l_whence, &target_fl->l_whence);
> + __put_user(fl->l_start, &target_fl->l_start);
> + __put_user(fl->l_len, &target_fl->l_len);
> + __put_user(fl->l_pid, &target_fl->l_pid);
> + unlock_user_struct(target_fl, target_flock_addr, 1);
> + return 0;
> +}
> +
> +static abi_long do_fcntl(int fd, int cmd, abi_ulong arg)
> +{
> struct flock64 fl64;
> - struct target_flock64 *target_fl64;
> #ifdef F_GETOWN_EX
> struct f_owner_ex fox;
> struct target_f_owner_ex *target_fox;
> @@ -5615,77 +5737,41 @@ static abi_long do_fcntl(int fd, int cmd, abi_ulong arg)
>
> switch(cmd) {
> case TARGET_F_GETLK:
> - if (!lock_user_struct(VERIFY_READ, target_fl, arg, 1))
> - return -TARGET_EFAULT;
> - fl.l_type =
> - target_to_host_bitmask(tswap16(target_fl->l_type), flock_tbl);
> - fl.l_whence = tswap16(target_fl->l_whence);
> - fl.l_start = tswapal(target_fl->l_start);
> - fl.l_len = tswapal(target_fl->l_len);
> - fl.l_pid = tswap32(target_fl->l_pid);
> - unlock_user_struct(target_fl, arg, 0);
> - ret = get_errno(fcntl(fd, host_cmd, &fl));
> + ret = copy_from_user_flock(&fl64, arg);
> + if (ret) {
> + return ret;
> + }
> + ret = get_errno(fcntl(fd, host_cmd, &fl64));
> if (ret == 0) {
> - if (!lock_user_struct(VERIFY_WRITE, target_fl, arg, 0))
> - return -TARGET_EFAULT;
> - target_fl->l_type =
> - host_to_target_bitmask(tswap16(fl.l_type), flock_tbl);
> - target_fl->l_whence = tswap16(fl.l_whence);
> - target_fl->l_start = tswapal(fl.l_start);
> - target_fl->l_len = tswapal(fl.l_len);
> - target_fl->l_pid = tswap32(fl.l_pid);
> - unlock_user_struct(target_fl, arg, 1);
> + ret = copy_to_user_flock(arg, &fl64);
> }
> break;
>
> case TARGET_F_SETLK:
> case TARGET_F_SETLKW:
> - if (!lock_user_struct(VERIFY_READ, target_fl, arg, 1))
> - return -TARGET_EFAULT;
> - fl.l_type =
> - target_to_host_bitmask(tswap16(target_fl->l_type), flock_tbl);
> - fl.l_whence = tswap16(target_fl->l_whence);
> - fl.l_start = tswapal(target_fl->l_start);
> - fl.l_len = tswapal(target_fl->l_len);
> - fl.l_pid = tswap32(target_fl->l_pid);
> - unlock_user_struct(target_fl, arg, 0);
> - ret = get_errno(fcntl(fd, host_cmd, &fl));
> + ret = copy_from_user_flock(&fl64, arg);
> + if (ret) {
> + return ret;
> + }
> + ret = get_errno(fcntl(fd, host_cmd, &fl64));
> break;
>
> case TARGET_F_GETLK64:
> - if (!lock_user_struct(VERIFY_READ, target_fl64, arg, 1))
> - return -TARGET_EFAULT;
> - fl64.l_type =
> - target_to_host_bitmask(tswap16(target_fl64->l_type), flock_tbl) >> 1;
> - fl64.l_whence = tswap16(target_fl64->l_whence);
> - fl64.l_start = tswap64(target_fl64->l_start);
> - fl64.l_len = tswap64(target_fl64->l_len);
> - fl64.l_pid = tswap32(target_fl64->l_pid);
> - unlock_user_struct(target_fl64, arg, 0);
> + ret = copy_from_user_flock64(&fl64, arg);
> + if (ret) {
> + return ret;
> + }
> ret = get_errno(fcntl(fd, host_cmd, &fl64));
> if (ret == 0) {
> - if (!lock_user_struct(VERIFY_WRITE, target_fl64, arg, 0))
> - return -TARGET_EFAULT;
> - target_fl64->l_type =
> - host_to_target_bitmask(tswap16(fl64.l_type), flock_tbl) >> 1;
> - target_fl64->l_whence = tswap16(fl64.l_whence);
> - target_fl64->l_start = tswap64(fl64.l_start);
> - target_fl64->l_len = tswap64(fl64.l_len);
> - target_fl64->l_pid = tswap32(fl64.l_pid);
> - unlock_user_struct(target_fl64, arg, 1);
> + ret = copy_to_user_flock64(arg, &fl64);
> }
> break;
> case TARGET_F_SETLK64:
> case TARGET_F_SETLKW64:
> - if (!lock_user_struct(VERIFY_READ, target_fl64, arg, 1))
> - return -TARGET_EFAULT;
> - fl64.l_type =
> - target_to_host_bitmask(tswap16(target_fl64->l_type), flock_tbl) >> 1;
> - fl64.l_whence = tswap16(target_fl64->l_whence);
> - fl64.l_start = tswap64(target_fl64->l_start);
> - fl64.l_len = tswap64(target_fl64->l_len);
> - fl64.l_pid = tswap32(target_fl64->l_pid);
> - unlock_user_struct(target_fl64, arg, 0);
> + ret = copy_from_user_flock64(&fl64, arg);
> + if (ret) {
> + return ret;
> + }
> ret = get_errno(fcntl(fd, host_cmd, &fl64));
> break;
>
> @@ -10133,9 +10219,14 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
> {
> int cmd;
> struct flock64 fl;
> - struct target_flock64 *target_fl;
> + from_flock64_fn *copyfrom = copy_from_user_flock64;
> + to_flock64_fn *copyto = copy_to_user_flock64;
> +
> #ifdef TARGET_ARM
> - struct target_eabi_flock64 *target_efl;
> + if (((CPUARMState *)cpu_env)->eabi) {
> + copyfrom = copy_from_user_eabi_flock64;
> + copyto = copy_to_user_eabi_flock64;
> + }
> #endif
>
> cmd = target_to_host_fcntl_cmd(arg2);
> @@ -10146,78 +10237,21 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>
> switch(arg2) {
> case TARGET_F_GETLK64:
> -#ifdef TARGET_ARM
> - if (((CPUARMState *)cpu_env)->eabi) {
> - if (!lock_user_struct(VERIFY_READ, target_efl, arg3, 1))
> - goto efault;
> - fl.l_type = tswap16(target_efl->l_type);
> - fl.l_whence = tswap16(target_efl->l_whence);
> - fl.l_start = tswap64(target_efl->l_start);
> - fl.l_len = tswap64(target_efl->l_len);
> - fl.l_pid = tswap32(target_efl->l_pid);
> - unlock_user_struct(target_efl, arg3, 0);
> - } else
> -#endif
> - {
> - if (!lock_user_struct(VERIFY_READ, target_fl, arg3, 1))
> - goto efault;
> - fl.l_type = tswap16(target_fl->l_type);
> - fl.l_whence = tswap16(target_fl->l_whence);
> - fl.l_start = tswap64(target_fl->l_start);
> - fl.l_len = tswap64(target_fl->l_len);
> - fl.l_pid = tswap32(target_fl->l_pid);
> - unlock_user_struct(target_fl, arg3, 0);
> + ret = copyfrom(&fl, arg3);
> + if (ret) {
> + break;
> }
> ret = get_errno(fcntl(arg1, cmd, &fl));
> - if (ret == 0) {
> -#ifdef TARGET_ARM
> - if (((CPUARMState *)cpu_env)->eabi) {
> - if (!lock_user_struct(VERIFY_WRITE, target_efl, arg3, 0))
> - goto efault;
> - target_efl->l_type = tswap16(fl.l_type);
> - target_efl->l_whence = tswap16(fl.l_whence);
> - target_efl->l_start = tswap64(fl.l_start);
> - target_efl->l_len = tswap64(fl.l_len);
> - target_efl->l_pid = tswap32(fl.l_pid);
> - unlock_user_struct(target_efl, arg3, 1);
> - } else
> -#endif
> - {
> - if (!lock_user_struct(VERIFY_WRITE, target_fl, arg3, 0))
> - goto efault;
> - target_fl->l_type = tswap16(fl.l_type);
> - target_fl->l_whence = tswap16(fl.l_whence);
> - target_fl->l_start = tswap64(fl.l_start);
> - target_fl->l_len = tswap64(fl.l_len);
> - target_fl->l_pid = tswap32(fl.l_pid);
> - unlock_user_struct(target_fl, arg3, 1);
> - }
> - }
> + if (ret == 0) {
> + ret = copyto(arg3, &fl);
> + }
> break;
>
> case TARGET_F_SETLK64:
> case TARGET_F_SETLKW64:
> -#ifdef TARGET_ARM
> - if (((CPUARMState *)cpu_env)->eabi) {
> - if (!lock_user_struct(VERIFY_READ, target_efl, arg3, 1))
> - goto efault;
> - fl.l_type = tswap16(target_efl->l_type);
> - fl.l_whence = tswap16(target_efl->l_whence);
> - fl.l_start = tswap64(target_efl->l_start);
> - fl.l_len = tswap64(target_efl->l_len);
> - fl.l_pid = tswap32(target_efl->l_pid);
> - unlock_user_struct(target_efl, arg3, 0);
> - } else
> -#endif
> - {
> - if (!lock_user_struct(VERIFY_READ, target_fl, arg3, 1))
> - goto efault;
> - fl.l_type = tswap16(target_fl->l_type);
> - fl.l_whence = tswap16(target_fl->l_whence);
> - fl.l_start = tswap64(target_fl->l_start);
> - fl.l_len = tswap64(target_fl->l_len);
> - fl.l_pid = tswap32(target_fl->l_pid);
> - unlock_user_struct(target_fl, arg3, 0);
> + ret = copyfrom(&fl, arg3);
> + if (ret) {
> + break;
> }
> ret = get_errno(fcntl(arg1, cmd, &fl));
> break;
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH v2 3/3] linux-user: Use safe_syscall wrapper for fcntl
2016-06-09 14:09 ` [Qemu-devel] [PATCH v2 3/3] linux-user: Use safe_syscall wrapper for fcntl Peter Maydell
@ 2016-06-09 19:28 ` Laurent Vivier
0 siblings, 0 replies; 7+ messages in thread
From: Laurent Vivier @ 2016-06-09 19:28 UTC (permalink / raw)
To: Peter Maydell, qemu-devel; +Cc: patches, Riku Voipio
Le 09/06/2016 à 16:09, Peter Maydell a écrit :
> Use the safe_syscall wrapper for fcntl. This is straightforward now
> that we always use 'struct fcntl64' on the host, as we don't need
> to select whether to call the host's fcntl64 or fcntl syscall
> (a detail that the libc previously hid for us).
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
> ---
> linux-user/syscall.c | 34 +++++++++++++++++++++++-----------
> 1 file changed, 23 insertions(+), 11 deletions(-)
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 78c36dd..121b89f 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -784,6 +784,16 @@ safe_syscall5(int, mq_timedreceive, int, mqdes, char *, msg_ptr,
> * the libc function.
> */
> #define safe_ioctl(...) safe_syscall(__NR_ioctl, __VA_ARGS__)
> +/* Similarly for fcntl. Note that callers must always:
> + * pass the F_GETLK64 etc constants rather than the unsuffixed F_GETLK
> + * use the flock64 struct rather than unsuffixed flock
> + * This will then work and use a 64-bit offset for both 32-bit and 64-bit hosts.
> + */
> +#ifdef __NR_fcntl64
> +#define safe_fcntl(...) safe_syscall(__NR_fcntl64, __VA_ARGS__)
> +#else
> +#define safe_fcntl(...) safe_syscall(__NR_fcntl, __VA_ARGS__)
> +#endif
>
> static inline int host_to_target_sock_type(int host_type)
> {
> @@ -5741,7 +5751,7 @@ static abi_long do_fcntl(int fd, int cmd, abi_ulong arg)
> if (ret) {
> return ret;
> }
> - ret = get_errno(fcntl(fd, host_cmd, &fl64));
> + ret = get_errno(safe_fcntl(fd, host_cmd, &fl64));
> if (ret == 0) {
> ret = copy_to_user_flock(arg, &fl64);
> }
> @@ -5753,7 +5763,7 @@ static abi_long do_fcntl(int fd, int cmd, abi_ulong arg)
> if (ret) {
> return ret;
> }
> - ret = get_errno(fcntl(fd, host_cmd, &fl64));
> + ret = get_errno(safe_fcntl(fd, host_cmd, &fl64));
> break;
>
> case TARGET_F_GETLK64:
> @@ -5761,7 +5771,7 @@ static abi_long do_fcntl(int fd, int cmd, abi_ulong arg)
> if (ret) {
> return ret;
> }
> - ret = get_errno(fcntl(fd, host_cmd, &fl64));
> + ret = get_errno(safe_fcntl(fd, host_cmd, &fl64));
> if (ret == 0) {
> ret = copy_to_user_flock64(arg, &fl64);
> }
> @@ -5772,23 +5782,25 @@ static abi_long do_fcntl(int fd, int cmd, abi_ulong arg)
> if (ret) {
> return ret;
> }
> - ret = get_errno(fcntl(fd, host_cmd, &fl64));
> + ret = get_errno(safe_fcntl(fd, host_cmd, &fl64));
> break;
>
> case TARGET_F_GETFL:
> - ret = get_errno(fcntl(fd, host_cmd, arg));
> + ret = get_errno(safe_fcntl(fd, host_cmd, arg));
> if (ret >= 0) {
> ret = host_to_target_bitmask(ret, fcntl_flags_tbl);
> }
> break;
>
> case TARGET_F_SETFL:
> - ret = get_errno(fcntl(fd, host_cmd, target_to_host_bitmask(arg, fcntl_flags_tbl)));
> + ret = get_errno(safe_fcntl(fd, host_cmd,
> + target_to_host_bitmask(arg,
> + fcntl_flags_tbl)));
> break;
>
> #ifdef F_GETOWN_EX
> case TARGET_F_GETOWN_EX:
> - ret = get_errno(fcntl(fd, host_cmd, &fox));
> + ret = get_errno(safe_fcntl(fd, host_cmd, &fox));
> if (ret >= 0) {
> if (!lock_user_struct(VERIFY_WRITE, target_fox, arg, 0))
> return -TARGET_EFAULT;
> @@ -5806,7 +5818,7 @@ static abi_long do_fcntl(int fd, int cmd, abi_ulong arg)
> fox.type = tswap32(target_fox->type);
> fox.pid = tswap32(target_fox->pid);
> unlock_user_struct(target_fox, arg, 0);
> - ret = get_errno(fcntl(fd, host_cmd, &fox));
> + ret = get_errno(safe_fcntl(fd, host_cmd, &fox));
> break;
> #endif
>
> @@ -5816,11 +5828,11 @@ static abi_long do_fcntl(int fd, int cmd, abi_ulong arg)
> case TARGET_F_GETSIG:
> case TARGET_F_SETLEASE:
> case TARGET_F_GETLEASE:
> - ret = get_errno(fcntl(fd, host_cmd, arg));
> + ret = get_errno(safe_fcntl(fd, host_cmd, arg));
> break;
>
> default:
> - ret = get_errno(fcntl(fd, cmd, arg));
> + ret = get_errno(safe_fcntl(fd, cmd, arg));
> break;
> }
> return ret;
> @@ -10253,7 +10265,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
> if (ret) {
> break;
> }
> - ret = get_errno(fcntl(arg1, cmd, &fl));
> + ret = get_errno(safe_fcntl(arg1, cmd, &fl));
> break;
> default:
> ret = do_fcntl(arg1, arg2, arg3);
>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2016-06-09 19:28 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-09 14:09 [Qemu-devel] [PATCH v2 0/3] Use safe_syscall wrapper for fcntl() Peter Maydell
2016-06-09 14:09 ` [Qemu-devel] [PATCH v2 1/3] linux-user: Avoid possible misalignment in host_to_target_siginfo() Peter Maydell
2016-06-09 19:27 ` Laurent Vivier
2016-06-09 14:09 ` [Qemu-devel] [PATCH v2 2/3] linux-user: Use __get_user() and __put_user() to handle structs in do_fcntl() Peter Maydell
2016-06-09 19:27 ` Laurent Vivier
2016-06-09 14:09 ` [Qemu-devel] [PATCH v2 3/3] linux-user: Use safe_syscall wrapper for fcntl Peter Maydell
2016-06-09 19:28 ` Laurent Vivier
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.