Re: [PATCH] Improve the filemode trustability check

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Torsten Bögershausen <tboegi@xxxxxx> writes:

> Some file systems do not support the executable bit:
> a) The user executable bit is always 0, e.g. VFAT mounted with -onoexec
> b) The user executable bit is always 1, e.g. cifs mounted with -ofile_mode=0755
> c) There are system where user executable bit is 1 even if it should be 0
>    like b), but the file mode can be maintained locally. chmod -x changes the
>    file mode from 0766 to 0666, until the file system is unmounted and
>    remounted and the file mode is 0766 again.
>    This been observed when a Windows machine with NTFS exports a share to
>    Mac OS X via smb or afp.
>
> Case a) and b) are handled by the current code.
> Case c) qualifies as "non trustable executable bit" and core.filemode
> should be false, but this is not done.
>
> Solution:
> Detect when ".git/config" has the user executable bit set after
> creat(".git/config", 0666) and set core.filemode to false.

The readers have been following along a nicely flowing prose; let
them keep going by dropping that abrupt "Solution:" line and instead
doing s/to false./to false to solve this./ or something like that.

The change does not seem to match the above design of the solution,
though.  We've run stat(path, &st1), and I would have expected from
the description that (st1.st_mode & S_IXUSR) without any chmod()
would be the "does the file have executable bit without us asking?"
Either the explanation or the code is wrong.  I cannot tell which.

> Signed-off-by: Torsten Bögershausen <tboegi@xxxxxx>
> ---
>
> This should go on top of "mh/config-flip-xbit-back-after-checking"
>
> Michael, thanks for the test case.
> And no, I havent seen any systems with behaving like d)
>
>  builtin/init-db.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/builtin/init-db.c b/builtin/init-db.c
> index aab44d2..9b50dde 100644
> --- a/builtin/init-db.c
> +++ b/builtin/init-db.c
> @@ -252,10 +252,10 @@ static int create_default_files(const char *template_path)
>  	filemode = TEST_FILEMODE;
>  	if (TEST_FILEMODE && !lstat(path, &st1)) {
>  		struct stat st2;
> -		filemode = (!chmod(path, st1.st_mode ^ S_IXUSR) &&
> +		filemode = (!chmod(path, st1.st_mode | S_IXUSR) &&
>  				!lstat(path, &st2) &&
>  				st1.st_mode != st2.st_mode &&
> -				!chmod(path, st1.st_mode));
> +				!chmod(path, st1.st_mode & (~S_IXUSR)));
>  	}
>  	git_config_set("core.filemode", filemode ? "true" : "false");
--
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




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]