diff mbox series

[v4] linux-user: Add close_range() syscall

Message ID Y1b45IL371MJP2WW@p100 (mailing list archive)
State New, archived
Headers show
Series [v4] linux-user: Add close_range() syscall | expand

Commit Message

Helge Deller Oct. 24, 2022, 8:43 p.m. UTC 
Signed-off-by: Helge Deller <deller@gmx.de>
---
Changes:
v4: Fix check for arg2
v3: fd_trans_unregister() only called if close_range() doesn't fail
v2: consider CLOSE_RANGE_CLOEXEC flag

Comments

Richard Henderson Oct. 24, 2022, 10:39 p.m. UTC | #1 
On 10/25/22 06:43, Helge Deller wrote:
> +            abi_long maxfd = arg2;
> +
> +            if ((sizeof(abi_long) == 4 && arg2 == (abi_long)0x7FFFFFFFUL) ||
> +                (sizeof(abi_long) == 8 && arg2 == (abi_long)0x7FFFFFFFFFFFFFFFULL)) {
> +                maxfd = target_fd_max;
> +            }
> +
> +            for (fd = arg1; fd < maxfd; fd++) {

Why do we need explicit checks for INT32/64_MAX?  If the guest passes 
0x7FFFFFFFFFFFFFFEULL, do we really need to iterate over all of those impossible values?

I should think some expression involving MIN() is in order.


r~
Helge Deller Oct. 25, 2022, 1:39 a.m. UTC | #2 
On 10/25/22 00:39, Richard Henderson wrote:
> On 10/25/22 06:43, Helge Deller wrote:
>> +            abi_long maxfd = arg2;
>> +
>> +            if ((sizeof(abi_long) == 4 && arg2 == (abi_long)0x7FFFFFFFUL) ||
>> +                (sizeof(abi_long) == 8 && arg2 == (abi_long)0x7FFFFFFFFFFFFFFFULL)) {
>> +                maxfd = target_fd_max;
>> +            }
>> +
>> +            for (fd = arg1; fd < maxfd; fd++) {
>
> Why do we need explicit checks for INT32/64_MAX?
> If the guest passes 0x7FFFFFFFFFFFFFFEULL,

A 32-bit guest (on a 64bit host) will pass 0x7FFFFFFFUL...

> do we really need to iterate over all of those impossible values?

The compiler will optimize one of those checks away, so it's effectively
just one expression.

> I should think some expression involving MIN() is in order.

Helge
Helge Deller Oct. 25, 2022, 2:15 a.m. UTC | #3 
On 10/25/22 03:39, Helge Deller wrote:
> On 10/25/22 00:39, Richard Henderson wrote:
>> On 10/25/22 06:43, Helge Deller wrote:
>>> +            abi_long maxfd = arg2;
>>> +
>>> +            if ((sizeof(abi_long) == 4 && arg2 == (abi_long)0x7FFFFFFFUL) ||
>>> +                (sizeof(abi_long) == 8 && arg2 == (abi_long)0x7FFFFFFFFFFFFFFFULL)) {
>>> +                maxfd = target_fd_max;
>>> +            }
>>> +
>>> +            for (fd = arg1; fd < maxfd; fd++) {
>>
>> Why do we need explicit checks for INT32/64_MAX?
>> If the guest passes 0x7FFFFFFFFFFFFFFEULL,
>
> A 32-bit guest (on a 64bit host) will pass 0x7FFFFFFFUL...
>
>> do we really need to iterate over all of those impossible values?
>
> The compiler will optimize one of those checks away, so it's effectively
> just one expression.

My above comments are correct, but....

>> I should think some expression involving MIN() is in order.

that's even better.

Will resend v5 patch.

Helge
Richard Henderson Oct. 25, 2022, 2:20 a.m. UTC | #4 
On 10/25/22 11:39, Helge Deller wrote:
> On 10/25/22 00:39, Richard Henderson wrote:
>> On 10/25/22 06:43, Helge Deller wrote:
>>> +            abi_long maxfd = arg2;
>>> +
>>> +            if ((sizeof(abi_long) == 4 && arg2 == (abi_long)0x7FFFFFFFUL) ||
>>> +                (sizeof(abi_long) == 8 && arg2 == (abi_long)0x7FFFFFFFFFFFFFFFULL)) {
>>> +                maxfd = target_fd_max;
>>> +            }
>>> +
>>> +            for (fd = arg1; fd < maxfd; fd++) {
>>
>> Why do we need explicit checks for INT32/64_MAX?
>> If the guest passes 0x7FFFFFFFFFFFFFFEULL,
> 
> A 32-bit guest (on a 64bit host) will pass 0x7FFFFFFFUL...
> 
>> do we really need to iterate over all of those impossible values?
> 
> The compiler will optimize one of those checks away, so it's effectively
> just one expression.

By impossible values, I mean all descriptors above target_fd_max.
The compiler will most certainly not optimize the number of loop iterations.


r~
diff mbox series

Patch

diff --git a/linux-user/strace.list b/linux-user/strace.list
index 3df2184580..cd995e5d56 100644
--- a/linux-user/strace.list
+++ b/linux-user/strace.list
@@ -103,6 +103,9 @@ 
 #ifdef TARGET_NR_close
 { TARGET_NR_close, "close" , "%s(%d)", NULL, NULL },
 #endif
+#ifdef TARGET_NR_close_range
+{ TARGET_NR_close_range, "close_range" , "%s(%u,%u,%u)", NULL, NULL },
+#endif
 #ifdef TARGET_NR_connect
 { TARGET_NR_connect, "connect" , "%s(%d,%#x,%d)", NULL, NULL },
 #endif
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 12195d4e99..984039f928 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -339,6 +339,13 @@  _syscall3(int,sys_syslog,int,type,char*,bufp,int,len)
 #ifdef __NR_exit_group
 _syscall1(int,exit_group,int,error_code)
 #endif
+#if defined(__NR_close_range) && defined(TARGET_NR_close_range)
+#define __NR_sys_close_range __NR_close_range
+_syscall3(int,sys_close_range,int,first,int,last,int,flags)
+#ifndef CLOSE_RANGE_CLOEXEC
+#define CLOSE_RANGE_CLOEXEC     (1U << 2)
+#endif
+#endif
 #if defined(__NR_futex)
 _syscall6(int,sys_futex,int *,uaddr,int,op,int,val,
           const struct timespec *,timeout,int *,uaddr2,int,val3)
@@ -8735,6 +8742,24 @@  static abi_long do_syscall1(CPUArchState *cpu_env, int num, abi_long arg1,
     case TARGET_NR_close:
         fd_trans_unregister(arg1);
         return get_errno(close(arg1));
+#if defined(__NR_close_range) && defined(TARGET_NR_close_range)
+    case TARGET_NR_close_range:
+        ret = get_errno(sys_close_range(arg1, arg2, arg3));
+        if (ret == 0 && !(arg3 & CLOSE_RANGE_CLOEXEC)) {
+            abi_long fd;
+            abi_long maxfd = arg2;
+
+            if ((sizeof(abi_long) == 4 && arg2 == (abi_long)0x7FFFFFFFUL) ||
+                (sizeof(abi_long) == 8 && arg2 == (abi_long)0x7FFFFFFFFFFFFFFFULL)) {
+                maxfd = target_fd_max;
+            }
+
+            for (fd = arg1; fd < maxfd; fd++) {
+                fd_trans_unregister(fd);
+            }
+        }
+        return ret;
+#endif

     case TARGET_NR_brk:
         return do_brk(arg1);