| 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 |
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~
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~
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 --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;
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(-)