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

@@ -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

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

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 */

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. */
+	/* Check permissions based on the given open flags, except when we are
-	if ((r = forbidden(fp, vp, bits)) == OK) {
+	 * 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:

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

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
 

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

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 */
+}