diff mbox series

[v3,2/8] linux-user: Add "safe" parameter to do_guest_openat()

Message ID 20230606132743.1386003-3-iii@linux.ibm.com (mailing list archive)
State New, archived
Headers show
Series gdbstub: Add support for info proc mappings | expand

Commit Message

Ilya Leoshkevich June 6, 2023, 1:27 p.m. UTC
gdbstub cannot meaningfully handle QEMU_ERESTARTSYS, and it doesn't
need to. Add a parameter to do_guest_openat() that makes it use
openat() instead of safe_openat(), so that it becomes usable from
gdbstub.

Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
 linux-user/qemu.h    |  2 +-
 linux-user/syscall.c | 18 +++++++++++++-----
 2 files changed, 14 insertions(+), 6 deletions(-)

Comments

Richard Henderson June 6, 2023, 6:24 p.m. UTC | #1
On 6/6/23 06:27, Ilya Leoshkevich wrote:
> @@ -8518,7 +8522,11 @@ int do_guest_openat(CPUArchState *cpu_env, int dirfd, const char *pathname,
>           return fd;
>       }
>   
> -    return safe_openat(dirfd, path(pathname), flags, mode);
> +    if (safe) {
> +        return safe_openat(dirfd, path(pathname), flags, mode);
> +    } else {
> +        return openat(dirfd, path(pathname), flags, mode);
> +    }
>   }

I'm not keen on this, as it seems like the wrong abstraction.  But I can't immediately 
think of how it could be better structured.

The only concrete objection I have is the change of API, which could be fixed with return 
get_errno(openat(...)).

With that,

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~
Ilya Leoshkevich June 6, 2023, 7:29 p.m. UTC | #2
On Tue, 2023-06-06 at 11:24 -0700, Richard Henderson wrote:
> On 6/6/23 06:27, Ilya Leoshkevich wrote:
> > @@ -8518,7 +8522,11 @@ int do_guest_openat(CPUArchState *cpu_env,
> > int dirfd, const char *pathname,
> >           return fd;
> >       }
> >   
> > -    return safe_openat(dirfd, path(pathname), flags, mode);
> > +    if (safe) {
> > +        return safe_openat(dirfd, path(pathname), flags, mode);
> > +    } else {
> > +        return openat(dirfd, path(pathname), flags, mode);
> > +    }
> >   }
> 
> I'm not keen on this, as it seems like the wrong abstraction.  But I
> can't immediately 
> think of how it could be better structured.

I also thought about temporarily clearing signal_pending in gdbstub,
but could not convince myself that this were safe.

> The only concrete objection I have is the change of API, which could
> be fixed with return 
> get_errno(openat(...)).

I believe both openat() and safe_openat() return -1 on error and set
errno, or am I missing something?

> 
> With that,
> 
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> 
> 
> r~
Richard Henderson June 6, 2023, 8:35 p.m. UTC | #3
On 6/6/23 12:29, Ilya Leoshkevich wrote:
>> The only concrete objection I have is the change of API, which could
>> be fixed with return
>> get_errno(openat(...)).
> 
> I believe both openat() and safe_openat() return -1 on error and set
> errno, or am I missing something?
Oops, no, bad memory on my part -- I thought safe_foo returned -errno.


r~
diff mbox series

Patch

diff --git a/linux-user/qemu.h b/linux-user/qemu.h
index a5830ec2396..9b8e0860d70 100644
--- a/linux-user/qemu.h
+++ b/linux-user/qemu.h
@@ -166,7 +166,7 @@  typedef struct TaskState {
 
 abi_long do_brk(abi_ulong new_brk);
 int do_guest_openat(CPUArchState *cpu_env, int dirfd, const char *pathname,
-                    int flags, mode_t mode);
+                    int flags, mode_t mode, bool safe);
 ssize_t do_guest_readlink(const char *pathname, char *buf, size_t bufsiz);
 
 /* user access */
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 2d3070cfd62..28a0b1f7882 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -8449,7 +8449,7 @@  static int open_hardware(CPUArchState *cpu_env, int fd)
 #endif
 
 int do_guest_openat(CPUArchState *cpu_env, int dirfd, const char *pathname,
-                    int flags, mode_t mode)
+                    int flags, mode_t mode, bool safe)
 {
     struct fake_open {
         const char *filename;
@@ -8476,7 +8476,11 @@  int do_guest_openat(CPUArchState *cpu_env, int dirfd, const char *pathname,
     };
 
     if (is_proc_myself(pathname, "exe")) {
-        return safe_openat(dirfd, exec_path, flags, mode);
+        if (safe) {
+            return safe_openat(dirfd, exec_path, flags, mode);
+        } else {
+            return openat(dirfd, exec_path, flags, mode);
+        }
     }
 
     for (fake_open = fakes; fake_open->filename; fake_open++) {
@@ -8518,7 +8522,11 @@  int do_guest_openat(CPUArchState *cpu_env, int dirfd, const char *pathname,
         return fd;
     }
 
-    return safe_openat(dirfd, path(pathname), flags, mode);
+    if (safe) {
+        return safe_openat(dirfd, path(pathname), flags, mode);
+    } else {
+        return openat(dirfd, path(pathname), flags, mode);
+    }
 }
 
 ssize_t do_guest_readlink(const char *pathname, char *buf, size_t bufsiz)
@@ -9027,7 +9035,7 @@  static abi_long do_syscall1(CPUArchState *cpu_env, int num, abi_long arg1,
             return -TARGET_EFAULT;
         ret = get_errno(do_guest_openat(cpu_env, AT_FDCWD, p,
                                   target_to_host_bitmask(arg2, fcntl_flags_tbl),
-                                  arg3));
+                                  arg3, true));
         fd_trans_unregister(ret);
         unlock_user(p, arg1, 0);
         return ret;
@@ -9037,7 +9045,7 @@  static abi_long do_syscall1(CPUArchState *cpu_env, int num, abi_long arg1,
             return -TARGET_EFAULT;
         ret = get_errno(do_guest_openat(cpu_env, arg1, p,
                                   target_to_host_bitmask(arg3, fcntl_flags_tbl),
-                                  arg4));
+                                  arg4, true));
         fd_trans_unregister(ret);
         unlock_user(p, arg2, 0);
         return ret;