Re: [PATCH 1/2] run-command: Add checks after execvp fails with EACCES

From: Frans Klaver <fransklaver@gmail.com>
Date: Dec 8, 2011, 21:44 UTC
Message-ID: <op.v56xbxqs0aolir@keputer>
In-Reply-To: <7vpqg1e3au.fsf@alter.siamese.dyndns.org>

On Tue, 06 Dec 2011 23:35:53 +0100, Junio C Hamano <gitster@pobox.com> wrote:

Show 20 quoted lines

> Frans Klaver <fransklaver@gmail.com> writes:
>
>> +#ifndef WIN32
>> +static int is_in_group(gid_t gid)
>> ...
>> +static int have_read_execute_permissions(const char *path)
>> +{
>> +	struct stat s;
>> +	trace_printf("checking '%s'\n", path);
>> +
>> +	if (stat(path, &s) < 0) {
>> + ...
>> +	/* check world permissions */
>> +	if ((s.st_mode&(S_IXOTH|S_IROTH)) == (S_IXOTH|S_IROTH))
>> +		return 1;
>
> Hmm, do you need to do this with stat(2)?
>
> Wouldn't access(2) with R_OK|X_OK give you exactly what you want without
> this much trouble?

I just had a good look through the man page of access(2), and I think it depends. access works for the real uid, which is what I attempted to implement in the above check as well. However, do we actually need to use the real uid or do we need the set uid (geteuid(2))? Would it be safe to assume we don't setuid?

Show 11 quoted lines

> I also think that your permission check is incorrectly implemented.
>
>     $ cd /var/tmp && date >j && chmod 044 j && ls -l j
>     ----r--r-- 1 junio junio 29 Dec  6 14:32 j
>     $ cat j
>     cat: j: Permission denied
>     $ su pogo
>     Password:
>     $ cat j
>     Tue Dec  6 14:32:23 PST 2011
> That's a world-readable but unreadable-only-to-me file.

Will fix if we can't use access(2) due to what I mentioned above.

Show 6 quoted lines

>> +				warn("file '%s' exists and permissions "
>> +				"seem OK.\nIf this is a script, see if you "
>> +				"have sufficient privileges to run the "
>> +				"interpreter", sb.buf);
>
> Does "warn()" do the right thing for multi-line strings like this?

Looking back on it, I think I actually wanted to use warning() from usage.c. I'll still have to check if that does the multi-line thing as I expect it to.