Frans Klaver <fransklaver@xxxxxxxxx> writes:
> +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;
> + }
> +
Three points.
- error() gives you a LF at the end, so you do not have to have your own.
- That "have_..._ions()" is too long and ugly.
- The only thing you care about this callsite is if you have enough
permission to execute the "cmd".
In fact, you should not unconditionally require read permissions here.
$ chmod a-r $(type --path git) && /bin/ls -l $(type --path git)
--wx--x--x 109 junio junio 5126580 Dec 13 09:47 /home/junio/git-active/bin/git
$ /home/junio/git-active/bin/git --version
git version 1.7.8.249.gb1b73
You may need read permission when the file is a script (i.e. not binary
executable).
> + 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)) {
When checking if you can run "foo/bar/baz", directories "foo/" and "foo/bar/"
do not have to be readable. They only have to have executable bit to allow
descending into them, and typically this is called "searchable" (see man chmod).
$ mkdir -p /var/tmp/a/b && cp $(type --path git) /var/tmp/a/b/git
$ chmod 111 /var/tmp/a /var/tmp/a/b
$ /var/tmp/a/b/git --version
git version 1.7.8.249.gb1b73
I'd suggest having two helper functions, instead of the single one with
overlong "have...ions" name.
- can_search_directory() checks with access(X_OK);
- can_execute_file() checks with access(X_OK|R_OK), even though R_OK is
not always needed.
Use the former here where you check the directory that contains the
command, and use the latter up above where you check the command that is
supposed to be executable, and also down below after you checked sb.buf is
a path to a file that may be the command that is supposed to be
executable.
Then patch 2/2 can extend can_execute() to enhance its support for scripts
by reading the hash-bang line and validating it, etc.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html