From 56ac45c10b294dc800c4cce9e53df7b2ae2495d2 Mon Sep 17 00:00:00 2001 From: David van Moolenbroek Date: Mon, 31 Aug 2015 12:12:28 +0000 Subject: [PATCH] VFS: check X bit, not R bit, opening executables For dynamically linked executables, the interpreter is passed a file descriptor of the binary being executed. To this end, VFS opens the target executable, but opening the file fails if it is not readable, even when it is executable. With this patch, when opening the executable, it verifies the X bit rather than the R bit on the file, thus allowing the execution of dynamically linked binaries that are executable but not readable. Add test86 to verify correctness. Change-Id: If3514add6a33b33d52c05a0a627d757bff118d77 --- distrib/sets/lists/minix/mi | 1 + minix/servers/vfs/exec.c | 3 ++- minix/servers/vfs/misc.c | 2 +- minix/servers/vfs/open.c | 13 ++++++---- minix/servers/vfs/proto.h | 2 +- minix/tests/Makefile | 2 +- minix/tests/run | 2 +- minix/tests/test86.c | 50 +++++++++++++++++++++++++++++++++++++ 8 files changed, 65 insertions(+), 10 deletions(-) create mode 100644 minix/tests/test86.c diff --git a/distrib/sets/lists/minix/mi b/distrib/sets/lists/minix/mi index ff0024099..ea23bc671 100644 --- a/distrib/sets/lists/minix/mi +++ b/distrib/sets/lists/minix/mi @@ -6307,6 +6307,7 @@ ./usr/tests/minix-posix/test83 minix-sys ./usr/tests/minix-posix/test84 minix-sys ./usr/tests/minix-posix/test85 minix-sys +./usr/tests/minix-posix/test86 minix-sys ./usr/tests/minix-posix/test9 minix-sys ./usr/tests/minix-posix/testinterp minix-sys ./usr/tests/minix-posix/testisofs minix-sys diff --git a/minix/servers/vfs/exec.c b/minix/servers/vfs/exec.c index 4a371a4fe..1eef3821d 100644 --- a/minix/servers/vfs/exec.c +++ b/minix/servers/vfs/exec.c @@ -285,7 +285,8 @@ int pm_exec(vir_bytes path, size_t path_len, vir_bytes frame, size_t frame_len, /* The interpreter (loader) needs an fd to the main program, * which is currently in finalexec */ - if((r = execi.elf_main_fd = common_open(finalexec, O_RDONLY, 0)) < 0) { + if ((r = execi.elf_main_fd = + common_open(finalexec, O_RDONLY, 0, TRUE /*for_exec*/)) < 0) { printf("VFS: exec: dynamic: open main exec failed %s (%d)\n", fullpath, r); FAILCHECK(r); diff --git a/minix/servers/vfs/misc.c b/minix/servers/vfs/misc.c index 8a88fa863..6e740f8a2 100644 --- a/minix/servers/vfs/misc.c +++ b/minix/servers/vfs/misc.c @@ -895,7 +895,7 @@ int pm_dumpcore(int csig, vir_bytes exe_name) /* open core file */ snprintf(core_path, PATH_MAX, "%s.%d", CORE_NAME, fp->fp_pid); r = core_fd = common_open(core_path, O_WRONLY | O_CREAT | O_TRUNC, - CORE_MODE); + CORE_MODE, FALSE /*for_exec*/); if (r < 0) goto core_exit; /* get process name */ diff --git a/minix/servers/vfs/open.c b/minix/servers/vfs/open.c index 1fbdbaf87..0ca684854 100644 --- a/minix/servers/vfs/open.c +++ b/minix/servers/vfs/open.c @@ -49,7 +49,7 @@ int do_open(void) if (copy_path(fullpath, sizeof(fullpath)) != OK) return(err_code); - return common_open(fullpath, open_flags, 0 /*omode*/); + return common_open(fullpath, open_flags, 0 /*omode*/, FALSE /*for_exec*/); } /*===========================================================================* @@ -74,13 +74,13 @@ int do_creat(void) if (fetch_name(vname, vname_length, fullpath) != OK) return(err_code); - return common_open(fullpath, open_flags, create_mode); + return common_open(fullpath, open_flags, create_mode, FALSE /*for_exec*/); } /*===========================================================================* * common_open * *===========================================================================*/ -int common_open(char path[PATH_MAX], int oflags, mode_t omode) +int common_open(char path[PATH_MAX], int oflags, mode_t omode, int for_exec) { /* Common code from do_creat and do_open. */ int b, r, exist = TRUE; @@ -139,8 +139,11 @@ int common_open(char path[PATH_MAX], int oflags, mode_t omode) /* Only do the normal open code if we didn't just create the file. */ if (exist) { - /* Check protections. */ - if ((r = forbidden(fp, vp, bits)) == OK) { + /* Check permissions based on the given open flags, except when we are + * opening an executable for the purpose of passing a file descriptor + * to its interpreter for execution, in which case we check the X bit. + */ + if ((r = forbidden(fp, vp, for_exec ? X_BIT : bits)) == OK) { /* Opening reg. files, directories, and special files differ */ switch (vp->v_mode & S_IFMT) { case S_IFREG: diff --git a/minix/servers/vfs/proto.h b/minix/servers/vfs/proto.h index ba37fb2ec..aeddf4646 100644 --- a/minix/servers/vfs/proto.h +++ b/minix/servers/vfs/proto.h @@ -138,7 +138,7 @@ void unmount_all(int force); /* open.c */ int do_close(void); int close_fd(struct fproc *rfp, int fd_nr); -int common_open(char path[PATH_MAX], int oflags, mode_t omode); +int common_open(char path[PATH_MAX], int oflags, mode_t omode, int for_exec); int do_creat(void); int do_lseek(void); int do_mknod(void); diff --git a/minix/tests/Makefile b/minix/tests/Makefile index db91e52d0..2543b0b6f 100644 --- a/minix/tests/Makefile +++ b/minix/tests/Makefile @@ -59,7 +59,7 @@ MINIX_TESTS= \ 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 \ 41 42 43 44 45 46 48 49 50 52 53 54 55 56 58 59 60 \ 61 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 \ -81 82 83 84 85 +81 82 83 84 85 86 FILES += t84_h_nonexec.sh diff --git a/minix/tests/run b/minix/tests/run index d442d2591..8beeaa4de 100755 --- a/minix/tests/run +++ b/minix/tests/run @@ -30,7 +30,7 @@ alltests="1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 \ 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 \ 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 \ 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 \ - 81 82 83 84 85 sh1 sh2 interp mfs isofs vnd" + 81 82 83 84 85 86 sh1 sh2 interp mfs isofs vnd" tests_no=`expr 0` # If root, make sure the setuid tests have the correct permissions diff --git a/minix/tests/test86.c b/minix/tests/test86.c new file mode 100644 index 000000000..97ee3bf1f --- /dev/null +++ b/minix/tests/test86.c @@ -0,0 +1,50 @@ +#include +#include +#include +#include +#include + +#include "common.h" + +/* + * Test for dynamic executables with no read permissions. This test relies on + * being linked dynamically. + */ +int +main(int argc, char ** argv) +{ + char *executable, cp_cmd[PATH_MAX + 9]; + int status; + + if (strcmp(argv[0], "DO CHECK") == 0) + exit(EXIT_SUCCESS); + + start(86); + + /* Make a copy of this binary which is executable-only. */ + executable = argv[0]; + + snprintf(cp_cmd, sizeof(cp_cmd), "cp ../%s .", executable); + status = system(cp_cmd); + if (status < 0 || !WIFEXITED(status) || + WEXITSTATUS(status) != EXIT_SUCCESS) e(0); + + if (chmod(executable, S_IXUSR) != 0) e(0); + + /* Invoke the changed binary in a child process. */ + switch (fork()) { + case -1: + e(0); + case 0: + execl(executable, "DO CHECK", NULL); + + exit(EXIT_FAILURE); + default: + if (wait(&status) <= 0) e(0); + if (!WIFEXITED(status) || WEXITSTATUS(status) != EXIT_SUCCESS) + e(0); + } + + quit(); + /* NOTREACHED */ +} -- 2.44.0