[Qemu-devel] [PATCH v2] syscall: fix special case of write(fd, NULL, 0)

zhuoweizhang--- via Qemu-devel posted 1 patch 6 years, 5 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/1506784985-4107-1-git-send-email-zhuoweizhang@yahoo.com
Test checkpatch passed
Test docker passed
Test s390x passed
linux-user/syscall.c | 5 +++++
1 file changed, 5 insertions(+)
[Qemu-devel] [PATCH v2] syscall: fix special case of write(fd, NULL, 0)
Posted by zhuoweizhang--- via Qemu-devel 6 years, 5 months ago
From: Zhuowei Zhang <zhuoweizhang@yahoo.com>

Linux returns success for the special case of calling write with a zero-length
NULL buffer: compiling and running

```
#include <stdio.h>
#include <unistd.h>
#include <fcntl.h>

int main() {
   ssize_t ret = write(STDOUT_FILENO, NULL, 0);
   fprintf(stderr, "write returned %ld\n", ret);
   return 0;
}
```
gives "write returned 0" when run directly, but "write returned -1" in QEMU.

This commit checks for this situation and calls the real syscall with a NULL
buffer and zero length, which gives the correct return value.

Signed-off-by: Zhuowei Zhang <zhuoweizhang@yahoo.com>
---
 linux-user/syscall.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 9b6364a..60769c0 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -7783,6 +7783,11 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
         }
         break;
     case TARGET_NR_write:
+        if (arg2 == 0 && arg3 == 0) {
+            /* special case: write(fd, NULL, 0) returns success. */
+            ret = get_errno(safe_write(arg1, NULL, 0));
+            break;
+        }
         if (!(p = lock_user(VERIFY_READ, arg2, arg3, 1)))
             goto efault;
         if (fd_trans_target_to_host_data(arg1)) {
-- 
1.9.1


.

Re: [Qemu-devel] [PATCH v2] syscall: fix special case of write(fd, NULL, 0)
Posted by Laurent Vivier 6 years, 5 months ago
Le 30/09/2017 à 17:23, zhuoweizhang@yahoo.com a écrit :
> From: Zhuowei Zhang <zhuoweizhang@yahoo.com>
> 
> Linux returns success for the special case of calling write with a zero-length
> NULL buffer: compiling and running
> 
> ```
> #include <stdio.h>
> #include <unistd.h>
> #include <fcntl.h>
> 
> int main() {
>    ssize_t ret = write(STDOUT_FILENO, NULL, 0);
>    fprintf(stderr, "write returned %ld\n", ret);
>    return 0;
> }
> ```
> gives "write returned 0" when run directly, but "write returned -1" in QEMU.
> 
> This commit checks for this situation and calls the real syscall with a NULL
> buffer and zero length, which gives the correct return value.
> 
> Signed-off-by: Zhuowei Zhang <zhuoweizhang@yahoo.com>
> ---
>  linux-user/syscall.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 9b6364a..60769c0 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -7783,6 +7783,11 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>          }
>          break;
>      case TARGET_NR_write:
> +        if (arg2 == 0 && arg3 == 0) {
> +            /* special case: write(fd, NULL, 0) returns success. */
> +            ret = get_errno(safe_write(arg1, NULL, 0));
> +            break;
> +        }
>          if (!(p = lock_user(VERIFY_READ, arg2, arg3, 1)))
>              goto efault;
>          if (fd_trans_target_to_host_data(arg1)) {
> 

Reviewed-by: Laurent Vivier <laurent@vivier.eu>

Could you change the NR_read too, for consistency?

Thanks,
Laurent

Re: [Qemu-devel] [PATCH v2] syscall: fix special case of write(fd, NULL, 0)
Posted by Carlo Arenas 6 years, 5 months ago
Reviewed-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
Re: [Qemu-devel] [PATCH v2] syscall: fix special case of write(fd, NULL, 0)
Posted by Carlo Arenas 6 years, 5 months ago
after looking at this further, I am now convinced this patch (while
correct) is only addressing the symptom and not the root cause, and
therefore should be improved.

the smoking gun is that it is not needed, when the guest bitness is smaller
than the host (ex: qemu-i386 in an amd64 host) and the real problem is that
the current code assumes NULL is always an access failure, while it is the
right response when NULL is passed as the buffer parameter.

will update my proposed fix for pwrite64 with code that fixes both places
but that still matches the observed behaviour and throws and error when it
really needs to (as per the documentation) :

root@df571dc8e664:/usr/src/qemu# aarch64-linux-gnu-gcc -static -o t.arm64
t.c

*t.c:* In function '*main*':

*t.c:11:28:* *warning: *passing argument 2 of '*write*' makes pointer from
integer without a cast [*-Wint-conversion*]

    ssize_t ret = write(fd, *-*1, 0);

                            *^*

In file included from *t.c:2:0*:

*/usr/aarch64-linux-gnu/include/unistd.h:369:16:* *note: *expected '*const
void **' but argument is of type '*int*'

 extern ssize_t *write* (int __fd, const void *__buf, size_t __n) __wur;

                *^~~~~*

root@df571dc8e664:/usr/src/qemu# ./aarch64-linux-user/qemu-aarch64 ./t.arm64


write returned -1 with errno 14 (Bad address)

had also proposed a test case[1] but I suspect that qemu might had been
accidentally compliant since it is clear (at least from the documentation)
that an EFAULT might be a valid response as well, and it might be what is
returned at least on some linux systems using uclibc

Carlo

[1] https://github.com/linux-test-project/ltp/pull/217