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

[Qemu-devel] [PATCH] 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

```

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 returns success if found.

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..ecadf49 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 = 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] syscall: fix special case of write(fd, NULL, 0)

[Laurent Vivier]

Le 29/09/2017 à 18:50, 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
> 
> ```
> 
> 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 returns success if found.
> 
> 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..ecadf49 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 = 0;
> +            break;
> +        }
>          if (!(p = lock_user(VERIFY_READ, arg2, arg3, 1)))
>              goto efault;
>          if (fd_trans_target_to_host_data(arg1)) {
> 

I think we should keep the call to the kernel write() as the behavior
depends on the driver behind the syscall. Moreover, calling write() with
(NULL, 0) can triggers "something" at kernel level.

Thanks,
Laurent

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

[Peter Maydell]

On 29 September 2017 at 12:14, Laurent Vivier <laurent@vivier.eu> wrote:
> Le 29/09/2017 à 18:50, 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
>>
>> ```
>>
>> 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 returns success if found.
>>
>> 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..ecadf49 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 = 0;
>> +            break;
>> +        }
>>          if (!(p = lock_user(VERIFY_READ, arg2, arg3, 1)))
>>              goto efault;
>>          if (fd_trans_target_to_host_data(arg1)) {
>>
>
> I think we should keep the call to the kernel write() as the behavior
> depends on the driver behind the syscall. Moreover, calling write() with
> (NULL, 0) can triggers "something" at kernel level.

I tend to agree. On the other hand our handling of NR_read
with a zero count just returns 0 without calling the syscall;
perhaps we should change that for consistency?

Do we also have the same issue with pread64/pwrite64 ?

thanks
-- PMM

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

[Carlo Arenas]

a return of 0 is a valid response for write per POSIX and just indicates 0
bytes were written.

the use of NULL for the buffer is not a trigger for that behaviour, but the
fact that the third parameter asks for 0 bytes to be copied.

therefore there is no need to get logic in this handler to check for NULL
IMHO

Carlo

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

[Peter Maydell]

On 29 September 2017 at 16:33, Carlo Arenas <carenas@gmail.com> wrote:
> a return of 0 is a valid response for write per POSIX and just indicates 0
> bytes were written.

The interface we are implementing here is not the POSIX write()
function but the Linux write syscall, whose semantics are not
necessarily identical, though perhaps they may be in this case.

thanks
-- PMM

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

[Oliver Smith]

Hello there,

I'm a little late to the party. But what is necessary to get this
upstreamed, and how can I help?

PS: Sorry if I picked the wrong e-mail addresses, I wasn't subscribed to
the ML at that point and used the addresses I could find for the people
who answered to the original thread here:

https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg08073.html

Thanks,
Oliver


From: Zhuowei Zhang <address@hidden>
> Linux returns success for the special case of calling write with a
> zero-length NULL buffer: compiling and running
>
> ```
>
> 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 returns success if found.
>
> Signed-off-by: Zhuowei Zhang <address@hidden>
> ---
>  linux-user/syscall.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 9b6364a..ecadf49 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 = 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] syscall: fix special case of write(fd, NULL, 0)

[Laurent Vivier]

Le 15/02/2018 à 00:33, Oliver Smith a écrit :
> Hello there,
> 
> I'm a little late to the party. But what is necessary to get this
> upstreamed, and how can I help?
> 
> PS: Sorry if I picked the wrong e-mail addresses, I wasn't subscribed to
> the ML at that point and used the addresses I could find for the people
> who answered to the original thread here:
> 
> https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg08073.html

According to comments in the ML thread, you need to:

- update the patch to call write() with NULL and 0, something like:

--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -7912,6 +7912,10 @@ abi_long do_syscall(void *cpu_env, int num,
abi_long arg1,
         }
         break;
     case TARGET_NR_write:
+        if (arg2 == 0 && arg3 == 0) {
+            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)) {

- change TARGET_NR_read to do the same

- check if we need to do the same for pread64/pwrite64

Thanks,
Laurent