From: Nicholas Nethercote Date: Fri, 14 Oct 2005 03:11:30 +0000 (+0000) Subject: Overhaul the way programs are loaded at startup and exec() works. Now the X-Git-Tag: svn/VALGRIND_3_1_0~314 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=82c3ab0af9041bb0a42aea0b0e44f4abb9dc4a6c;p=thirdparty%2Fvalgrind.git Overhaul the way programs are loaded at startup and exec() works. Now the checking of programs done in these two places are combined, which avoids duplicate code and greatly reduces the number of cases in which exec() fails causing Valgrind to bomb out. Also, we can now load some programs we could not previously, such as scripts lacking a "#!" line at the start. Also, the startup failure messages for bad programs match the shell's messages very closely. And I added a whole bunch of regtests to test all this. git-svn-id: svn://svn.valgrind.org/valgrind/trunk@4918 --- diff --git a/coregrind/m_libcfile.c b/coregrind/m_libcfile.c index f86f2e2772..413bd11a3d 100644 --- a/coregrind/m_libcfile.c +++ b/coregrind/m_libcfile.c @@ -138,6 +138,14 @@ Int VG_(fsize) ( Int fd ) return res.isError ? (-1) : buf.st_size; } +Bool VG_(is_dir) ( HChar* f ) +{ + struct vki_stat buf; + SysRes res = VG_(do_syscall2)(__NR_stat, (UWord)f, (UWord)&buf); + return res.isError ? False + : VKI_S_ISDIR(buf.st_mode) ? True : False; +} + SysRes VG_(dup) ( Int oldfd ) { return VG_(do_syscall1)(__NR_dup, oldfd); @@ -207,6 +215,66 @@ Int VG_(access) ( HChar* path, Bool irusr, Bool iwusr, Bool ixusr ) #endif } +/* + Emulate the normal Unix permissions checking algorithm. + + If owner matches, then use the owner permissions, else + if group matches, then use the group permissions, else + use other permissions. + + Note that we can't deal with SUID/SGID, so we refuse to run them + (otherwise the executable may misbehave if it doesn't have the + permissions it thinks it does). +*/ +/* returns: 0 = success, non-0 is failure */ +Int VG_(check_executable)(HChar* f) +{ + struct vki_stat st; + SysRes res; + + res = VG_(stat)(f, &st); + if (res.isError) { + return res.val; + } + + if (st.st_mode & (VKI_S_ISUID | VKI_S_ISGID)) { + //VG_(printf)("Can't execute suid/sgid executable %s\n", exe); + return VKI_EACCES; + } + + if (VG_(geteuid)() == st.st_uid) { + if (!(st.st_mode & VKI_S_IXUSR)) + return VKI_EACCES; + } else { + int grpmatch = 0; + + if (VG_(getegid)() == st.st_gid) + grpmatch = 1; + else { + UInt groups[32]; + Int ngrp = VG_(getgroups)(32, groups); + Int i; + /* ngrp will be -1 if VG_(getgroups) failed. */ + for (i = 0; i < ngrp; i++) { + if (groups[i] == st.st_gid) { + grpmatch = 1; + break; + } + } + } + + if (grpmatch) { + if (!(st.st_mode & VKI_S_IXGRP)) { + return VKI_EACCES; + } + } else if (!(st.st_mode & VKI_S_IXOTH)) { + return VKI_EACCES; + } + } + + return 0; +} + SysRes VG_(pread) ( Int fd, void* buf, Int count, Int offset ) { OffT off = VG_(lseek)( fd, (OffT)offset, VKI_SEEK_SET); diff --git a/coregrind/m_main.c b/coregrind/m_main.c index a76bc4722c..8727d7ff5d 100644 --- a/coregrind/m_main.c +++ b/coregrind/m_main.c @@ -724,37 +724,56 @@ static Bool scan_colsep(char *colsep, Bool (*func)(const char *)) } /* Need a static copy because can't use dynamic mem allocation yet */ -static HChar executable_name[VKI_PATH_MAX]; +static HChar executable_name_in [VKI_PATH_MAX]; +static HChar executable_name_out[VKI_PATH_MAX]; static Bool match_executable(const char *entry) { - HChar buf[VG_(strlen)(entry) + VG_(strlen)(executable_name) + 3]; + HChar buf[VG_(strlen)(entry) + VG_(strlen)(executable_name_in) + 3]; - /* empty PATH element means . */ + /* empty PATH element means '.' */ if (*entry == '\0') entry = "."; - VG_(snprintf)(buf, sizeof(buf), "%s/%s", entry, executable_name); + VG_(snprintf)(buf, sizeof(buf), "%s/%s", entry, executable_name_in); + + // Don't match directories + if (VG_(is_dir)(buf)) + return False; + + // If we match an executable, we choose that immediately. If we find a + // matching non-executable we remember it but keep looking for an + // matching executable later in the path. if (VG_(access)(buf, True/*r*/, False/*w*/, True/*x*/) == 0) { - VG_(strncpy)( executable_name, buf, VKI_PATH_MAX-1 ); - executable_name[VKI_PATH_MAX-1] = 0; - return True; + VG_(strncpy)( executable_name_out, buf, VKI_PATH_MAX-1 ); + executable_name_out[VKI_PATH_MAX-1] = 0; + return True; // Stop looking + } else if (VG_(access)(buf, True/*r*/, False/*w*/, False/*x*/) == 0 + && VG_STREQ(executable_name_out, "")) + { + VG_(strncpy)( executable_name_out, buf, VKI_PATH_MAX-1 ); + executable_name_out[VKI_PATH_MAX-1] = 0; + return False; // Keep looking + } else { + return False; // Keep looking } - return False; } +// Returns NULL if it wasn't found. static HChar* find_executable ( HChar* exec ) { vg_assert(NULL != exec); - VG_(strncpy)( executable_name, exec, VKI_PATH_MAX-1 ); - executable_name[VKI_PATH_MAX-1] = 0; - - if (VG_(strchr)(executable_name, '/') == NULL) { - /* no '/' - we need to search the path */ + if (VG_(strchr)(exec, '/')) { + // Has a '/' - use the name as is + VG_(strncpy)( executable_name_out, exec, VKI_PATH_MAX-1 ); + } else { + // No '/' - we need to search the path + VG_(strncpy)( executable_name_in, exec, VKI_PATH_MAX-1 ); + VG_(memset) ( executable_name_out, 0, VKI_PATH_MAX ); HChar *path = VG_(getenv)("PATH"); scan_colsep(path, match_executable); } - return executable_name; + return VG_STREQ(executable_name_out, "") ? NULL : executable_name_out; } @@ -802,27 +821,29 @@ static void config_error ( Char* msg ) static void load_client ( /*OUT*/struct exeinfo* info, /*OUT*/Addr* client_eip) { - HChar* exec; + HChar* exe_name; Int ret; SysRes res; vg_assert( VG_(args_the_exename) != NULL); - exec = find_executable( VG_(args_the_exename) ); + exe_name = find_executable( VG_(args_the_exename) ); + + if (!exe_name) { + VG_(printf)("valgrind: %s: command not found\n", VG_(args_the_exename)); + VG_(exit)(127); // 127 is Posix NOTFOUND + } VG_(memset)(info, 0, sizeof(*info)); info->exe_base = VG_(client_base); info->exe_end = VG_(client_end); - ret = VG_(do_exec)(exec, info); - if (ret != 0) { - VG_(printf)("valgrind: do_exec(%s) failed: %s\n", - exec, VG_(strerror)(ret)); - VG_(exit)(127); - } + ret = VG_(do_exec)(exe_name, info); + + // The client was successfully loaded! Continue. /* Get hold of a file descriptor which refers to the client executable. This is needed for attaching to GDB. */ - res = VG_(open)(exec, VKI_O_RDONLY, VKI_S_IRUSR); + res = VG_(open)(exe_name, VKI_O_RDONLY, VKI_S_IRUSR); if (!res.isError) VG_(cl_exec_fd) = res.val; diff --git a/coregrind/m_syswrap/syswrap-generic.c b/coregrind/m_syswrap/syswrap-generic.c index 005197f457..ba07e6db49 100644 --- a/coregrind/m_syswrap/syswrap-generic.c +++ b/coregrind/m_syswrap/syswrap-generic.c @@ -50,6 +50,7 @@ #include "pub_core_syscall.h" #include "pub_core_syswrap.h" #include "pub_core_tooliface.h" +#include "pub_core_ume.h" #include "priv_types_n_macros.h" #include "priv_syswrap-generic.h" @@ -2288,6 +2289,7 @@ PRE(sys_execve) Char* launcher_basename = NULL; ThreadState* tst; Int i, j, tot_args; + SysRes res; PRINT("sys_execve ( %p(%s), %p, %p )", ARG1, ARG1, ARG2, ARG3); PRE_REG_READ3(vki_off_t, "execve", @@ -2307,31 +2309,21 @@ PRE(sys_execve) POST(execve), but that's close to impossible. Instead, we make an effort to check that the execve will work before actually doing it. */ - { - struct vki_stat st; - SysRes r = VG_(stat)((Char *)ARG1, &st); - - if (r.isError) { - /* stat failed */ - SET_STATUS_from_SysRes( r ); - return; - } - /* just look for regular file with any X bit set - XXX do proper permissions check? - */ - if ((st.st_mode & 0100111) == 0100000) { - SET_STATUS_Failure( VKI_EACCES ); - return; - } - } - /* Check more .. that the name at least begins in client-accessible - storage. */ + /* Check that the name at least begins in client-accessible storage. */ if (!VG_(am_is_valid_for_client)( ARG1, 1, VKI_PROT_READ )) { SET_STATUS_Failure( VKI_EFAULT ); return; } + // Do the important checks: it is a file, is executable, permissions are + // ok, etc. + res = VG_(pre_exec_check)((const Char*)ARG1, NULL); + if (res.isError) { + SET_STATUS_Failure( res.val ); + return; + } + /* If we're tracing the child, and the launcher name looks bogus (possibly because launcher.c couldn't figure it out, see comments therein) then we have no option but to fail. */ diff --git a/coregrind/m_ume.c b/coregrind/m_ume.c index 40091f722f..3c63c9d78d 100644 --- a/coregrind/m_ume.c +++ b/coregrind/m_ume.c @@ -38,6 +38,7 @@ // included ahead of the glibc ones. This fix is a kludge; the right // solution is to entirely remove the glibc dependency. #include "pub_core_basics.h" +#include "pub_core_aspacemgr.h" // various mapping fns #include "pub_core_debuglog.h" #include "pub_core_libcbase.h" #include "pub_core_machine.h" @@ -45,9 +46,8 @@ #include "pub_core_libcfile.h" // VG_(close) et al #include "pub_core_libcproc.h" // VG_(geteuid), VG_(getegid) #include "pub_core_libcassert.h" // VG_(exit), vg_assert -#include "pub_core_syscall.h" // VG_(strerror) #include "pub_core_mallocfree.h" // VG_(malloc), VG_(free) -#include "pub_core_aspacemgr.h" // various mapping fns +#include "pub_core_syscall.h" // VG_(strerror) #include "vki_unistd.h" // mmap-related constants #include "pub_core_ume.h" @@ -65,7 +65,7 @@ struct elfinfo { ESZ(Ehdr) e; ESZ(Phdr) *p; - int fd; + Int fd; }; static void check_mmap(SysRes res, Addr base, SizeT len) @@ -77,165 +77,6 @@ static void check_mmap(SysRes res, Addr base, SizeT len) } } -//zz // 'extra' allows the caller to pass in extra args to 'fn', like free -//zz // variables to a closure. -//zz void VG_(foreach_map)(int (*fn)(char *start, char *end, -//zz const char *perm, off_t offset, -//zz int maj, int min, int ino, void* extra), -//zz void* extra) -//zz { -//zz static char buf[10240]; -//zz char *bufptr = buf; -//zz int ret, fd; -//zz -//zz fd = open("/proc/self/maps", O_RDONLY); -//zz -//zz if (fd == -1) { -//zz perror("open /proc/self/maps"); -//zz return; -//zz } -//zz -//zz ret = read(fd, buf, sizeof(buf)); -//zz -//zz if (ret == -1) { -//zz perror("read /proc/self/maps"); -//zz close(fd); -//zz return; -//zz } -//zz close(fd); -//zz -//zz if (ret == sizeof(buf)) { -//zz VG_(printf)("coregrind/m_ume.c: buf too small\n"); -//zz return; -//zz } -//zz -//zz while(bufptr && bufptr < buf+ret) { -//zz char perm[5]; -//zz ULong offset; -//zz int maj, min; -//zz int ino; -//zz void *segstart, *segend; -//zz -//zz sscanf(bufptr, "%p-%p %s %llx %x:%x %d", -//zz &segstart, &segend, perm, &offset, &maj, &min, &ino); -//zz bufptr = strchr(bufptr, '\n'); -//zz if (bufptr != NULL) -//zz bufptr++; /* skip \n */ -//zz -//zz if (!(*fn)(segstart, segend, perm, offset, maj, min, ino, extra)) -//zz break; -//zz } -//zz } -//zz -//zz /*------------------------------------------------------------*/ -//zz /*--- Stack switching ---*/ -//zz /*------------------------------------------------------------*/ -//zz -//zz // __attribute__((noreturn)) -//zz // void VG_(jump_and_switch_stacks) ( Addr stack, Addr dst ); -//zz #if defined(VGA_x86) -//zz // 4(%esp) == stack -//zz // 8(%esp) == dst -//zz asm( -//zz ".global vgPlain_jump_and_switch_stacks\n" -//zz "vgPlain_jump_and_switch_stacks:\n" -//zz " movl %esp, %esi\n" // remember old stack pointer -//zz " movl 4(%esi), %esp\n" // set stack -//zz " pushl 8(%esi)\n" // dst to stack -//zz " movl $0, %eax\n" // zero all GP regs -//zz " movl $0, %ebx\n" -//zz " movl $0, %ecx\n" -//zz " movl $0, %edx\n" -//zz " movl $0, %esi\n" -//zz " movl $0, %edi\n" -//zz " movl $0, %ebp\n" -//zz " ret\n" // jump to dst -//zz " ud2\n" // should never get here -//zz ); -//zz #elif defined(VGA_amd64) -//zz // %rdi == stack -//zz // %rsi == dst -//zz asm( -//zz ".global vgPlain_jump_and_switch_stacks\n" -//zz "vgPlain_jump_and_switch_stacks:\n" -//zz " movq %rdi, %rsp\n" // set stack -//zz " pushq %rsi\n" // dst to stack -//zz " movq $0, %rax\n" // zero all GP regs -//zz " movq $0, %rbx\n" -//zz " movq $0, %rcx\n" -//zz " movq $0, %rdx\n" -//zz " movq $0, %rsi\n" -//zz " movq $0, %rdi\n" -//zz " movq $0, %rbp\n" -//zz " movq $0, %r8\n" -//zz " movq $0, %r9\n" -//zz " movq $0, %r10\n" -//zz " movq $0, %r11\n" -//zz " movq $0, %r12\n" -//zz " movq $0, %r13\n" -//zz " movq $0, %r14\n" -//zz " movq $0, %r15\n" -//zz " ret\n" // jump to dst -//zz " ud2\n" // should never get here -//zz ); -//zz -//zz #elif defined(VGA_ppc32) -//zz /* Jump to 'dst', but first set the stack pointer to 'stack'. Also, -//zz clear all the integer registers before entering 'dst'. It's -//zz important that the stack pointer is set to exactly 'stack' and not -//zz (eg) stack - apparently_harmless_looking_small_offset. Basically -//zz because the code at 'dst' might be wanting to scan the area above -//zz 'stack' (viz, the auxv array), and putting spurious words on the -//zz stack confuses it. -//zz */ -//zz // %r3 == stack -//zz // %r4 == dst -//zz asm( -//zz ".global vgPlain_jump_and_switch_stacks\n" -//zz "vgPlain_jump_and_switch_stacks:\n" -//zz " mtctr %r4\n\t" // dst to %ctr -//zz " mr %r1,%r3\n\t" // stack to %sp -//zz " li 0,0\n\t" // zero all GP regs -//zz " li 3,0\n\t" -//zz " li 4,0\n\t" -//zz " li 5,0\n\t" -//zz " li 6,0\n\t" -//zz " li 7,0\n\t" -//zz " li 8,0\n\t" -//zz " li 9,0\n\t" -//zz " li 10,0\n\t" -//zz " li 11,0\n\t" -//zz " li 12,0\n\t" -//zz " li 13,0\n\t" // CAB: This right? r13 = small data area ptr -//zz " li 14,0\n\t" -//zz " li 15,0\n\t" -//zz " li 16,0\n\t" -//zz " li 17,0\n\t" -//zz " li 18,0\n\t" -//zz " li 19,0\n\t" -//zz " li 20,0\n\t" -//zz " li 21,0\n\t" -//zz " li 22,0\n\t" -//zz " li 23,0\n\t" -//zz " li 24,0\n\t" -//zz " li 25,0\n\t" -//zz " li 26,0\n\t" -//zz " li 27,0\n\t" -//zz " li 28,0\n\t" -//zz " li 29,0\n\t" -//zz " li 30,0\n\t" -//zz " li 31,0\n\t" -//zz " mtxer 0\n\t" -//zz " mtcr 0\n\t" -//zz " mtlr %r0\n\t" -//zz " bctr\n\t" // jump to dst -//zz " trap\n" // should never get here -//zz ); -//zz -//zz #else -//zz # error Unknown architecture -//zz #endif - /*------------------------------------------------------------*/ /*--- Finding auxv on the stack ---*/ /*------------------------------------------------------------*/ @@ -267,11 +108,11 @@ struct ume_auxv *VG_(find_auxv)(UWord* sp) /*------------------------------------------------------------*/ static -struct elfinfo *readelf(int fd, const char *filename) +struct elfinfo *readelf(Int fd, const char *filename) { SysRes sres; struct elfinfo *e = VG_(malloc)(sizeof(*e)); - int phsz; + Int phsz; vg_assert(e); e->fd = fd; @@ -424,11 +265,7 @@ ESZ(Addr) mapelf(struct elfinfo *e, ESZ(Addr) base) return elfbrk; } -// Forward declaration. -/* returns: 0 = success, non-0 is failure */ -static int do_exec_inner(const char *exe, struct exeinfo *info); - -static int match_ELF(const char *hdr, int len) +static Bool match_ELF(const char *hdr, Int len) { ESZ(Ehdr) *e = (ESZ(Ehdr) *)hdr; return (len > sizeof(*e)) && VG_(memcmp)(&e->e_ident[0], ELFMAG, SELFMAG) == 0; @@ -478,8 +315,7 @@ static int match_ELF(const char *hdr, int len) - The entry point in INFO is set to the interpreter's entry point, and we're done. */ -static int load_ELF(char *hdr, int len, int fd, const char *name, - /*MOD*/struct exeinfo *info) +static Int load_ELF(Int fd, const char *name, /*MOD*/struct exeinfo *info) { SysRes sres; struct elfinfo *e; @@ -489,7 +325,7 @@ static int load_ELF(char *hdr, int len, int fd, const char *name, ESZ(Addr) interp_addr = 0; /* interpreter (ld.so) address */ ESZ(Word) interp_size = 0; /* interpreter size */ ESZ(Word) interp_align = VKI_PAGE_SIZE; - int i; + Int i; void *entry; ESZ(Addr) ebase = 0; @@ -529,9 +365,9 @@ static int load_ELF(char *hdr, int len, int fd, const char *name, case PT_INTERP: { char *buf = VG_(malloc)(ph->p_filesz+1); - int j; - int intfd; - int baseaddr_set; + Int j; + Int intfd; + Int baseaddr_set; vg_assert(buf); VG_(pread)(fd, buf, ph->p_filesz, ph->p_offset); @@ -646,30 +482,63 @@ static int load_ELF(char *hdr, int len, int fd, const char *name, } -static int match_script(const char *hdr, Int len) +static Bool match_script(char *hdr, Int len) { - return (len > 2) && VG_(memcmp)(hdr, "#!", 2) == 0; + Char* end = hdr + len; + Char* interp = hdr + 2; + + // len < 4: need '#', '!', plus at least a '/' and one more char + if (len < 4) return False; + if (0 != VG_(memcmp)(hdr, "#!", 2)) return False; + + // Find interpreter name, make sure it's an absolute path (starts with + // '/') and has at least one more char. + while (interp < end && VG_(isspace)(*interp)) interp++; + if (*interp != '/') return False; // absolute path only for interpreter + if (interp == end) return False; // nothing after the '/' + + // Here we should get the full interpreter name and check it with + // check_executable(). See the "EXEC FAILED" failure when running shell + // for an example. + + return True; // looks like a #! script } +// Forward declaration. +static Int do_exec_inner(const char *exe, struct exeinfo *info); + /* returns: 0 = success, non-0 is failure */ -static int load_script(char *hdr, int len, int fd, const char *name, - struct exeinfo *info) +static Int load_script(Int fd, const char *name, struct exeinfo *info) { - char *interp; - char *const end = hdr+len; - char *cp; - char *arg = NULL; - int eol; + Char hdr[VKI_PAGE_SIZE]; + Int len = VKI_PAGE_SIZE; + Int eol; + Char* interp; + Char* end; + Char* cp; + Char* arg = NULL; + SysRes res; + // Read the first part of the file. + res = VG_(pread)(fd, hdr, len, 0); + if (res.isError) { + VG_(close)(fd); + return VKI_EACCES; + } else { + len = res.val; + } + + vg_assert('#' == hdr[0] && '!' == hdr[1]); + + end = hdr + len; interp = hdr + 2; - while(interp < end && (*interp == ' ' || *interp == '\t')) + while (interp < end && VG_(isspace)(*interp)) interp++; - if (*interp != '/') - return VKI_ENOEXEC; /* absolute path only for interpreter */ + vg_assert(*interp == '/'); /* absolute path only for interpreter */ /* skip over interpreter name */ - for(cp = interp; cp < end && *cp != ' ' && *cp != '\t' && *cp != '\n'; cp++) + for (cp = interp; cp < end && !VG_(isspace)(*cp); cp++) ; eol = (*cp == '\n'); @@ -678,7 +547,7 @@ static int load_script(char *hdr, int len, int fd, const char *name, if (!eol && cp < end) { /* skip space before arg */ - while (cp < end && (*cp == '\t' || *cp == ' ')) + while (cp < end && VG_(isspace)(*cp)) cp++; /* arg is from here to eol */ @@ -705,130 +574,215 @@ static int load_script(char *hdr, int len, int fd, const char *name, return do_exec_inner(interp, info); } -/* - Emulate the normal Unix permissions checking algorithm. - If owner matches, then use the owner permissions, else - if group matches, then use the group permissions, else - use other permissions. +typedef enum { + VG_EXE_FORMAT_ELF = 1, + VG_EXE_FORMAT_SCRIPT = 2, +} ExeFormat; - Note that we can't deal with SUID/SGID, so we refuse to run them - (otherwise the executable may misbehave if it doesn't have the - permissions it thinks it does). -*/ -/* returns: 0 = success, non-0 is failure */ -static int check_perms(int fd) +// Check the file looks executable. +SysRes VG_(pre_exec_check)(const Char* exe_name, Int* out_fd) { - struct vki_stat st; + Int fd, ret; + SysRes res; + Char buf[VKI_PAGE_SIZE]; + SizeT bufsz = VKI_PAGE_SIZE, fsz; - if (VG_(fstat)(fd, &st) == -1) - return VKI_EACCES; + // Check it's readable + res = VG_(open)(exe_name, VKI_O_RDONLY, 0); + if (res.isError) { + return res; + } + fd = res.val; - if (st.st_mode & (VKI_S_ISUID | VKI_S_ISGID)) { - //VG_(printf)("Can't execute suid/sgid executable %s\n", exe); - return VKI_EACCES; + // Check we have execute permissions + ret = VG_(check_executable)((HChar*)exe_name); + if (0 != ret) { + VG_(close)(fd); + return VG_(mk_SysRes_Error)(ret); + } + + fsz = VG_(fsize)(fd); + if (fsz < bufsz) + bufsz = fsz; + + res = VG_(pread)(fd, buf, bufsz, 0); + if (res.isError || res.val != bufsz) { + VG_(close)(fd); + return VG_(mk_SysRes_Error)(VKI_EACCES); } + bufsz = res.val; - if (VG_(geteuid)() == st.st_uid) { - if (!(st.st_mode & VKI_S_IXUSR)) - return VKI_EACCES; + if (match_ELF(buf, bufsz)) { + res = VG_(mk_SysRes_Success)(VG_EXE_FORMAT_ELF); + } else if (match_script(buf, bufsz)) { + res = VG_(mk_SysRes_Success)(VG_EXE_FORMAT_SCRIPT); } else { - int grpmatch = 0; - - if (VG_(getegid)() == st.st_gid) - grpmatch = 1; - else { - UInt groups[32]; - Int ngrp = VG_(getgroups)(32, groups); - Int i; - /* ngrp will be -1 if VG_(getgroups) failed. */ - for (i = 0; i < ngrp; i++) { - if (groups[i] == st.st_gid) { - grpmatch = 1; - break; - } - } - } + res = VG_(mk_SysRes_Error)(VKI_ENOEXEC); + } - if (grpmatch) { - if (!(st.st_mode & VKI_S_IXGRP)) - return VKI_EACCES; - } else if (!(st.st_mode & VKI_S_IXOTH)) - return VKI_EACCES; + // Write the 'out_fd' param if necessary, or close the file. + if (!res.isError && out_fd) { + *out_fd = fd; + } else { + VG_(close)(fd); } - return 0; + return res; } -/* returns: 0 = success, non-0 is failure */ -static int do_exec_inner(const char *exe, struct exeinfo *info) +// returns: 0 = success, non-0 is failure +// +// We can execute only ELF binaries or scripts that begin with "#!". (Not, +// for example, scripts that don't begin with "#!"; see the VG_(do_exec)() +// invocation from m_main.c for how that's handled.) +static Int do_exec_inner(const char *exe, struct exeinfo *info) { - SysRes sres; - int fd; - int err; - char buf[VKI_PAGE_SIZE]; - int bufsz; - int i; - int ret; - static const struct { - int (*match)(const char *hdr, int len); - int (*load) ( char *hdr, int len, int fd2, const char *name, - struct exeinfo *); - } formats[] = { - { match_ELF, load_ELF }, - { match_script, load_script }, - }; - - sres = VG_(open)(exe, VKI_O_RDONLY, 0); - if (sres.isError) { - if (0) - VG_(printf)("Can't open executable %s: %s\n", - exe, VG_(strerror)(sres.val)); - return sres.val; + SysRes res; + Int fd; + Int ret; + + res = VG_(pre_exec_check)(exe, &fd); + if (res.isError) + return res.val; + + switch (res.val) { + case VG_EXE_FORMAT_ELF: ret = load_ELF (fd, exe, info); break; + case VG_EXE_FORMAT_SCRIPT: ret = load_script(fd, exe, info); break; + default: + vg_assert2(0, "unrecognised VG_EXE_FORMAT value\n"); } - fd = sres.val; - err = check_perms(fd); - if (err != 0) { - VG_(close)(fd); - return err; - } + VG_(close)(fd); + + return ret; +} - bufsz = VG_(fsize)(fd); - if (bufsz > sizeof(buf)) - bufsz = sizeof(buf); - sres = VG_(pread)(fd, buf, bufsz, 0); - if (sres.isError || sres.val != bufsz) { - VG_(printf)("Can't read executable header: %s\n", - VG_(strerror)(sres.val)); - VG_(close)(fd); - return sres.val; +static Bool is_hash_bang_file(Char* f) +{ + SysRes res = VG_(open)(f, VKI_O_RDONLY, 0); + if (!res.isError) { + Char buf[3] = {0,0,0}; + Int fd = res.val; + Int n = VG_(read)(fd, buf, 2); + if (n == 2 && VG_STREQ("#!", buf)) + return True; } - bufsz = sres.val; + return False; +} - ret = VKI_ENOEXEC; - for(i = 0; i < sizeof(formats)/sizeof(*formats); i++) { - if ((formats[i].match)(buf, bufsz)) { - ret = (formats[i].load)(buf, bufsz, fd, exe, info); - break; +// Look at the first 80 chars, and if any are greater than 127, it's binary. +// This is crude, but should be good enough. Note that it fails on a +// zero-length file, as we want. +static Bool is_binary_file(Char* f) +{ + SysRes res = VG_(open)(f, VKI_O_RDONLY, 0); + if (!res.isError) { + UChar buf[80]; + Int fd = res.val; + Int n = VG_(read)(fd, buf, 80); + Int i; + for (i = 0; i < n; i++) { + if (buf[i] > 127) + return True; // binary char found } + return False; + } else { + // Something went wrong. This will only happen if we earlier + // succeeded in opening the file but fail here (eg. the file was + // deleted between then and now). + VG_(printf)("valgrind: %s: unknown error\n", f); + VG_(exit)(126); // 126 == NOEXEC } +} - VG_(close)(fd); +// If the do_exec fails we try to emulate what the shell does (I used +// bash as a guide). It's worth noting that the shell can execute some +// things that VG_(do_exec)() (which subsitutes for the kernel's exec()) +// will refuse to (eg. scripts lacking a "#!" prefix). +static Int do_exec_shell_followup(Int ret, Char* exe_name, + struct exeinfo* info) +{ + Char* default_interp_name = "/bin/sh"; + SysRes res; + struct vki_stat st; + + if (VKI_ENOEXEC == ret) { + // It was an executable file, but in an unacceptable format. Probably + // is a shell script lacking the "#!" prefix; try to execute it so. + + // Is it a binary file? + if (is_binary_file(exe_name)) { + VG_(printf)("valgrind: %s: cannot execute binary file\n", exe_name); + VG_(exit)(126); // 126 == NOEXEC + } + + // Looks like a script. Run it with /bin/sh. This includes + // zero-length files. + + info->interp_name = VG_(strdup)(default_interp_name); + info->interp_args = NULL; + if (info->argv && info->argv[0] != NULL) + info->argv[0] = (char *)exe_name; + ret = do_exec_inner(info->interp_name, info); + + if (0 != ret) { + // Something went wrong with executing the default interpreter + VG_(printf)("valgrind: %s: bad interpreter (%s): %s\n", + exe_name, info->interp_name, VG_(strerror)(ret)); + VG_(exit)(126); // 126 == NOEXEC + } + + } else if (0 != ret) { + // Something else went wrong. Try to make the error more specific, + // and then print a message and abort. + + // Was it a directory? + res = VG_(stat)(exe_name, &st); + if (!res.isError && VKI_S_ISDIR(st.st_mode)) { + VG_(printf)("valgrind: %s: is a directory\n", exe_name); + + // Was it not executable? + } else if (0 != VG_(check_executable)(exe_name)) { + VG_(printf)("valgrind: %s: %s\n", exe_name, VG_(strerror)(ret)); + + // Did it start with "#!"? If so, it must have been a bad interpreter. + } else if (is_hash_bang_file(exe_name)) { + VG_(printf)("valgrind: %s: bad interpreter: %s\n", + exe_name, VG_(strerror)(ret)); + + // Otherwise it was something else. + } else { + VG_(printf)("valgrind: %s\n", exe_name, VG_(strerror)(ret)); + } + // 126 means NOEXEC; I think this is Posix, and that in some cases we + // should be returning 127, meaning NOTFOUND. Oh well. + VG_(exit)(126); + } return ret; } + +// This emulates the kernel's exec(). If it fails, it then emulates the +// shell's handling of the situation. // See ume.h for an indication of which entries of 'info' are inputs, which // are outputs, and which are both. /* returns: 0 = success, non-0 is failure */ -int VG_(do_exec)(const char *exe, struct exeinfo *info) +Int VG_(do_exec)(const char *exe_name, struct exeinfo *info) { + Int ret; + info->interp_name = NULL; info->interp_args = NULL; - return do_exec_inner(exe, info); + ret = do_exec_inner(exe_name, info); + + if (0 != ret) { + ret = do_exec_shell_followup(ret, (Char*)exe_name, info); + } + return ret; } /*--------------------------------------------------------------------*/ diff --git a/coregrind/pub_core_libcfile.h b/coregrind/pub_core_libcfile.h index 48bd581831..ef0791e32d 100644 --- a/coregrind/pub_core_libcfile.h +++ b/coregrind/pub_core_libcfile.h @@ -48,6 +48,9 @@ extern Bool VG_(resolve_filename) ( Int fd, HChar* buf, Int n_buf ); /* Return the size of a file */ extern Int VG_(fsize) ( Int fd ); +/* Is the file a directory? */ +extern Bool VG_(is_dir) ( HChar* f ); + /* Default destination port to be used in logging over a network, if none specified. */ #define VG_CLO_DEFAULT_LOGPORT 1500 @@ -61,6 +64,9 @@ extern Int VG_(getsockopt) ( Int sd, Int level, Int optname, void *optval, extern Int VG_(access) ( HChar* path, Bool irusr, Bool iwusr, Bool ixusr ); +/* Is the file executable? Returns: 0 = success, non-0 is failure */ +extern Int VG_(check_executable)(HChar* f); + extern SysRes VG_(pread) ( Int fd, void* buf, Int count, Int offset ); /* Create and open (-rw------) a tmp file name incorporating said arg. diff --git a/coregrind/pub_core_ume.h b/coregrind/pub_core_ume.h index fbf414f34e..1054e9b08e 100644 --- a/coregrind/pub_core_ume.h +++ b/coregrind/pub_core_ume.h @@ -93,11 +93,17 @@ struct exeinfo char* interp_args; // OUT: the args for the interpreter }; +// Do a number of appropriate checks to see if the file looks executable by +// the kernel: ie. it's a file, it's readable and executable, and it's in +// either ELF or "#!" format. On success, 'out_fd' gets the fd of the file +// if it's non-NULL. Otherwise the fd is closed. +extern SysRes VG_(pre_exec_check)(const Char* exe_name, Int* out_fd); + // Does everything short of actually running 'exe': finds the file, // checks execute permissions, sets up interpreter if program is a script, // reads headers, maps file into memory, and returns important info about // the program. -extern int VG_(do_exec)(const char *exe, struct exeinfo *info); +extern Int VG_(do_exec)(const char *exe, struct exeinfo *info); /*------------------------------------------------------------*/ /*--- Finding and dealing with auxv ---*/ diff --git a/none/tests/Makefile.am b/none/tests/Makefile.am index 480220b2ef..b7b287b034 100644 --- a/none/tests/Makefile.am +++ b/none/tests/Makefile.am @@ -84,6 +84,17 @@ EXTRA_DIST = $(noinst_SCRIPTS) \ selfrun.stderr.exp selfrun.stdout.exp selfrun.vgtest \ sem.stderr.exp sem.stdout.exp sem.vgtest \ semlimit.stderr.exp semlimit.stdout.exp semlimit.vgtest \ + shell shell.vgtest shell.stderr.exp shell.stdout.exp \ + shell_badinterp shell_badinterp.vgtest shell_badinterp.stderr.exp \ + shell_binaryfile shell_binaryfile.vgtest shell_binaryfile.stderr.exp \ + shell_dir.vgtest shell_dir.stderr.exp \ + shell_nonexec shell_nonexec.vgtest shell_nonexec.stderr.exp \ + shell_nonexec shell_nonexec.vgtest shell_nonexec.stderr.exp \ + shell_nosuchfile.vgtest shell_nosuchfile.stderr.exp \ + shell_valid1 shell_valid1.vgtest shell_valid1.stderr.exp \ + shell_valid2 shell_valid2.vgtest shell_valid2.stderr.exp \ + shell_valid3 shell_valid3.vgtest shell_valid3.stderr.exp \ + shell_zerolength shell_zerolength.vgtest shell_zerolength.stderr.exp \ susphello.stdout.exp susphello.stderr.exp susphello.vgtest \ sha1_test.stderr.exp sha1_test.vgtest \ shortpush.stderr.exp shortpush.vgtest \ diff --git a/none/tests/cmdline5.stderr.exp b/none/tests/cmdline5.stderr.exp index 6a96f59df4..3771e61151 100644 --- a/none/tests/cmdline5.stderr.exp +++ b/none/tests/cmdline5.stderr.exp @@ -1 +1 @@ -valgrind: do_exec(./no-such-program-my-friend) failed: No such file or directory +valgrind: ./no-such-program-my-friend: No such file or directory diff --git a/none/tests/cmdline6.stderr.exp b/none/tests/cmdline6.stderr.exp index e4c4c44131..ecd85346fd 100644 --- a/none/tests/cmdline6.stderr.exp +++ b/none/tests/cmdline6.stderr.exp @@ -1 +1 @@ -valgrind: do_exec(./cmdline6.vgtest) failed: Permission denied +valgrind: ./cmdline6.vgtest: Permission denied diff --git a/none/tests/shell b/none/tests/shell new file mode 100755 index 0000000000..e954cd0e26 --- /dev/null +++ b/none/tests/shell @@ -0,0 +1,41 @@ +#! /bin/sh +# +# Testing various shell script invocations. + +#---------------------------------------------------------------------------- +# Shell scripts that should fail +#---------------------------------------------------------------------------- + +echo "Execute a directory" +x86/ + +echo "Execute a directory (2)" +x86 + +echo "Execute a non-executable file" +shell.vgtest + +echo "Execute a script with a bad interpreter name" +shell_badinterp + +echo "Execute a binary file" +shell_binaryfile + +echo "Execute a non-existent file" +shell_nosuchfile + +#---------------------------------------------------------------------------- +# Shell scripts that should pass +#---------------------------------------------------------------------------- +echo "Execute a valid script with a #! line" +shell_valid1 + +echo "Execute a valid script without a #! line" +shell_valid2 + +echo "Execute a valid script with #! but no interpname" +shell_valid3 + +echo "Execute a zero-length file" +shell_zerolength + diff --git a/none/tests/shell.stderr.exp b/none/tests/shell.stderr.exp new file mode 100644 index 0000000000..8d82aed470 --- /dev/null +++ b/none/tests/shell.stderr.exp @@ -0,0 +1,18 @@ + +./shell: x86/: is a directory + +./shell: x86: command not found + +./shell: ./shell.vgtest: Permission denied + +execve(0x........(./shell_badinterp), 0x........, 0x........) failed, errno 2 +EXEC FAILED: I can't recover from execve() failing, so I'm dying. +Add more stringent tests in PRE(sys_execve), or work out how to recover. +./shell: ./shell_binaryfile: cannot execute binary file + +./shell: shell_nosuchfile: command not found + + + + + diff --git a/none/tests/shell.stdout.exp b/none/tests/shell.stdout.exp new file mode 100644 index 0000000000..50da06706a --- /dev/null +++ b/none/tests/shell.stdout.exp @@ -0,0 +1,10 @@ +Execute a directory +Execute a directory (2) +Execute a non-executable file +Execute a script with a bad interpreter name +Execute a binary file +Execute a non-existent file +Execute a valid script with a #! line +Execute a valid script without a #! line +Execute a valid script with #! but no interpname +Execute a zero-length file diff --git a/none/tests/shell.vgtest b/none/tests/shell.vgtest new file mode 100644 index 0000000000..be86e961c6 --- /dev/null +++ b/none/tests/shell.vgtest @@ -0,0 +1 @@ +prog: shell diff --git a/none/tests/shell_badinterp b/none/tests/shell_badinterp new file mode 100755 index 0000000000..8d795898dd --- /dev/null +++ b/none/tests/shell_badinterp @@ -0,0 +1,3 @@ +#! /this/is/a/bogus/interpreter/name + +true diff --git a/none/tests/shell_badinterp.stderr.exp b/none/tests/shell_badinterp.stderr.exp new file mode 100644 index 0000000000..5a568b7f9d --- /dev/null +++ b/none/tests/shell_badinterp.stderr.exp @@ -0,0 +1 @@ +valgrind: ./shell_badinterp: bad interpreter: No such file or directory diff --git a/none/tests/shell_badinterp.vgtest b/none/tests/shell_badinterp.vgtest new file mode 100644 index 0000000000..ec04ecfd4c --- /dev/null +++ b/none/tests/shell_badinterp.vgtest @@ -0,0 +1 @@ +prog: shell_badinterp diff --git a/none/tests/shell_binaryfile b/none/tests/shell_binaryfile new file mode 100755 index 0000000000..41bcd1019f Binary files /dev/null and b/none/tests/shell_binaryfile differ diff --git a/none/tests/shell_binaryfile.stderr.exp b/none/tests/shell_binaryfile.stderr.exp new file mode 100644 index 0000000000..d22300e29d --- /dev/null +++ b/none/tests/shell_binaryfile.stderr.exp @@ -0,0 +1 @@ +valgrind: ./shell_binaryfile: cannot execute binary file diff --git a/none/tests/shell_binaryfile.vgtest b/none/tests/shell_binaryfile.vgtest new file mode 100644 index 0000000000..113d172594 --- /dev/null +++ b/none/tests/shell_binaryfile.vgtest @@ -0,0 +1 @@ +prog: shell_binaryfile diff --git a/none/tests/shell_dir.stderr.exp b/none/tests/shell_dir.stderr.exp new file mode 100644 index 0000000000..6cd6bbe1bc --- /dev/null +++ b/none/tests/shell_dir.stderr.exp @@ -0,0 +1 @@ +valgrind: ./x86/: is a directory diff --git a/none/tests/shell_dir.vgtest b/none/tests/shell_dir.vgtest new file mode 100644 index 0000000000..366f5c8802 --- /dev/null +++ b/none/tests/shell_dir.vgtest @@ -0,0 +1 @@ +prog: x86/ diff --git a/none/tests/shell_nonexec.stderr.exp b/none/tests/shell_nonexec.stderr.exp new file mode 100644 index 0000000000..58a91c0668 --- /dev/null +++ b/none/tests/shell_nonexec.stderr.exp @@ -0,0 +1 @@ +valgrind: ./shell.vgtest: Permission denied diff --git a/none/tests/shell_nonexec.vgtest b/none/tests/shell_nonexec.vgtest new file mode 100644 index 0000000000..8bd76557f4 --- /dev/null +++ b/none/tests/shell_nonexec.vgtest @@ -0,0 +1 @@ +prog: shell.vgtest diff --git a/none/tests/shell_nosuchfile.stderr.exp b/none/tests/shell_nosuchfile.stderr.exp new file mode 100644 index 0000000000..b87efba283 --- /dev/null +++ b/none/tests/shell_nosuchfile.stderr.exp @@ -0,0 +1 @@ +valgrind: ./shell_nosuchfile: No such file or directory diff --git a/none/tests/shell_nosuchfile.vgtest b/none/tests/shell_nosuchfile.vgtest new file mode 100644 index 0000000000..13d3e65c10 --- /dev/null +++ b/none/tests/shell_nosuchfile.vgtest @@ -0,0 +1 @@ +prog: shell_nosuchfile diff --git a/none/tests/shell_valid1 b/none/tests/shell_valid1 new file mode 100755 index 0000000000..153962c74e --- /dev/null +++ b/none/tests/shell_valid1 @@ -0,0 +1,5 @@ +#! /bin/sh +# +# This is a valid script with a #! line + +true diff --git a/none/tests/shell_valid1.stderr.exp b/none/tests/shell_valid1.stderr.exp new file mode 100644 index 0000000000..e69de29bb2 diff --git a/none/tests/shell_valid1.vgtest b/none/tests/shell_valid1.vgtest new file mode 100644 index 0000000000..2c4de94e4e --- /dev/null +++ b/none/tests/shell_valid1.vgtest @@ -0,0 +1,2 @@ +prog: shell_valid1 +vgopts: -q diff --git a/none/tests/shell_valid2 b/none/tests/shell_valid2 new file mode 100755 index 0000000000..f00fc86ce5 --- /dev/null +++ b/none/tests/shell_valid2 @@ -0,0 +1,6 @@ +# +# +# This is a valid script without a #! line + +true + diff --git a/none/tests/shell_valid2.stderr.exp b/none/tests/shell_valid2.stderr.exp new file mode 100644 index 0000000000..e69de29bb2 diff --git a/none/tests/shell_valid2.vgtest b/none/tests/shell_valid2.vgtest new file mode 100644 index 0000000000..2c4de94e4e --- /dev/null +++ b/none/tests/shell_valid2.vgtest @@ -0,0 +1,2 @@ +prog: shell_valid1 +vgopts: -q diff --git a/none/tests/shell_valid3 b/none/tests/shell_valid3 new file mode 100755 index 0000000000..7c3cb3e3e3 --- /dev/null +++ b/none/tests/shell_valid3 @@ -0,0 +1,5 @@ +#! +# +# The interpreter name is missing in this file. + +true diff --git a/none/tests/shell_valid3.stderr.exp b/none/tests/shell_valid3.stderr.exp new file mode 100644 index 0000000000..e69de29bb2 diff --git a/none/tests/shell_valid3.vgtest b/none/tests/shell_valid3.vgtest new file mode 100644 index 0000000000..2c4de94e4e --- /dev/null +++ b/none/tests/shell_valid3.vgtest @@ -0,0 +1,2 @@ +prog: shell_valid1 +vgopts: -q diff --git a/none/tests/shell_zerolength b/none/tests/shell_zerolength new file mode 100755 index 0000000000..e69de29bb2 diff --git a/none/tests/shell_zerolength.stderr.exp b/none/tests/shell_zerolength.stderr.exp new file mode 100644 index 0000000000..e69de29bb2 diff --git a/none/tests/shell_zerolength.vgtest b/none/tests/shell_zerolength.vgtest new file mode 100644 index 0000000000..04180c4f65 --- /dev/null +++ b/none/tests/shell_zerolength.vgtest @@ -0,0 +1,2 @@ +prog: shell_zerolength +vgopts: -q