git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Frans Klaver <fransklaver@gmail.com>
To: git@vger.kernel.org
Cc: Junio C Hamano <gitster@pobox.com>, Frans Klaver <fransklaver@gmail.com>
Subject: [PATCH 1/2] run-command: Add checks after execvp fails with EACCES
Date: Tue, 13 Dec 2011 16:08:36 +0100	[thread overview]
Message-ID: <1323788917-4141-2-git-send-email-fransklaver@gmail.com> (raw)
In-Reply-To: <1323788917-4141-1-git-send-email-fransklaver@gmail.com>

execvp returns ENOENT if a command was not found after searching PATH.
If path contains a directory that current user has insufficient
privileges to, EACCES is returned. This may still mean the program
wasn't found and may cause confusion to the user, especially when the
file mentioned doesn't exist -- that is, the user would expect NOENT to
be returned -- and the user was actually hoping for an alias to be executed.

To help users track down the core issue more easily, perform some checks
on the path and file permissions involved. Output errors when paths or
files don't have enough permissions.

Signed-off-by: Frans Klaver <fransklaver@gmail.com>
---
 run-command.c          |   79 ++++++++++++++++++++++++++++++++++++++++++++++++
 t/t0061-run-command.sh |   16 +++++++++-
 2 files changed, 94 insertions(+), 1 deletions(-)

diff --git a/run-command.c b/run-command.c
index 1c51043..3f136f4 100644
--- a/run-command.c
+++ b/run-command.c
@@ -2,6 +2,7 @@
 #include "run-command.h"
 #include "exec_cmd.h"
 #include "argv-array.h"
+#include "dir.h"
 
 static inline void close_pair(int fd[2])
 {
@@ -134,6 +135,80 @@ static int wait_or_whine(pid_t pid, const char *argv0, int silent_exec_failure)
 	return code;
 }
 
+#ifndef WIN32
+static int have_read_execute_permissions(const char *path)
+{
+	if (access(path, R_OK|X_OK) == 0)
+		return 1;
+
+	if (errno == EACCES)
+		return 0;
+
+	trace_printf("could not determine permissions for '%s': %s\n", path,
+				strerror(errno));
+	return 0;
+}
+
+static void diagnose_execvp_eacces(const char *cmd, const char **argv)
+{
+	/*
+	 * man 2 execve states that EACCES is returned for:
+	 * - Search permission is denied on a component of the path prefix
+	 *   of cmd or the name of a script interpreter
+	 * - The file or script interpreter is not a regular file
+	 * - Execute permission is denied for the file, script or ELF
+	 *   interpreter
+	 * - The file system is mounted noexec
+	 */
+	struct strbuf sb = STRBUF_INIT;
+	char *path;
+	char *next;
+
+	if (strchr(cmd, '/')) {
+		if (!have_read_execute_permissions(cmd))
+			error("no read/execute permissions on '%s'\n", cmd);
+		return;
+	}
+
+	path = getenv("PATH");
+	while (path) {
+		next = strchrnul(path, ':');
+		if (path < next)
+			strbuf_add(&sb, path, next - path);
+		else
+			strbuf_addch(&sb, '.');
+
+		if (!*next)
+			path = NULL;
+		else
+			path = next + 1;
+
+		if (!have_read_execute_permissions(sb.buf)) {
+			error("no read/execute permissions on '%s'\n", sb.buf);
+			strbuf_release(&sb);
+			continue;
+		}
+
+		if (sb.len && sb.buf[sb.len - 1] != '/')
+			strbuf_addch(&sb, '/');
+		strbuf_addstr(&sb, cmd);
+
+		if (file_exists(sb.buf)) {
+			if (!have_read_execute_permissions(sb.buf))
+				error("no read/execute permissions on '%s'\n",
+						sb.buf);
+			else
+				warning("file '%s' exists and permissions "
+				"seem OK.\nIf this is a script, see if you "
+				"have sufficient privileges to run the "
+				"interpreter", sb.buf);
+		}
+
+		strbuf_release(&sb);
+	}
+}
+#endif
+
 int start_command(struct child_process *cmd)
 {
 	int need_in, need_out, need_err;
@@ -285,6 +360,10 @@ fail_pipe:
 				error("cannot run %s: %s", cmd->argv[0],
 					strerror(ENOENT));
 			exit(127);
+		} else if (errno == EACCES) {
+			diagnose_execvp_eacces(cmd->argv[0], cmd->argv);
+			die("cannot exec '%s': %s", cmd->argv[0],
+				strerror(EACCES));
 		} else {
 			die_errno("cannot exec '%s'", cmd->argv[0]);
 		}
diff --git a/t/t0061-run-command.sh b/t/t0061-run-command.sh
index 8d4938f..b39bd16 100755
--- a/t/t0061-run-command.sh
+++ b/t/t0061-run-command.sh
@@ -26,7 +26,7 @@ test_expect_success 'run_command can run a command' '
 	test_cmp empty err
 '
 
-test_expect_success POSIXPERM 'run_command reports EACCES' '
+test_expect_success POSIXPERM 'run_command reports EACCES, file permissions' '
 	cat hello-script >hello.sh &&
 	chmod -x hello.sh &&
 	test_must_fail test-run-command run-command ./hello.sh 2>err &&
@@ -34,4 +34,18 @@ test_expect_success POSIXPERM 'run_command reports EACCES' '
 	grep "fatal: cannot exec.*hello.sh" err
 '
 
+test_expect_success POSIXPERM 'run_command reports EACCES, search path permisions' '
+	mkdir -p inaccessible &&
+	PATH=$(pwd)/inaccessible:$PATH &&
+	export PATH &&
+
+	cat hello-script >inaccessible/hello.sh &&
+	chmod 400 inaccessible &&
+	test_must_fail test-run-command run-command hello.sh 2>err &&
+	chmod 755 inaccessible &&
+
+	grep "fatal: cannot exec.*hello.sh" err &&
+	grep "no read/execute permissions on" err
+'
+
 test_done
-- 
1.7.8

  reply	other threads:[~2011-12-13 15:09 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-11-21 21:53 [PATCH] run-command.c: Accept EACCES as command not found Frans Klaver
2011-11-21 22:13 ` Junio C Hamano
2011-11-21 23:06   ` Frans Klaver
2011-11-21 23:54     ` Junio C Hamano
2011-11-22  9:31       ` Frans Klaver
2011-11-23  8:17         ` Frans Klaver
2011-11-23 12:04           ` Nguyen Thai Ngoc Duy
2011-11-23 13:25             ` Frans Klaver
2011-11-23 22:55           ` Frans Klaver
2011-12-06 21:38             ` [PATCH 0/2] run-command: Add EACCES diagnostics Frans Klaver
2011-12-06 21:38               ` [PATCH 1/2] run-command: Add checks after execvp fails with EACCES Frans Klaver
2011-12-06 22:35                 ` Junio C Hamano
2011-12-07  8:31                   ` Frans Klaver
2011-12-08 21:44                   ` Frans Klaver
2011-12-09 17:23                     ` Junio C Hamano
2011-12-09 21:35                       ` Frans Klaver
2011-12-06 21:38               ` [PATCH 2/2] run-command: Add interpreter permissions check Frans Klaver
2011-12-06 22:47                 ` Junio C Hamano
2011-12-07  8:37                   ` Frans Klaver
2011-12-13 15:08             ` [PATCH 0/2 v2] run-command: Add eacces diagnostics Frans Klaver
2011-12-13 15:08               ` Frans Klaver [this message]
2011-12-13 19:01                 ` [PATCH 1/2] run-command: Add checks after execvp fails with EACCES Junio C Hamano
2011-12-14 14:31                   ` Frans Klaver
2011-12-14 22:06                     ` Frans Klaver
2011-12-13 15:08               ` [PATCH 2/2] run-command: Add interpreter permissions check Frans Klaver

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1323788917-4141-2-git-send-email-fransklaver@gmail.com \
    --to=fransklaver@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).