[PULL 2/2] linux-user: report EFAULT for a NULL sysinfo/getitimer buffer

[Helge Deller <deller@kernel.org>]

From: TAKANO Mitsuhiro <takano32@gmail.com>

Both handlers skip the copy-out when the guest pointer is zero and return
success, so a caller that forgets the buffer gets an uninitialised result
instead of the EFAULT the kernel reports for a NULL address.

Copy unconditionally: a NULL pointer then fails in lock_user_struct() and
in copy_to_user_timeval() like any other unusable address.

Signed-off-by: TAKANO Mitsuhiro <takano32@gmail.com>
Signed-off-by: Helge Deller <deller@gmx.de>
---
 linux-user/syscall.c                          |  4 +--
 tests/tcg/multiarch/linux/linux-null-buffer.c | 35 +++++++++++++++++--
 2 files changed, 34 insertions(+), 5 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 59cbf5feaa..2fc61b3eab 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -11551,7 +11551,7 @@ static abi_long do_syscall1(CPUArchState *cpu_env, int num, abi_long arg1,
             struct itimerval value;
 
             ret = get_errno(getitimer(arg1, &value));
-            if (!is_error(ret) && arg2) {
+            if (!is_error(ret)) {
                 if (copy_to_user_timeval(arg2,
                                          &value.it_interval)
                     || copy_to_user_timeval(arg2 + sizeof(struct target_timeval),
@@ -11666,7 +11666,7 @@ static abi_long do_syscall1(CPUArchState *cpu_env, int num, abi_long arg1,
             struct target_sysinfo *target_value;
             struct sysinfo value;
             ret = get_errno(sysinfo(&value));
-            if (!is_error(ret) && arg1)
+            if (!is_error(ret))
             {
                 if (!lock_user_struct(VERIFY_WRITE, target_value, arg1, 0))
                     return -TARGET_EFAULT;
diff --git a/tests/tcg/multiarch/linux/linux-null-buffer.c b/tests/tcg/multiarch/linux/linux-null-buffer.c
index 6246e47707..b099803c57 100644
--- a/tests/tcg/multiarch/linux/linux-null-buffer.c
+++ b/tests/tcg/multiarch/linux/linux-null-buffer.c
@@ -1,6 +1,15 @@
 /*
- * A zero-length getrandom() needs no valid buffer: the kernel never looks at
- * the pointer, so getrandom(NULL, 0, 0) succeeds.
+ * Degenerate buffer pointers must behave like they do in the kernel.
+ *
+ * A zero-length getrandom() needs no valid buffer: the pointer is never
+ * used, so getrandom(NULL, 0, 0) succeeds.  A required output buffer, on
+ * the other hand, must not be silently skipped: sysinfo() and getitimer()
+ * have to report EFAULT for a NULL pointer.
+ *
+ * The NULL cases go through syscall() rather than the libc wrappers: on
+ * 32-bit x86, for example, getitimer() hands the kernel a local structure
+ * and copies the result out itself, so it would follow the NULL pointer
+ * before the kernel's answer could be seen.
  *
  * SPDX-License-Identifier: GPL-2.0-or-later
  */
@@ -9,11 +18,31 @@
 #include <errno.h>
 #include <stdlib.h>
 #include <sys/random.h>
+#include <sys/syscall.h>
+#include <sys/sysinfo.h>
+#include <sys/time.h>
+#include <unistd.h>
 
 int main(void)
 {
+    char buf[8];
+    struct itimerval it;
+    struct sysinfo si;
+
+    /* A zero-length request is not rejected for a NULL buffer. */
     errno = 0;
-    assert(getrandom(NULL, 0, 0) == 0);
+    assert(syscall(SYS_getrandom, NULL, 0, 0) == 0);
+
+    /* A NULL output pointer is reported, not silently skipped. */
+    errno = 0;
+    assert(syscall(SYS_sysinfo, NULL) == -1 && errno == EFAULT);
+    errno = 0;
+    assert(syscall(SYS_getitimer, ITIMER_REAL, NULL) == -1 && errno == EFAULT);
+
+    /* The ordinary calls still work. */
+    assert(getrandom(buf, sizeof(buf), 0) == sizeof(buf));
+    assert(sysinfo(&si) == 0);
+    assert(getitimer(ITIMER_REAL, &it) == 0);
 
     return EXIT_SUCCESS;
 }
-- 
2.54.0