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

diff --git a/distrib/sets/lists/minix/mi b/distrib/sets/lists/minix/mi
index ff0024099a1b0948501df3db8164f1a0391b08f5..ea23bc671ff9db6cfcbf18c64b52fe13cd673aee 100644 (file)
--- 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 4a371a4fe03efb9993a92e3548a4f513cb771139..1eef3821dbd33861f5fbe0ce23602d5bd77e14a5 100644 (file)
--- 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 8a88fa863ac9db5af107a31db4aa0b080f5d266b..6e740f8a232076800eee4db8176683b87c713f12 100644 (file)
--- 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 1fbdbaf87b10c5d1a49129829d27fc9c29f9e908..0ca6848548865f68da9b8cf4b1ffff95299d3c0e 100644 (file)
--- 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 ba37fb2ecea481e68068a755ccf9a8c13ec51136..aeddf46469b380fd4eb62b11a8273e4bc7afdf72 100644 (file)
--- 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 db91e52d07e7e5d8dbe921ed96aedba60242ad9c..2543b0b6fd59a69453a2a6ef0dd56b4635c2dc51 100644 (file)
--- 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 d442d2591c1a418aeaca5b52fe91d816cf0ee6bc..8beeaa4de04278b03a940e6771ed81cd834e79ec 100755 (executable)
--- 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 (file)
index 0000000..97ee3bf
--- /dev/null
+++ b/minix/tests/test86.c
@@ -0,0 +1,50 @@
+#include <stdlib.h>
+#include <string.h>
+#include <limits.h>
+#include <fcntl.h>
+#include <sys/wait.h>
+
+#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 */
+}