The accept4() syscall takes two flags only: SOCK_NONBLOCK and
SOCK_CLOEXEC.
Even the real Linux kernel returns -EINVAL if any other bits
have been set.

Change the implementation of accept4() to recognize those two values
only, instead of using the fcntl_flags_tbl[] bitmask translation.

Beside this correction in behaviour, it actually fixes the accept4()
emulation for hppa, mips and alpha targets for which SOCK_NONBLOCK is
different than TARGET_SOCK_NONBLOCK.

I noticed this wrong behaviour with the testcase of the debian lwt package
which failed (by timeout while hanging in the read() syscall) in qemu but
succeeded on real hardware.

Signed-off-by: Helge Deller <deller@gmx.de>
---
 linux-user/syscall.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 3f1e8e7ad9..9e9317237d 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -3440,7 +3440,18 @@ static abi_long do_accept4(int fd, abi_ulong target_addr,
     abi_long ret;
     int host_flags;

-    host_flags = target_to_host_bitmask(flags, fcntl_flags_tbl);
+    host_flags = 0;
+#if defined(SOCK_NONBLOCK)
+    if (flags & ~(TARGET_SOCK_CLOEXEC | TARGET_SOCK_NONBLOCK)) {
+        return -TARGET_EINVAL;
+    }
+    if (flags & TARGET_SOCK_NONBLOCK) {
+        host_flags |= SOCK_NONBLOCK;
+    }
+#endif
+    if (flags & TARGET_SOCK_CLOEXEC) {
+        host_flags |= SOCK_CLOEXEC;
+    }

     if (target_addr == 0) {
         return get_errno(safe_accept4(fd, NULL, NULL, host_flags));
--
2.41.0

On 7/7/23 14:19, Helge Deller wrote:
> The accept4() syscall takes two flags only: SOCK_NONBLOCK and
> SOCK_CLOEXEC.
> Even the real Linux kernel returns -EINVAL if any other bits
> have been set.
> 
> Change the implementation of accept4() to recognize those two values
> only, instead of using the fcntl_flags_tbl[] bitmask translation.
> 
> Beside this correction in behaviour, it actually fixes the accept4()
> emulation for hppa, mips and alpha targets for which SOCK_NONBLOCK is
> different than TARGET_SOCK_NONBLOCK.
> 
> I noticed this wrong behaviour with the testcase of the debian lwt package
> which failed (by timeout while hanging in the read() syscall) in qemu but
> succeeded on real hardware.
> 
> Signed-off-by: Helge Deller <deller@gmx.de>
> ---
>   linux-user/syscall.c | 13 ++++++++++++-
>   1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 3f1e8e7ad9..9e9317237d 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -3440,7 +3440,18 @@ static abi_long do_accept4(int fd, abi_ulong target_addr,
>       abi_long ret;
>       int host_flags;
> 
> -    host_flags = target_to_host_bitmask(flags, fcntl_flags_tbl);
> +    host_flags = 0;
> +#if defined(SOCK_NONBLOCK)
> +    if (flags & ~(TARGET_SOCK_CLOEXEC | TARGET_SOCK_NONBLOCK)) {
> +        return -TARGET_EINVAL;
> +    }
> +    if (flags & TARGET_SOCK_NONBLOCK) {
> +        host_flags |= SOCK_NONBLOCK;
> +    }
> +#endif

Can we avoid the ifdef?  Anyway, surely the TARGET bit check should not be protected by 
the #ifdef.


r~

On 7/7/23 22:15, Richard Henderson wrote:
> On 7/7/23 14:19, Helge Deller wrote:
>> The accept4() syscall takes two flags only: SOCK_NONBLOCK and
>> SOCK_CLOEXEC.
>> Even the real Linux kernel returns -EINVAL if any other bits
>> have been set.
>>
>> Change the implementation of accept4() to recognize those two values
>> only, instead of using the fcntl_flags_tbl[] bitmask translation.
>>
>> Beside this correction in behaviour, it actually fixes the accept4()
>> emulation for hppa, mips and alpha targets for which SOCK_NONBLOCK is
>> different than TARGET_SOCK_NONBLOCK.
>>
>> I noticed this wrong behaviour with the testcase of the debian lwt package
>> which failed (by timeout while hanging in the read() syscall) in qemu but
>> succeeded on real hardware.
>>
>> Signed-off-by: Helge Deller <deller@gmx.de>
>> ---
>>   linux-user/syscall.c | 13 ++++++++++++-
>>   1 file changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>> index 3f1e8e7ad9..9e9317237d 100644
>> --- a/linux-user/syscall.c
>> +++ b/linux-user/syscall.c
>> @@ -3440,7 +3440,18 @@ static abi_long do_accept4(int fd, abi_ulong target_addr,
>>       abi_long ret;
>>       int host_flags;
>>
>> -    host_flags = target_to_host_bitmask(flags, fcntl_flags_tbl);
>> +    host_flags = 0;
>> +#if defined(SOCK_NONBLOCK)
>> +    if (flags & ~(TARGET_SOCK_CLOEXEC | TARGET_SOCK_NONBLOCK)) {
>> +        return -TARGET_EINVAL;
>> +    }
>> +    if (flags & TARGET_SOCK_NONBLOCK) {
>> +        host_flags |= SOCK_NONBLOCK;
>> +    }
>> +#endif
>
> Can we avoid the ifdef?

I don't know. There are multiple such SOCK_NONBLOCK checks in the code.

> Anyway, surely the TARGET bit check should not be protected by the #ifdef.

Ok.

Helge