[Qemu-devel] [PATCH] linux-user: avoid EFAULT when NULL is passed as buffer for [p]write[64]

Carlo Marcelo Arenas Belón posted 1 patch 6 years, 6 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20171002020132.28762-1-carenas@gmail.com
Test checkpatch passed
Test docker passed
Test s390x passed
linux-user/syscall.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
[Qemu-devel] [PATCH] linux-user: avoid EFAULT when NULL is passed as buffer for [p]write[64]
Posted by Carlo Marcelo Arenas Belón 6 years, 6 months ago
as documented in write(2)[1], if count is zero and fd refers to a regular
file the syscall might return 0 (if error detection is not performed).

the current version of the code will force and EFAULT if the buffer was
pointing to NULL and therefore will result in the following call failing:

  write(fd, NULL, 0)

instead of failing early, pass the call to the host instead so that any
further checks could be done there and the documented shortcut could be
used.

[1] http://man7.org/linux/man-pages/man2/write.2.html#RETURN_VALUE

Reported-by: Zhuowei Zhang <zhuoweizhang@yahoo.com>
Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
---
 linux-user/syscall.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 9b6364a266..c0171e4d8c 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -7783,8 +7783,10 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
         }
         break;
     case TARGET_NR_write:
-        if (!(p = lock_user(VERIFY_READ, arg2, arg3, 1)))
+        p = lock_user(VERIFY_READ, arg2, arg3, 1);
+        if ((p == NULL) && (arg2 != 0)) {
             goto efault;
+        }
         if (fd_trans_target_to_host_data(arg1)) {
             void *copy = g_malloc(arg3);
             memcpy(copy, p, arg3);
@@ -10505,12 +10507,14 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
         unlock_user(p, arg2, ret);
         break;
     case TARGET_NR_pwrite64:
+        p = lock_user(VERIFY_READ, arg2, arg3, 1);
+        if ((p == NULL) && (arg2 != 0)) {
+            goto efault;
+        }
         if (regpairs_aligned(cpu_env)) {
             arg4 = arg5;
             arg5 = arg6;
         }
-        if (!(p = lock_user(VERIFY_READ, arg2, arg3, 1)))
-            goto efault;
         ret = get_errno(pwrite64(arg1, p, arg3, target_offset64(arg4, arg5)));
         unlock_user(p, arg2, 0);
         break;
-- 
2.14.2