{"thread":{"id":"38009","subject":"[PATCH] Improve the filemode trustability check","startedAt":"2014-11-19T15:10:48Z","lastAt":"2014-11-19T18:58:54Z","messageCount":2,"participants":["Torsten Bögershausen","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"252181","messageId":"546CB2F8.7040501@web.de","threadId":"38009","inReplyTo":null,"subject":"[PATCH] Improve the filemode trustability check","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2014-11-19T15:10:48Z","receivedAt":"2014-11-19T15:10:48Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"Some file systems do not support the executable bit:\na) The user executable bit is always 0, e.g. VFAT mounted with -onoexec\nb) The user executable bit is always 1, e.g. cifs mounted with -ofile_mode=0755\nc) There are system where user executable bit is 1 even if it should be 0\n like b), but the file mode can be maintained locally. chmod -x changes the\n file mode from 0766 to 0666, until the file system is unmounted and\n remounted and the file mode is 0766 again.\n This been observed when a Windows machine with NTFS exports a share to\n Mac OS X via smb or afp.\n\nCase a) and b) are handled by the current code.\nCase c) qualifies as \"non trustable executable bit\" and core.filemode\nshould be false, but this is not done.\n\nSolution:\nDetect when \".git/config\" has the user executable bit set after\ncreat(\".git/config\", 0666) and set core.filemode to false.\n\nSigned-off-by: Torsten Bögershausen \n---\n\nThis should go on top of \"mh/config-flip-xbit-back-after-checking\"\n\nMichael, thanks for the test case.\nAnd no, I havent seen any systems with behaving like d)\n\n builtin/init-db.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/init-db.c b/builtin/init-db.c\nindex aab44d2..9b50dde 100644\n--- a/builtin/init-db.c\n+++ b/builtin/init-db.c\n@@ -252,10 +252,10 @@ static int create_default_files(const char *template_path)\n \tfilemode = TEST_FILEMODE;\n \tif (TEST_FILEMODE && !lstat(path, &st1)) {\n \t\tstruct stat st2;\n-\t\tfilemode = (!chmod(path, st1.st_mode ^ S_IXUSR) &&\n+\t\tfilemode = (!chmod(path, st1.st_mode | S_IXUSR) &&\n \t\t\t\t!lstat(path, &st2) &&\n \t\t\t\tst1.st_mode != st2.st_mode &&\n-\t\t\t\t!chmod(path, st1.st_mode));\n+\t\t\t\t!chmod(path, st1.st_mode & (~S_IXUSR)));\n \t}\n \tgit_config_set(\"core.filemode\", filemode ? \"true\" : \"false\");\n \n-- \n1.9.1.dirty\n"},{"id":"252195","messageId":"xmqqegszrp8x.fsf@gitster.dls.corp.google.com","threadId":"38009","inReplyTo":"546CB2F8.7040501@web.de","subject":"Re: [PATCH] Improve the filemode trustability check","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-11-19T18:58:54Z","receivedAt":"2014-11-19T18:58:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Torsten Bögershausen writes:\n\n> Some file systems do not support the executable bit:\n> a) The user executable bit is always 0, e.g. VFAT mounted with -onoexec\n> b) The user executable bit is always 1, e.g. cifs mounted with -ofile_mode=0755\n> c) There are system where user executable bit is 1 even if it should be 0\n> like b), but the file mode can be maintained locally. chmod -x changes the\n> file mode from 0766 to 0666, until the file system is unmounted and\n> remounted and the file mode is 0766 again.\n> This been observed when a Windows machine with NTFS exports a share to\n> Mac OS X via smb or afp.\n>\n> Case a) and b) are handled by the current code.\n> Case c) qualifies as \"non trustable executable bit\" and core.filemode\n> should be false, but this is not done.\n>\n> Solution:\n> Detect when \".git/config\" has the user executable bit set after\n> creat(\".git/config\", 0666) and set core.filemode to false.\n\nThe readers have been following along a nicely flowing prose; let\nthem keep going by dropping that abrupt \"Solution:\" line and instead\ndoing s/to false./to false to solve this./ or something like that.\n\nThe change does not seem to match the above design of the solution,\nthough. We've run stat(path, &st1), and I would have expected from\nthe description that (st1.st_mode & S_IXUSR) without any chmod()\nwould be the \"does the file have executable bit without us asking?\"\nEither the explanation or the code is wrong. I cannot tell which.\n\n> Signed-off-by: Torsten Bögershausen \n> ---\n>\n> This should go on top of \"mh/config-flip-xbit-back-after-checking\"\n>\n> Michael, thanks for the test case.\n> And no, I havent seen any systems with behaving like d)\n>\n> builtin/init-db.c | 4 ++--\n> 1 file changed, 2 insertions(+), 2 deletions(-)\n>\n> diff --git a/builtin/init-db.c b/builtin/init-db.c\n> index aab44d2..9b50dde 100644\n> --- a/builtin/init-db.c\n> +++ b/builtin/init-db.c\n> @@ -252,10 +252,10 @@ static int create_default_files(const char *template_path)\n> \tfilemode = TEST_FILEMODE;\n> \tif (TEST_FILEMODE && !lstat(path, &st1)) {\n> \t\tstruct stat st2;\n> -\t\tfilemode = (!chmod(path, st1.st_mode ^ S_IXUSR) &&\n> +\t\tfilemode = (!chmod(path, st1.st_mode | S_IXUSR) &&\n> \t\t\t\t!lstat(path, &st2) &&\n> \t\t\t\tst1.st_mode != st2.st_mode &&\n> -\t\t\t\t!chmod(path, st1.st_mode));\n> +\t\t\t\t!chmod(path, st1.st_mode & (~S_IXUSR)));\n> \t}\n> \tgit_config_set(\"core.filemode\", filemode ? \"true\" : \"false\");\n"}]}