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

zhuoweizhang--- via Qemu-devel posted 1 patch 6 years, 6 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/1506703816-7359-1-git-send-email-zhuoweizhang@yahoo.com
Test checkpatch passed
Test docker passed
Test s390x passed
There is a newer version of this series
linux-user/syscall.c | 5 +++++
1 file changed, 5 insertions(+)
[Qemu-devel] [PATCH] syscall: fix special case of write(fd, NULL, 0)
Posted by zhuoweizhang--- via Qemu-devel 6 years, 6 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

```

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)
Posted by Laurent Vivier 6 years, 6 months ago
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)
Posted by Peter Maydell 6 years, 6 months ago
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)
Posted by Carlo Arenas 6 years, 6 months ago
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)
Posted by Peter Maydell 6 years, 6 months ago
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