From: Paul Floyd Date: Tue, 31 Jan 2023 20:52:36 +0000 (+0100) Subject: FreeBSD: cleanup and refactor syscalls readlink and readlinkat X-Git-Tag: VALGRIND_3_21_0~186 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=b1aba91160c53326eb3b41e8fd6c9a5c00902b24;p=thirdparty%2Fvalgrind.git FreeBSD: cleanup and refactor syscalls readlink and readlinkat There was some code to handle /proc/curproc/file (a symlink to the exe that wee need to bodge as it refers to the tool exe). But it was neither tested nor working. Can't use the same technique as Linux and Solaris which have more complete /proc filesystems where each pid has symlinks for each open file, which we use for the guest. Instead need to copy the path ourselves. So move sys_readlink out of generic. Simplify the handling of the resolved guest exe name - store it in a global like VG_(args_the_exename). --- diff --git a/.gitignore b/.gitignore index ae71cbbbdd..cc6b189b4b 100644 --- a/.gitignore +++ b/.gitignore @@ -2101,6 +2101,7 @@ /none/tests/freebsd/hello_world /none/tests/freebsd/452275 /none/tests/freebsd/usrstack +/none/tests/freebsd/proc_pid_file # /none/tests/x86/ /none/tests/x86/*.dSYM diff --git a/coregrind/m_clientstate.c b/coregrind/m_clientstate.c index 7b343f27a5..404a60f372 100644 --- a/coregrind/m_clientstate.c +++ b/coregrind/m_clientstate.c @@ -93,6 +93,10 @@ Int VG_(args_for_valgrind_noexecpass) = 0; line. */ const HChar* VG_(args_the_exename) = NULL; +/* The real name of the executable, with resolved + * relative paths and symlinks */ +const HChar* VG_(resolved_exename) = NULL; + // Client's original rlimit data and rlimit stack struct vki_rlimit VG_(client_rlimit_data); struct vki_rlimit VG_(client_rlimit_stack); diff --git a/coregrind/m_initimg/initimg-freebsd.c b/coregrind/m_initimg/initimg-freebsd.c index 22c210cdeb..53a9aca873 100644 --- a/coregrind/m_initimg/initimg-freebsd.c +++ b/coregrind/m_initimg/initimg-freebsd.c @@ -362,8 +362,7 @@ static HChar *copy_bytes(HChar **tab, const HChar *src, SizeT size) ---------------------------------------------------------------- */ -static -struct auxv *find_auxv(UWord* sp) +static struct auxv *find_auxv(UWord* sp) { sp++; // skip argc (Nb: is word-sized, not int-sized!) @@ -380,13 +379,12 @@ struct auxv *find_auxv(UWord* sp) return (struct auxv *)sp; } -static -Addr setup_client_stack( void* init_sp, - HChar** orig_envp, - const ExeInfo* info, - UInt** client_auxv, - Addr clstack_end, - SizeT clstack_max_size ) +static Addr setup_client_stack(void* init_sp, + HChar** orig_envp, + const ExeInfo* info, + UInt** client_auxv, + Addr clstack_end, + SizeT clstack_max_size ) { SysRes res; HChar **cpp; @@ -690,6 +688,7 @@ Addr setup_client_stack( void* init_sp, case VKI_AT_EXECPATH: auxv->u.a_ptr = copy_str(&strtab, resolved_name); + VG_(resolved_exename) = auxv->u.a_ptr; break; case VKI_AT_CANARY: if (canarylen >= 1) { @@ -795,6 +794,11 @@ Addr setup_client_stack( void* init_sp, if (0) { VG_(printf)("startup SP = %#lx\n", client_SP); } + + if (VG_(resolved_exename) == NULL) { + VG_(resolved_exename) = VG_(strdup)("initimg-freebsd.sre.1", resolved_name); + } + return client_SP; } diff --git a/coregrind/m_main.c b/coregrind/m_main.c index 4316e625fa..0a7e96e500 100644 --- a/coregrind/m_main.c +++ b/coregrind/m_main.c @@ -1724,6 +1724,13 @@ Int valgrind_main ( Int argc, HChar **argv, HChar **envp ) if (!sr_isError(statres) || VKI_S_ISLNK(statbuf.mode)) { VG_(have_slash_proc) = True; } + // each directory contains the following that might get read + // file - a symlink to the exe + // cmdline - null separate command line + // etype - the executable type e.g., FreeBSD ELF64 (same for guest and host) + // map - a memory map, tricky to synthesize + // rlimit - list of process limits + // status - process, pid, ppid pts cty uid gid and some other stuff } #endif diff --git a/coregrind/m_syswrap/priv_syswrap-freebsd.h b/coregrind/m_syswrap/priv_syswrap-freebsd.h index dd065c6f58..8822f22f21 100644 --- a/coregrind/m_syswrap/priv_syswrap-freebsd.h +++ b/coregrind/m_syswrap/priv_syswrap-freebsd.h @@ -90,7 +90,7 @@ DECL_TEMPLATE(freebsd, sys_ioctl) // 54 DECL_TEMPLATE(freebsd, sys_reboot) // 55 DECL_TEMPLATE(freebsd, sys_revoke) // 56 // generic symlink 57 -// generic readlink 58 +DECL_TEMPLATE(freebsd, sys_readlink) // 58 // generic execve 59 // generic umask 60 // generic chroot 61 diff --git a/coregrind/m_syswrap/syswrap-freebsd.c b/coregrind/m_syswrap/syswrap-freebsd.c index 13d3b6bf25..1ab63ba491 100644 --- a/coregrind/m_syswrap/syswrap-freebsd.c +++ b/coregrind/m_syswrap/syswrap-freebsd.c @@ -1139,8 +1139,52 @@ PRE(sys_revoke) // SYS_symlink 57 // generic +static void do_readlink(const HChar* path, HChar *buf, SizeT bufsize, SyscallStatus* status, Bool* curproc_file) +{ + HChar name[30]; + VG_(sprintf)(name, "/proc/%d/file", VG_(getpid)()); + if (ML_(safe_to_deref)(path, 1) + && (VG_(strcmp)(path, name) == 0 + || VG_(strcmp)(path, "/proc/curproc/file") == 0)) { + vg_assert(VG_(resolved_exename)); + Int len = VG_(snprintf)(buf, bufsize, "%s", VG_(resolved_exename)); + SET_STATUS_Success(len); + *curproc_file = True; + } +} + // SYS_readlink 58 -// generic +// ssize_t readlink(const char *restrict path, char *restrict buf, size_t bufsiz); +PRE(sys_readlink) +{ + FUSE_COMPATIBLE_MAY_BLOCK(); + Word saved = SYSNO; + Bool curproc_file = False; + + PRINT("sys_readlink ( %#" FMT_REGWORD "x(%s), %#" FMT_REGWORD "x, %llu )", + ARG1, (char*)(Addr)ARG1, ARG2, (ULong)ARG3); + PRE_REG_READ3(long, "readlink", + const char *, path, char *, buf, int, bufsiz); + PRE_MEM_RASCIIZ( "readlink(path)", ARG1 ); + PRE_MEM_WRITE( "readlink(buf)", ARG2,ARG3 ); + + if (VG_(have_slash_proc) == True) + { + /* + * Handle the case where readlink is looking at /proc/curproc/file or + * /proc//file + */ + do_readlink((const HChar *)ARG1, (HChar *)ARG2, (SizeT)ARG3, status, &curproc_file); + } + + if (!curproc_file) { + /* Normal case */ + SET_STATUS_from_SysRes( VG_(do_syscall3)(saved, ARG1, ARG2, ARG3)); + } + if (SUCCESS && RES > 0) { + POST_MEM_WRITE( ARG2, RES ); + } +} // SYS_execve 59 // generic @@ -1941,8 +1985,7 @@ static void sysctl_kern_usrstack(SizeT* out, SizeT* outlen) static Bool sysctl_kern_proc_pathname(HChar *out, SizeT *len) { - // is this stashed somewhere? - const HChar *exe_name = VG_(find_executable)(VG_(args_the_exename)); + const HChar *exe_name = VG_(resolved_exename); if (!VG_(realpath)(exe_name, out)) { return False; @@ -5325,8 +5368,8 @@ POST(sys_openat) // size_t bufsize); PRE(sys_readlinkat) { - HChar name[25]; Word saved = SYSNO; + Bool curproc_file = False; PRINT("sys_readlinkat ( %" FMT_REGWORD "u, %#" FMT_REGWORD "x(%s), %#" FMT_REGWORD "x, %llu )", ARG1,ARG2,(char*)ARG2,ARG3,(ULong)ARG4); PRE_REG_READ4(ssize_t, "readlinkat", @@ -5334,23 +5377,24 @@ PRE(sys_readlinkat) PRE_MEM_RASCIIZ( "readlinkat(path)", ARG2 ); PRE_MEM_WRITE( "readlinkat(buf)", ARG3,ARG4 ); - if (VG_(have_slash_proc) == True) { + if (VG_(have_slash_proc) == True && (Int)ARG1 == VKI_AT_FDCWD) { /* * Handle the case where readlinkat is looking at /proc/curproc/file or * /proc//file. */ - VG_(sprintf)(name, "/proc/%d/file", VG_(getpid)()); - if (ML_(safe_to_deref)((void*)ARG2, 1) - && (VG_(strcmp)((HChar *)ARG2, name) == 0 - || VG_(strcmp)((HChar *)ARG2, "/proc/curproc/file") == 0)) { - VG_(sprintf)(name, "/proc/curproc/fd/%d", VG_(cl_exec_fd)); - SET_STATUS_from_SysRes( VG_(do_syscall4)(saved, ARG1, (UWord)name, - ARG3, ARG4)); - return; - } + do_readlink((const HChar *)ARG2, (HChar *)ARG3, (SizeT)ARG4, status, &curproc_file); + } + + // @todo PJF there is still the case where fd refers to /proc or /proc/pid + // or /proc/curproc and path is relative pid/file, curptoc/file or just file + + if (!curproc_file) { + /* Normal case */ + SET_STATUS_from_SysRes( VG_(do_syscall4)(saved, ARG1, ARG2, ARG3, ARG4)); + } + if (SUCCESS && RES > 0) { + POST_MEM_WRITE( ARG3, RES ); } - /* Normal case */ - SET_STATUS_from_SysRes( VG_(do_syscall4)(saved, ARG1, ARG2, ARG3, ARG4)); } POST(sys_readlinkat) @@ -6696,7 +6740,7 @@ const SyscallTableEntry ML_(syscall_table)[] = { BSDX_(__NR_revoke, sys_revoke), // 56 GENX_(__NR_symlink, sys_symlink), // 57 - GENX_(__NR_readlink, sys_readlink), // 58 + BSDX_(__NR_readlink, sys_readlink), // 58 GENX_(__NR_execve, sys_execve), // 59 GENX_(__NR_umask, sys_umask), // 60 diff --git a/include/pub_tool_clientstate.h b/include/pub_tool_clientstate.h index 556ab03dd5..e25a59702a 100644 --- a/include/pub_tool_clientstate.h +++ b/include/pub_tool_clientstate.h @@ -63,6 +63,7 @@ extern Int VG_(args_for_valgrind_noexecpass); line. */ extern const HChar* VG_(args_the_exename); +extern const HChar* VG_(resolved_exename); #endif // __PUB_TOOL_CLIENTSTATE_H diff --git a/none/tests/freebsd/Makefile.am b/none/tests/freebsd/Makefile.am index 5fe5de3627..f956078d68 100644 --- a/none/tests/freebsd/Makefile.am +++ b/none/tests/freebsd/Makefile.am @@ -35,10 +35,13 @@ EXTRA_DIST = \ 452275.stderr.exp \ usrstack.vgtest \ usrstack.stderr.exp \ - usrstack.stdout.exp + usrstack.stdout.exp \ + proc_pid_file.vgtest \ + proc_pid_file.stderr.exp check_PROGRAMS = \ - auxv osrel swapcontext hello_world fexecve 452275 usrstack + auxv osrel swapcontext hello_world fexecve 452275 usrstack \ + proc_pid_file AM_CFLAGS += $(AM_FLAG_M3264_PRI) AM_CXXFLAGS += $(AM_FLAG_M3264_PRI) @@ -48,3 +51,5 @@ osrel_CFLAGS = ${AM_CFLAGS} swapcontext_CFLAGS = ${AM_CFLAGS} hello_world_SOURCES = hello_world.cpp + +proc_pid_file_SOURCES = proc_pid_file.cpp diff --git a/none/tests/freebsd/proc_pid_file.cpp b/none/tests/freebsd/proc_pid_file.cpp new file mode 100644 index 0000000000..cf9d185381 --- /dev/null +++ b/none/tests/freebsd/proc_pid_file.cpp @@ -0,0 +1,27 @@ +#include +//#include +#include +//#include +#include +#include +#include + +int main() +{ + char resolvedPath[PATH_MAX]; + auto count = readlink("/proc/curproc/file", resolvedPath, PATH_MAX); + resolvedPath[count] = '\0'; + //std::cout << "resolvedPath: " << resolvedPath << '\n'; + char resolvedPath2[PATH_MAX]; + auto count2 = readlinkat(AT_FDCWD, "/proc/curproc/file", resolvedPath2, PATH_MAX); + resolvedPath2[count2] = '\0'; + //std::cout << "resolvedPath2: " << resolvedPath2 << '\n'; + std::string rp(resolvedPath); + assert(rp == resolvedPath2); + + auto n = rp.rfind("proc_pid_file"); + + std::string filename(rp.substr(n)); + //std::cout << "filename: " << filename << '\n'; + assert(filename == "proc_pid_file"); +} diff --git a/none/tests/freebsd/proc_pid_file.stderr.exp b/none/tests/freebsd/proc_pid_file.stderr.exp new file mode 100644 index 0000000000..e69de29bb2 diff --git a/none/tests/freebsd/proc_pid_file.vgtest b/none/tests/freebsd/proc_pid_file.vgtest new file mode 100644 index 0000000000..0361a5982b --- /dev/null +++ b/none/tests/freebsd/proc_pid_file.vgtest @@ -0,0 +1,3 @@ +prog: proc_pid_file +vgopts: -q +