{"thread":{"id":"38016","subject":"[PATCH v2] Improve the filemode trustability check","startedAt":"2014-11-20T11:49:03Z","lastAt":"2014-11-20T17:55:53Z","messageCount":2,"participants":["Torsten Bögershausen","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"252250","messageId":"546DD52F.90302@web.de","threadId":"38016","inReplyTo":null,"subject":"[PATCH v2] Improve the filemode trustability check","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2014-11-20T11:49:03Z","receivedAt":"2014-11-20T11:49:03Z","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 currently not done.\n\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---\nChanges since V1:\n- Improved commit msg (hopefully)\n- Simplified the patch\n builtin/init-db.c | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/init-db.c b/builtin/init-db.c\nindex aab44d2..195a88b 100644\n--- a/builtin/init-db.c\n+++ b/builtin/init-db.c\n@@ -252,7 +252,8 @@ 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 = (!(st1.st_mode & S_IXUSR) &&\n+\t\t\t\t!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-- \n2.0.0.GIT\n"},{"id":"252270","messageId":"xmqq61e94uza.fsf@gitster.dls.corp.google.com","threadId":"38016","inReplyTo":"546DD52F.90302@web.de","subject":"Re: [PATCH v2] Improve the filemode trustability check","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-11-20T17:55:53Z","receivedAt":"2014-11-20T17:55:53Z","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 currently not done.\n>\n> Detect when \".git/config\" has the user executable bit set after\n> creat(\".git/config\", 0666) and set core.filemode to false.\n\nIs this codepath reached _only_ and immediately after creat(0666) of\nthe config file in the same process? The function has a local\nvariable reinit, which is returned to the caller to help it decide\nif we are re-initializing an existing repository, so I suspect that\nthe call to git_config_set() before this part of the code may not\nnecessarily be creating the file [*1*].\n\nI _think_ in the reinit case that makes us rewrite an existing\nconfig file, the mode bits are propagated to the new file we just\nwrote from the old one; checking st1.st_mode is therefore seeing\nnot what create(0666) gave us but whatever random bits the user had\non the original.\n\nWhich is probably not a useful source of information to gauge the\ncharacterisic of the filesystem, no?\n\n[Footnote]\n\n*1* We may have been asked to reinitialize and update an existing\nrepository that was created with the same or older repository format\nnumber.\n\n\n> Signed-off-by: Torsten Bögershausen \n> ---\n> Changes since V1:\n> - Improved commit msg (hopefully)\n> - Simplified the patch\n> builtin/init-db.c | 3 ++-\n> 1 file changed, 2 insertions(+), 1 deletion(-)\n>\n> diff --git a/builtin/init-db.c b/builtin/init-db.c\n> index aab44d2..195a88b 100644\n> --- a/builtin/init-db.c\n> +++ b/builtin/init-db.c\n> @@ -252,7 +252,8 @@ 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 = (!(st1.st_mode & S_IXUSR) &&\n> +\t\t\t\t!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"}]}