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

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

[zhuoweizhang]

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)

[Laurent Vivier]

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)

[Carlo Arenas]

Reviewed-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>

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

[Carlo Arenas]

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

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

[Zhuowei zhang]

--------------------------------------------
On Sat, 9/30/17, Laurent Vivier <laurent@vivier.eu> wrote:
 
 > Could you change the NR_read too, for consistency?
 
The NR_read case for length 0 doesn't check if the buffer is NULL, unlike this patch, and I'm not sure if switching that to make a real read() syscall with a NULL buffer even if the original buffer is non-null would break anything.

Thanks
Zhuowei