{"thread":{"id":"35042","subject":"[PATCH] mingw-multibyte: fix memory acces violation and path length limits.","startedAt":"2013-09-28T21:17:16Z","lastAt":"2013-12-14T11:31:16Z","messageCount":44,"participants":["Wataru Noguchi","Johannes Schindelin","Stefan Beller","René Scharfe","Erik Faye-Lund","Antoine Pelisse","Torsten Bögershausen","Ondřej Bílka","Duy Nguyen","Johannes Sixt","Jeff King","Nguyễn Thái Ngọc Duy","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"228364","messageId":"1380403036-20413-1-git-send-email-wnoguchi.0727@gmail.com","threadId":"35042","inReplyTo":null,"subject":"[PATCH] mingw-multibyte: fix memory acces violation and path length limits.","fromName":"Wataru Noguchi","fromEmail":"wnoguchi.0727@gmail.com","sentAt":"2013-09-28T21:17:16Z","receivedAt":"2013-09-28T21:17:16Z","isPatch":true,"sender":{"key":"wnoguchi.0727@gmail.com","avatar":"https://gravatar.com/avatar/92adeb8d30332b0a1204df49b55442bf91ea38b678bb7406c7bf66f72f51b569?d=mp&s=160"},"body":"fix: Git for Windows crashes when clone Japanese multibyte repository.\n\nReproduce condition:\n\n- Japanese Base Encoding is Shift-JIS.\n- It happens Japanese multibyte directory name and too-long directory path\n- Linux(ex. Ubuntu 13.04 amd64) can clone normally.\n- example repository is here:\n\ngit clone https://github.com/wnoguchi/mingw-checkout-crash.git\n\n- The reproduce crash repository contains following file only.\n - following directory and file name is encoded for this commit log.\n - actually file name is decoded.]\n %E6%97%A5%E6%9C%AC%E8%AA%9E%E3%83%87%E3%82%A3%E3%83%AC%E3%82%AF%E3%83%88%E3%83%AA%201-long-long-long-dirname/%E6%97%A5%E6%9C%AC%E8%AA%9E%E3%83%87%E3%82%A3%E3%83%AC%E3%82%AF%E3%83%88%E3%83%AA%202-long-long-long-dirname/%E6%97%A5%E6%9C%AC%E8%AA%9E%E3%83%87%E3%82%A3%E3%83%AC%E3%82%AF%E3%83%88%E3%83%AA%203-long-long-long-dirname/%E6%97%A5%E6%9C%AC%E8%AA%9E%E3%83%87%E3%82%A3%E3%83%AC%E3%82%AF%E3%83%88%E3%83%AA%204-long-long-long-dirname/%E6%97%A5%E6%9C%AC%E8%AA%9E%E3%83%87%E3%82%A3%E3%83%AC%E3%82%AF%E3%83%88%E3%83%AA%205-long-long-long-dirname/%E3%81%AF%E3%81%98%E3%82%81%E3%81%AB%E3%81%8A%E8%AA%AD%E3%81%BF%E3%81%8F%E3%81%A0%E3%81%95%E3%81%84.txt\n- only one commit.\n\nCause:\n\n- convert_attrs() in convert.c: if (!ccheck[0].attr) but ccheck[0].attr always not NULL.\n thus git_check_attr() in attr.c (check[i].attr->attr_nr) cause access violation.\n- checkout_entry() in entry.c: static char path[PATH_MAX + 1]; declared.\n But its size is 261 on MinGW environment.\n This length is Windows full path limits. But for relative path is too short.\n\nThis commit fixes:\n\n- convert_attrs() in convert.c: initialize ccheck[0].attr with NULL.\n- git-compat-util.h: redifine PATH_MAX value to 4096 when MinGW environment.\n\nSigned-off-by: Wataru Noguchi \n---\n convert.c | 5 +++++\n git-compat-util.h | 10 ++++++++++\n 2 files changed, 15 insertions(+)\n\ndiff --git a/convert.c b/convert.c\nindex 11a95fc..5eaa206 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -724,6 +724,11 @@ static void convert_attrs(struct conv_attrs *ca, const char *path)\n {\n \tint i;\n \tstatic struct git_attr_check ccheck[NUM_CONV_ATTRS];\n+\t\n+\tif (NUM_CONV_ATTRS != 0) {\n+\t\tccheck[0].attr = NULL;\n+\t\tccheck[0].value = NULL;\n+\t}\n \n \tif (!ccheck[0].attr) {\n \t\tfor (i = 0; i < NUM_CONV_ATTRS; i++)\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex a31127f..ba02c69 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -237,6 +237,16 @@ extern char *gitbasename(char *);\n #ifndef PATH_MAX\n #define PATH_MAX 4096\n #endif\n+#ifdef GIT_WINDOWS_NATIVE\n+/* Git for Windows checkout PATH_MAX is reduce to 260.\n+ * but if checkout relative long path name, its length too short.\n+ * thus, expand length.\n+ */\n+#ifdef PATH_MAX\n+#undef PATH_MAX\n+#endif\n+#define PATH_MAX 4096\n+#endif\n \n #ifndef PRIuMAX\n #define PRIuMAX \"llu\"\n-- \n1.8.1.2\n\n-- \n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n\n--- \nYou received this message because you are subscribed to the Google Groups \"msysGit\" group.\nTo unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.\nFor more options, visit https://groups.google.com/groups/opt_out.\n"},{"id":"228426","messageId":"alpine.DEB.1.00.1309290112380.1191@s15462909.onlinehome-server.info","threadId":"35042","inReplyTo":"1380403036-20413-1-git-send-email-wnoguchi.0727@gmail.com","subject":"Re: [PATCH] mingw-multibyte: fix memory acces violation and path length limits.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2013-09-28T23:18:38Z","receivedAt":"2013-09-28T23:18:38Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 29 Sep 2013, Wataru Noguchi wrote:\n\n> --- a/convert.c\n> +++ b/convert.c\n> @@ -724,6 +724,11 @@ static void convert_attrs(struct conv_attrs *ca, const char *path)\n> {\n> \tint i;\n> \tstatic struct git_attr_check ccheck[NUM_CONV_ATTRS];\n> +\t\n> +\tif (NUM_CONV_ATTRS != 0) {\n> +\t\tccheck[0].attr = NULL;\n> +\t\tccheck[0].value = NULL;\n> +\t}\n\nI wonder whether it would make more sense to use\n\n\tmemset(ccheck, 0, sizeof(ccheck))\n\n? But then, ccheck is static and *should* be initialized to all 0\naccording to the C standard. And re-initializing it to NULL would\ninvalidate the values that were set earlier.\n\nAlso, if NUM_CONV_ATTRS == 0, I would expect\n\n> \tif (!ccheck[0].attr) {\n\nto access an invalid location...\n\n> diff --git a/git-compat-util.h b/git-compat-util.h\n> index a31127f..ba02c69 100644\n> --- a/git-compat-util.h\n> +++ b/git-compat-util.h\n> @@ -237,6 +237,16 @@ extern char *gitbasename(char *);\n> #ifndef PATH_MAX\n> #define PATH_MAX 4096\n> #endif\n> +#ifdef GIT_WINDOWS_NATIVE\n> +/* Git for Windows checkout PATH_MAX is reduce to 260.\n> + * but if checkout relative long path name, its length too short.\n> + * thus, expand length.\n> + */\n> +#ifdef PATH_MAX\n> +#undef PATH_MAX\n> +#endif\n> +#define PATH_MAX 4096\n> +#endif\n\nThis looks fine, but I am wary... did you not say that a crash was caused\nby this? In that case, we would have a user that accesses the respective\nbuffer without checking the size and we would still have to fix that bug..\n\nCiao,\nDscho\n\n-- \n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n\n--- \nYou received this message because you are subscribed to the Google Groups \"msysGit\" group.\nTo unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.\nFor more options, visit https://groups.google.com/groups/opt_out.\n"},{"id":"228431","messageId":"524796DC.5020302@gmail.com","threadId":"35042","inReplyTo":"alpine.DEB.1.00.1309290112380.1191@s15462909.onlinehome-server.info","subject":"Re: [PATCH] mingw-multibyte: fix memory acces violation and path length limits.","fromName":"Wataru Noguchi","fromEmail":"wnoguchi.0727@gmail.com","sentAt":"2013-09-29T02:56:28Z","receivedAt":"2013-09-29T02:56:28Z","isPatch":true,"sender":{"key":"wnoguchi.0727@gmail.com","avatar":"https://gravatar.com/avatar/92adeb8d30332b0a1204df49b55442bf91ea38b678bb7406c7bf66f72f51b569?d=mp&s=160"},"body":"Hi,\n\nThanks for comments.\n\nMy currently working repository is\n\nhttps://github.com/wnoguchi/git/tree/hotfix/mingw-multibyte-path-checkout-failure\n\nI have revert commits to 1f10da3.\nI'll try failure step.\n\n- gcc optimization level is O2.(fail)\n- gcc O0, O1 works fine.\n\n\n$ gdb git-clone\nGNU gdb 6.8\nCopyright (C) 2008 Free Software Foundation, Inc.\nLicense GPLv3+: GNU GPL version 3 or later \nThis is free software: you are free to change and redistribute it.\nThere is NO WARRANTY, to the extent permitted by law. Type \"show copying\"\nand \"show warranty\" for details.\nThis GDB was configured as \"i686-pc-mingw32\"...\n(gdb) r https://github.com/wnoguchi/mingw-checkout-crash.git\nStarting program: C:\\msysgit\\git/git-clone.exe https://github.com/wnoguchi/mingw\n-checkout-crash.git\n[New thread 800.0xa10]\nError: dll starting at 0x779f0000 not found.\nError: dll starting at 0x75900000 not found.\nError: dll starting at 0x779f0000 not found.\nError: dll starting at 0x778f0000 not found.\n[New thread 800.0x92c]\nCloning into 'mingw-checkout-crash'...\nError: dll starting at 0x29f0000 not found.\nremote: Counting objects: 8, done.\nremote: Compressing objects: 100% (7/7), done.\nremote: Total 8 (delta 0), reused 8 (delta 0)\nUnpacking objects: 100% (8/8), done.\nChecking connectivity... done\n[New thread 800.0xea0]\n\nProgram received signal SIGSEGV, Segmentation fault.\n0x004d5200 in git_check_attr (\n path=0xacc6a0 \"\"..., num=5, check=0x572440) at attr.c:754\n754 const char *value = check_all_attr[check[i].attr->attr_n\nr].value;\n(gdb) list\n749 int i;\n750\n751 collect_all_attrs(path);\n752\n753 for (i = 0; i < num; i++) {\n754 const char *value = check_all_attr[check[i].attr->attr_n\nr].value;\n755 if (value == ATTR__UNKNOWN)\n756 value = ATTR__UNSET;\n757 check[i].value = value;\n758 }\n(gdb) up\n#1 0x004a796b in convert_attrs (ca=0x28f950,\n path=0xacc6a0 \"\"...) at convert.c:740\n740 if (!git_check_attr(path, NUM_CONV_ATTRS, ccheck)) {\n(gdb) list\n735 ccheck[i].attr = git_attr(conv_attr_name[i]);\n736 user_convert_tail = &user_convert;\n737 git_config(read_convert_config, NULL);\n738 }\n739\n740 if (!git_check_attr(path, NUM_CONV_ATTRS, ccheck)) {\n741 ca->crlf_action = git_path_check_crlf(path, ccheck + 4);\n\n742 if (ca->crlf_action == CRLF_GUESS)\n743 ca->crlf_action = git_path_check_crlf(path, cche\nck + 0);\n744 ca->ident = git_path_check_ident(path, ccheck + 1);\n(gdb) list -\n725 int i;\n726 static struct git_attr_check ccheck[NUM_CONV_ATTRS];\n727\n728 // if (NUM_CONV_ATTRS != 0) {\n729 // ccheck[0].attr = NULL;\n730 // ccheck[0].value = NULL;\n731 // }\n732\n733 if (!ccheck[0].attr) {\n734 for (i = 0; i < NUM_CONV_ATTRS; i++)\n(gdb) p ccheck[0].attr\n$1 = (struct git_attr *) 0xa081e38f\n\n\nNext, change PATH_MAX value to 4096 if MinGW environment.(6cae216)\nWorks fine.\nIs this failure caused by PATH_MAX length too short? or my repository directory depth too deep?\nBut when optimization disabled(O0) in Makefile , works fine...(bf0acff)\nDo you know what's happen?\n\n\nThanks.\n\n\n(2013/09/29 8:18), Johannes Schindelin wrote:\n> Hi,\n>\n> On Sun, 29 Sep 2013, Wataru Noguchi wrote:\n>\n>> --- a/convert.c\n>> +++ b/convert.c\n>> @@ -724,6 +724,11 @@ static void convert_attrs(struct conv_attrs *ca, const char *path)\n>> {\n>> \tint i;\n>> \tstatic struct git_attr_check ccheck[NUM_CONV_ATTRS];\n>> +\t\n>> +\tif (NUM_CONV_ATTRS != 0) {\n>> +\t\tccheck[0].attr = NULL;\n>> +\t\tccheck[0].value = NULL;\n>> +\t}\n>\n> I wonder whether it would make more sense to use\n>\n> \tmemset(ccheck, 0, sizeof(ccheck))\n>\n> ? But then, ccheck is static and *should* be initialized to all 0\n> according to the C standard. And re-initializing it to NULL would\n> invalidate the values that were set earlier.\n>\n> Also, if NUM_CONV_ATTRS == 0, I would expect\n>\n>> \tif (!ccheck[0].attr) {\n>\n> to access an invalid location...\n>\n>> diff --git a/git-compat-util.h b/git-compat-util.h\n>> index a31127f..ba02c69 100644\n>> --- a/git-compat-util.h\n>> +++ b/git-compat-util.h\n>> @@ -237,6 +237,16 @@ extern char *gitbasename(char *);\n>> #ifndef PATH_MAX\n>> #define PATH_MAX 4096\n>> #endif\n>> +#ifdef GIT_WINDOWS_NATIVE\n>> +/* Git for Windows checkout PATH_MAX is reduce to 260.\n>> + * but if checkout relative long path name, its length too short.\n>> + * thus, expand length.\n>> + */\n>> +#ifdef PATH_MAX\n>> +#undef PATH_MAX\n>> +#endif\n>> +#define PATH_MAX 4096\n>> +#endif\n>\n> This looks fine, but I am wary... did you not say that a crash was caused\n> by this? In that case, we would have a user that accesses the respective\n> buffer without checking the size and we would still have to fix that bug..\n>\n> Ciao,\n> Dscho\n>\n\n-- \n=========================================\n Wataru Noguchi\n wnoguchi.0727@gmail.com\n http://wnoguchi.github.io/\n=========================================\n\n-- \n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n\n--- \nYou received this message because you are subscribed to the Google Groups \"msysGit\" group.\nTo unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.\nFor more options, visit https://groups.google.com/groups/opt_out.\n"},{"id":"228451","messageId":"5248088F.2060902@googlemail.com","threadId":"35042","inReplyTo":"524796DC.5020302@gmail.com","subject":"Re: [msysGit] [PATCH] mingw-multibyte: fix memory acces violation and path length limits.","fromName":"Stefan Beller","fromEmail":"stefanbeller@googlemail.com","sentAt":"2013-09-29T11:01:35Z","receivedAt":"2013-09-29T11:01:35Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On 09/29/2013 04:56 AM, Wataru Noguchi wrote:\n> \n> - gcc optimization level is O2.(fail)\n> - gcc O0, O1 works fine.\n\nMaybe you could try to compile with\nSTACK found at http://css.csail.mit.edu/stack/\nThat tool is designed to find\nOptimization-unstable code.\n\n\n\n"},{"id":"228471","messageId":"5249AE2A.3050302@web.de","threadId":"35042","inReplyTo":"524796DC.5020302@gmail.com","subject":"Re: [PATCH] mingw-multibyte: fix memory acces violation and path length limits.","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2013-09-30T17:00:26Z","receivedAt":"2013-09-30T17:00:26Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 29.09.2013 04:56, schrieb Wataru Noguchi:\n> Hi,\n> \n> Thanks for comments.\n> \n> My currently working repository is\n> \n> https://github.com/wnoguchi/git/tree/hotfix/mingw-multibyte-path-checkout-failure\n> \n> I have revert commits to 1f10da3.\n> I'll try failure step.\n> \n> - gcc optimization level is O2.(fail)\n> - gcc O0, O1 works fine.\n> \n> \n> $ gdb git-clone\n> GNU gdb 6.8\n> Copyright (C) 2008 Free Software Foundation, Inc.\n> License GPLv3+: GNU GPL version 3 or later \n> This is free software: you are free to change and redistribute it.\n> There is NO WARRANTY, to the extent permitted by law. Type \"show copying\"\n> and \"show warranty\" for details.\n> This GDB was configured as \"i686-pc-mingw32\"...\n> (gdb) r https://github.com/wnoguchi/mingw-checkout-crash.git\n> Starting program: C:\\msysgit\\git/git-clone.exe https://github.com/wnoguchi/mingw\n> -checkout-crash.git\n> [New thread 800.0xa10]\n> Error: dll starting at 0x779f0000 not found.\n> Error: dll starting at 0x75900000 not found.\n> Error: dll starting at 0x779f0000 not found.\n> Error: dll starting at 0x778f0000 not found.\n> [New thread 800.0x92c]\n> Cloning into 'mingw-checkout-crash'...\n> Error: dll starting at 0x29f0000 not found.\n> remote: Counting objects: 8, done.\n> remote: Compressing objects: 100% (7/7), done.\n> remote: Total 8 (delta 0), reused 8 (delta 0)\n> Unpacking objects: 100% (8/8), done.\n> Checking connectivity... done\n> [New thread 800.0xea0]\n> \n> Program received signal SIGSEGV, Segmentation fault.\n> 0x004d5200 in git_check_attr (\n> path=0xacc6a0 \"\"..., num=5, check=0x572440) at attr.c:754\n> 754 const char *value = check_all_attr[check[i].attr->attr_n\n> r].value;\n> (gdb) list\n> 749 int i;\n> 750\n> 751 collect_all_attrs(path);\n> 752\n> 753 for (i = 0; i < num; i++) {\n> 754 const char *value = check_all_attr[check[i].attr->attr_n\n> r].value;\n> 755 if (value == ATTR__UNKNOWN)\n> 756 value = ATTR__UNSET;\n> 757 check[i].value = value;\n> 758 }\n\nI get a different crash on Linux if I set PATH_MAX to 260. The following\nhackish patch prevents it. Does it help in your case as well? If it does\nthen I'll send a nicer (but longer) one.\n\nThanks,\nRené\n\n\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex 1a61e6f..9bd7dcb 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -961,7 +961,7 @@ static int clear_ce_flags(struct cache_entry **cache, int nr,\n \t\t\t int select_mask, int clear_mask,\n \t\t\t struct exclude_list *el)\n {\n-\tchar prefix[PATH_MAX];\n+\tchar prefix[4096];\n \treturn clear_ce_flags_1(cache, nr,\n \t\t\t\tprefix, 0,\n \t\t\t\tselect_mask, clear_mask,\n"},{"id":"228479","messageId":"CABPQNSbsySuqyb-KucGMgnUVG1zSttkgUxsXPYzp=68YdGV7cA@mail.gmail.com","threadId":"35042","inReplyTo":"5249AE2A.3050302@web.de","subject":"Re: [PATCH] mingw-multibyte: fix memory acces violation and path length limits.","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2013-09-30T21:02:05Z","receivedAt":"2013-09-30T21:02:05Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Mon, Sep 30, 2013 at 7:00 PM, René Scharfe wrote:\n> Am 29.09.2013 04:56, schrieb Wataru Noguchi:\n>> Hi,\n>>\n>> Thanks for comments.\n>>\n>> My currently working repository is\n>>\n>> https://github.com/wnoguchi/git/tree/hotfix/mingw-multibyte-path-checkout-failure\n>>\n>> I have revert commits to 1f10da3.\n>> I'll try failure step.\n>>\n>> - gcc optimization level is O2.(fail)\n>> - gcc O0, O1 works fine.\n>>\n>>\n>> $ gdb git-clone\n>> GNU gdb 6.8\n>> Copyright (C) 2008 Free Software Foundation, Inc.\n>> License GPLv3+: GNU GPL version 3 or later \n>> This is free software: you are free to change and redistribute it.\n>> There is NO WARRANTY, to the extent permitted by law. Type \"show copying\"\n>> and \"show warranty\" for details.\n>> This GDB was configured as \"i686-pc-mingw32\"...\n>> (gdb) r https://github.com/wnoguchi/mingw-checkout-crash.git\n>> Starting program: C:\\msysgit\\git/git-clone.exe https://github.com/wnoguchi/mingw\n>> -checkout-crash.git\n>> [New thread 800.0xa10]\n>> Error: dll starting at 0x779f0000 not found.\n>> Error: dll starting at 0x75900000 not found.\n>> Error: dll starting at 0x779f0000 not found.\n>> Error: dll starting at 0x778f0000 not found.\n>> [New thread 800.0x92c]\n>> Cloning into 'mingw-checkout-crash'...\n>> Error: dll starting at 0x29f0000 not found.\n>> remote: Counting objects: 8, done.\n>> remote: Compressing objects: 100% (7/7), done.\n>> remote: Total 8 (delta 0), reused 8 (delta 0)\n>> Unpacking objects: 100% (8/8), done.\n>> Checking connectivity... done\n>> [New thread 800.0xea0]\n>>\n>> Program received signal SIGSEGV, Segmentation fault.\n>> 0x004d5200 in git_check_attr (\n>> path=0xacc6a0 \"\"..., num=5, check=0x572440) at attr.c:754\n>> 754 const char *value = check_all_attr[check[i].attr->attr_n\n>> r].value;\n>> (gdb) list\n>> 749 int i;\n>> 750\n>> 751 collect_all_attrs(path);\n>> 752\n>> 753 for (i = 0; i < num; i++) {\n>> 754 const char *value = check_all_attr[check[i].attr->attr_n\n>> r].value;\n>> 755 if (value == ATTR__UNKNOWN)\n>> 756 value = ATTR__UNSET;\n>> 757 check[i].value = value;\n>> 758 }\n>\n> I get a different crash on Linux if I set PATH_MAX to 260. The following\n> hackish patch prevents it.\n\nInteresting. It does not seem to help here. Multiple issues in the\nsame code-path?\n\n> Does it help in your case as well? If it does\n> then I'll send a nicer (but longer) one.\n\nSounds like something that's still worth investigating, even if it\ndoes not help.\n\n-- \n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n\n--- \nYou received this message because you are subscribed to the Google Groups \"msysGit\" group.\nTo unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.\nFor more options, visit https://groups.google.com/groups/opt_out.\n"},{"id":"228511","messageId":"524ACFAE.4040701@gmail.com","threadId":"35042","inReplyTo":"5249AE2A.3050302@web.de","subject":"Re: [PATCH] mingw-multibyte: fix memory acces violation and path length limits.","fromName":"Wataru Noguchi","fromEmail":"wnoguchi.0727@gmail.com","sentAt":"2013-10-01T13:35:42Z","receivedAt":"2013-10-01T13:35:42Z","isPatch":true,"sender":{"key":"wnoguchi.0727@gmail.com","avatar":"https://gravatar.com/avatar/92adeb8d30332b0a1204df49b55442bf91ea38b678bb7406c7bf66f72f51b569?d=mp&s=160"},"body":"Hi,\n\nThanks for your patch.\n\nUnfortunately, in my case still crash...\n\nBut PATH_MAX length kinds issues interesting.\n\nI'll try investigate a little more.\n\n- PATH_MAX and O2\n\nThanks.\n\n(2013/10/01 2:00), René Scharfe wrote:\n> Am 29.09.2013 04:56, schrieb Wataru Noguchi:\n>> Hi,\n>>\n>> Thanks for comments.\n>>\n>> My currently working repository is\n>>\n>> https://github.com/wnoguchi/git/tree/hotfix/mingw-multibyte-path-checkout-failure\n>>\n>> I have revert commits to 1f10da3.\n>> I'll try failure step.\n>>\n>> - gcc optimization level is O2.(fail)\n>> - gcc O0, O1 works fine.\n>>\n>>\n>> $ gdb git-clone\n>> GNU gdb 6.8\n>> Copyright (C) 2008 Free Software Foundation, Inc.\n>> License GPLv3+: GNU GPL version 3 or later \n>> This is free software: you are free to change and redistribute it.\n>> There is NO WARRANTY, to the extent permitted by law. Type \"show copying\"\n>> and \"show warranty\" for details.\n>> This GDB was configured as \"i686-pc-mingw32\"...\n>> (gdb) r https://github.com/wnoguchi/mingw-checkout-crash.git\n>> Starting program: C:\\msysgit\\git/git-clone.exe https://github.com/wnoguchi/mingw\n>> -checkout-crash.git\n>> [New thread 800.0xa10]\n>> Error: dll starting at 0x779f0000 not found.\n>> Error: dll starting at 0x75900000 not found.\n>> Error: dll starting at 0x779f0000 not found.\n>> Error: dll starting at 0x778f0000 not found.\n>> [New thread 800.0x92c]\n>> Cloning into 'mingw-checkout-crash'...\n>> Error: dll starting at 0x29f0000 not found.\n>> remote: Counting objects: 8, done.\n>> remote: Compressing objects: 100% (7/7), done.\n>> remote: Total 8 (delta 0), reused 8 (delta 0)\n>> Unpacking objects: 100% (8/8), done.\n>> Checking connectivity... done\n>> [New thread 800.0xea0]\n>>\n>> Program received signal SIGSEGV, Segmentation fault.\n>> 0x004d5200 in git_check_attr (\n>> path=0xacc6a0 \"\"..., num=5, check=0x572440) at attr.c:754\n>> 754 const char *value = check_all_attr[check[i].attr->attr_n\n>> r].value;\n>> (gdb) list\n>> 749 int i;\n>> 750\n>> 751 collect_all_attrs(path);\n>> 752\n>> 753 for (i = 0; i < num; i++) {\n>> 754 const char *value = check_all_attr[check[i].attr->attr_n\n>> r].value;\n>> 755 if (value == ATTR__UNKNOWN)\n>> 756 value = ATTR__UNSET;\n>> 757 check[i].value = value;\n>> 758 }\n>\n> I get a different crash on Linux if I set PATH_MAX to 260. The following\n> hackish patch prevents it. Does it help in your case as well? If it does\n> then I'll send a nicer (but longer) one.\n>\n> Thanks,\n> René\n>\n>\n> diff --git a/unpack-trees.c b/unpack-trees.c\n> index 1a61e6f..9bd7dcb 100644\n> --- a/unpack-trees.c\n> +++ b/unpack-trees.c\n> @@ -961,7 +961,7 @@ static int clear_ce_flags(struct cache_entry **cache, int nr,\n> \t\t\t int select_mask, int clear_mask,\n> \t\t\t struct exclude_list *el)\n> {\n> -\tchar prefix[PATH_MAX];\n> +\tchar prefix[4096];\n> \treturn clear_ce_flags_1(cache, nr,\n> \t\t\t\tprefix, 0,\n> \t\t\t\tselect_mask, clear_mask,\n>\n>\n\n-- \n=========================================\n Wataru Noguchi\n wnoguchi.0727@gmail.com\n http://wnoguchi.github.io/\n=========================================\n\n-- \n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n\n--- \nYou received this message because you are subscribed to the Google Groups \"msysGit\" group.\nTo unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.\nFor more options, visit https://groups.google.com/groups/opt_out.\n"},{"id":"228512","messageId":"524AD023.4040804@gmail.com","threadId":"35042","inReplyTo":"5248088F.2060902@googlemail.com","subject":"Re: [PATCH] mingw-multibyte: fix memory acces violation and path length limits.","fromName":"Wataru Noguchi","fromEmail":"wnoguchi.0727@gmail.com","sentAt":"2013-10-01T13:37:39Z","receivedAt":"2013-10-01T13:37:39Z","isPatch":true,"sender":{"key":"wnoguchi.0727@gmail.com","avatar":"https://gravatar.com/avatar/92adeb8d30332b0a1204df49b55442bf91ea38b678bb7406c7bf66f72f51b569?d=mp&s=160"},"body":"Hi,\n\nThanks for your advice.\n\nI see. I'll try following tool for optimization affection.\n\nThanks.\n\n(2013/09/29 20:01), Stefan Beller wrote:\n> On 09/29/2013 04:56 AM, Wataru Noguchi wrote:\n>>\n>> - gcc optimization level is O2.(fail)\n>> - gcc O0, O1 works fine.\n>\n> Maybe you could try to compile with\n> STACK found at http://css.csail.mit.edu/stack/\n> That tool is designed to find\n> Optimization-unstable code.\n>\n>\n>\n\n-- \n=========================================\n Wataru Noguchi\n wnoguchi.0727@gmail.com\n http://wnoguchi.github.io/\n=========================================\n\n-- \n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n\n--- \nYou received this message because you are subscribed to the Google Groups \"msysGit\" group.\nTo unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.\nFor more options, visit https://groups.google.com/groups/opt_out.\n"},{"id":"228526","messageId":"524C9D8F.2090107@gmail.com","threadId":"35042","inReplyTo":"524ACFAE.4040701@gmail.com","subject":"Re: [PATCH] mingw-multibyte: fix memory acces violation and path length limits.","fromName":"Wataru Noguchi","fromEmail":"wnoguchi.0727@gmail.com","sentAt":"2013-10-02T22:26:23Z","receivedAt":"2013-10-02T22:26:23Z","isPatch":true,"sender":{"key":"wnoguchi.0727@gmail.com","avatar":"https://gravatar.com/avatar/92adeb8d30332b0a1204df49b55442bf91ea38b678bb7406c7bf66f72f51b569?d=mp&s=160"},"body":"Hi,\n\nAt last, I foundfollowing Makefile optimization suppression works fine in my case.\n\nCFLAGS = -g -O2 -fno-inline-small-functions -Wall\n\nFollowing optimization option cause crash,\n\n-finline-small-functions\n\n\n// entry.c:237\n\nint checkout_entry(struct cache_entry *ce,\n\t\t const struct checkout *state, char *topath)\n{\n//------------------------------------------------------------------\n// crash!\n\tstatic char path[PATH_MAX + 1];\n//------------------------------------------------------------------\n// works fine. but bad practice.\n\tstatic char path[4096 + 1];\n//------------------------------------------------------------------\n\tstruct stat st;\n\tint len = state->base_dir_len;\n\n\tif (topath)\n\t\treturn write_entry(ce, topath, state, 1);\n\n\nThanks.\n\n\n(2013/10/01 22:35), Wataru Noguchi wrote:\n> Hi,\n>\n> Thanks for your patch.\n>\n> Unfortunately, in my case still crash...\n>\n> But PATH_MAX length kinds issues interesting.\n>\n> I'll try investigate a little more.\n>\n> - PATH_MAX and O2\n>\n> Thanks.\n>\n> (2013/10/01 2:00), René Scharfe wrote:\n>> Am 29.09.2013 04:56, schrieb Wataru Noguchi:\n>>> Hi,\n>>>\n>>> Thanks for comments.\n>>>\n>>> My currently working repository is\n>>>\n>>> https://github.com/wnoguchi/git/tree/hotfix/mingw-multibyte-path-checkout-failure\n>>>\n>>> I have revert commits to 1f10da3.\n>>> I'll try failure step.\n>>>\n>>> - gcc optimization level is O2.(fail)\n>>> - gcc O0, O1 works fine.\n>>>\n>>>\n>>> $ gdb git-clone\n>>> GNU gdb 6.8\n>>> Copyright (C) 2008 Free Software Foundation, Inc.\n>>> License GPLv3+: GNU GPL version 3 or later \n>>> This is free software: you are free to change and redistribute it.\n>>> There is NO WARRANTY, to the extent permitted by law. Type \"show copying\"\n>>> and \"show warranty\" for details.\n>>> This GDB was configured as \"i686-pc-mingw32\"...\n>>> (gdb) r https://github.com/wnoguchi/mingw-checkout-crash.git\n>>> Starting program: C:\\msysgit\\git/git-clone.exe https://github.com/wnoguchi/mingw\n>>> -checkout-crash.git\n>>> [New thread 800.0xa10]\n>>> Error: dll starting at 0x779f0000 not found.\n>>> Error: dll starting at 0x75900000 not found.\n>>> Error: dll starting at 0x779f0000 not found.\n>>> Error: dll starting at 0x778f0000 not found.\n>>> [New thread 800.0x92c]\n>>> Cloning into 'mingw-checkout-crash'...\n>>> Error: dll starting at 0x29f0000 not found.\n>>> remote: Counting objects: 8, done.\n>>> remote: Compressing objects: 100% (7/7), done.\n>>> remote: Total 8 (delta 0), reused 8 (delta 0)\n>>> Unpacking objects: 100% (8/8), done.\n>>> Checking connectivity... done\n>>> [New thread 800.0xea0]\n>>>\n>>> Program received signal SIGSEGV, Segmentation fault.\n>>> 0x004d5200 in git_check_attr (\n>>> path=0xacc6a0 \"\"..., num=5, check=0x572440) at attr.c:754\n>>> 754 const char *value = check_all_attr[check[i].attr->attr_n\n>>> r].value;\n>>> (gdb) list\n>>> 749 int i;\n>>> 750\n>>> 751 collect_all_attrs(path);\n>>> 752\n>>> 753 for (i = 0; i < num; i++) {\n>>> 754 const char *value = check_all_attr[check[i].attr->attr_n\n>>> r].value;\n>>> 755 if (value == ATTR__UNKNOWN)\n>>> 756 value = ATTR__UNSET;\n>>> 757 check[i].value = value;\n>>> 758 }\n>>\n>> I get a different crash on Linux if I set PATH_MAX to 260. The following\n>> hackish patch prevents it. Does it help in your case as well? If it does\n>> then I'll send a nicer (but longer) one.\n>>\n>> Thanks,\n>> René\n>>\n>>\n>> diff --git a/unpack-trees.c b/unpack-trees.c\n>> index 1a61e6f..9bd7dcb 100644\n>> --- a/unpack-trees.c\n>> +++ b/unpack-trees.c\n>> @@ -961,7 +961,7 @@ static int clear_ce_flags(struct cache_entry **cache, int nr,\n>> int select_mask, int clear_mask,\n>> struct exclude_list *el)\n>> {\n>> - char prefix[PATH_MAX];\n>> + char prefix[4096];\n>> return clear_ce_flags_1(cache, nr,\n>> prefix, 0,\n>> select_mask, clear_mask,\n>>\n>>\n>\n\n\n-- \n================================\n Wataru Noguchi\n wnoguchi.0727@gmail.com\n http://wnoguchi.github.io/\n================================\n\n-- \n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n\n--- \nYou received this message because you are subscribed to the Google Groups \"msysGit\" group.\nTo unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.\nFor more options, visit https://groups.google.com/groups/opt_out.\n"},{"id":"228535","messageId":"CALWbr2zDi6XjMCRimUHu2=1qrA_=3ATq+50KBa1aNoBf4X_L9g@mail.gmail.com","threadId":"35042","inReplyTo":"524C9D8F.2090107@gmail.com","subject":"Re: [PATCH] mingw-multibyte: fix memory acces violation and path length limits.","fromName":"Antoine Pelisse","fromEmail":"apelisse@gmail.com","sentAt":"2013-10-03T17:25:51Z","receivedAt":"2013-10-03T17:25:51Z","isPatch":true,"sender":{"key":"apelisse@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1929644?v=4"},"body":"I've not followed the thread so much but, in that\nentry.c::checkout_entry,() we do:\n\nmemcpy(path, state->base_dir, len);\nstrcpy(path + len, ce->name);\n\nwhich can of course result in memory violation if PATH is not long enough.\n\nOn Thu, Oct 3, 2013 at 12:26 AM, Wataru Noguchi wrote:\n> Hi,\n>\n> At last, I foundfollowing Makefile optimization suppression works fine in my\n> case.\n>\n> CFLAGS = -g -O2 -fno-inline-small-functions -Wall\n>\n> Following optimization option cause crash,\n>\n> -finline-small-functions\n"},{"id":"228536","messageId":"CABPQNSaqjKPGAQ4EKBSk+bQP2WMksc6M0YQxSkB91UrnFF28xQ@mail.gmail.com","threadId":"35042","inReplyTo":"CALWbr2zDi6XjMCRimUHu2=1qrA_=3ATq+50KBa1aNoBf4X_L9g@mail.gmail.com","subject":"Re: [PATCH] mingw-multibyte: fix memory acces violation and path length limits.","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2013-10-03T17:36:48Z","receivedAt":"2013-10-03T17:36:48Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Thu, Oct 3, 2013 at 7:25 PM, Antoine Pelisse wrote:\n> I've not followed the thread so much but, in that\n> entry.c::checkout_entry,() we do:\n>\n> memcpy(path, state->base_dir, len);\n> strcpy(path + len, ce->name);\n>\n> which can of course result in memory violation if PATH is not long enough.\n>\n\n...aaand you're spot on. The following patch illustrates it:\n\n$ /git/git-clone.exe mingw-checkout-crash.git\nCloning into 'mingw-checkout-crash'...\ndone.\nfatal: argh, this won't work!\nwarning: Clone succeeded, but checkout failed.\nYou can inspect what was checked out with 'git status'\nand retry the checkout with 'git checkout -f HEAD'\n\n---\n\ndiff --git a/entry.c b/entry.c\nindex acc892f..505638e 100644\n--- a/entry.c\n+++ b/entry.c\n@@ -244,6 +244,9 @@ int checkout_entry(struct cache_entry *ce,\n if (topath)\n return write_entry(ce, topath, state, 1);\n\n+ if (len > PATH_MAX || len + strlen(ce->name) > PATH_MAX)\n+ die(\"argh, this won't work!\");\n+\n memcpy(path, state->base_dir, len);\n strcpy(path + len, ce->name);\n len += ce_namelen(ce);\n\n\n> On Thu, Oct 3, 2013 at 12:26 AM, Wataru Noguchi wrote:\n>> Hi,\n>>\n>> At last, I foundfollowing Makefile optimization suppression works fine in my\n>> case.\n>>\n>> CFLAGS = -g -O2 -fno-inline-small-functions -Wall\n>>\n>> Following optimization option cause crash,\n>>\n>> -finline-small-functions\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at http://vger.kernel.org/majordomo-info.html\n\n-- \n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n\n--- \nYou received this message because you are subscribed to the Google Groups \"msysGit\" group.\nTo unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.\nFor more options, visit https://groups.google.com/groups/opt_out.\n"},{"id":"228569","messageId":"524FFA8E.70009@gmail.com","threadId":"35042","inReplyTo":"CABPQNSaqjKPGAQ4EKBSk+bQP2WMksc6M0YQxSkB91UrnFF28xQ@mail.gmail.com","subject":"Re: [PATCH] mingw-multibyte: fix memory acces violation and path length limits.","fromName":"Wataru Noguchi","fromEmail":"wnoguchi.0727@gmail.com","sentAt":"2013-10-05T11:39:58Z","receivedAt":"2013-10-05T11:39:58Z","isPatch":true,"sender":{"key":"wnoguchi.0727@gmail.com","avatar":"https://gravatar.com/avatar/92adeb8d30332b0a1204df49b55442bf91ea38b678bb7406c7bf66f72f51b569?d=mp&s=160"},"body":"Hi,\n\nI put following printf logs.\n\nint checkout_entry(struct cache_entry *ce,\n\t\t const struct checkout *state, char *topath)\n{\n\tstatic char path[PATH_MAX + 1];\n\tstruct stat st;\n\tint len = state->base_dir_len;\n\n\tif (topath)\n\t\treturn write_entry(ce, topath, state, 1);\n\n\tmemcpy(path, state->base_dir, len);\n\tfprintf(stderr, \"path: %s\\n\", path);\n\tfprintf(stderr, \"len: %d\\n\", len);\n\tstrcpy(path + len, ce->name);\n\tlen += ce_namelen(ce);\n\tfprintf(stderr, \"path: %s\\n\", path);\n\tfprintf(stderr, \"len: %d\\n\", len);\n\tfprintf(stderr, \"path_max: %d\\n\", PATH_MAX);\n\t\n\n--------------------------------------------------------------------------------------\n\n\ncrash result\n\nwnoguchi@WIN-72R9044R72V /usr/tmp (master)\n$ git clone https://github.com/wnoguchi/mingw-checkout-crash.git a2\nCloning into 'a2'...\nremote: Counting objects: 8, done.\nremote: Compressing objects: 100% (7/7), done.\nremote: Total 8 (delta 0), reused 8 (delta 0)\nUnpacking objects: 100% (8/8), done.\nChecking connectivity... done\npath:\nlen: 0\npath: dummy 1-long-long-long-dirname/dummy 2-long-long\n-long-dirname/dummy 3-long-long-long-dirname/dummy 4-l\nong-long-long-dirname/dummy 5-long-long-long-dirname/aaaaaaaaaaaa.txt\nlen: 302\npath_max: 259\n\ncrash!!\n\n--------------------------------------------------------------------------------------\n\nbuild with\n\nCFLAGS = -g -O2 -fno-inline-small-functions -Wall\n\n\nwnoguchi@WIN-72R9044R72V /usr/tmp (master)\n$ git clone https://github.com/wnoguchi/mingw-checkout-crash.git a3\nCloning into 'a3'...\nremote: Counting objects: 8, done.\nremote: Compressing objects: 100% (7/7), done.\nremote: Total 8 (delta 0), reused 8 (delta 0)\nUnpacking objects: 100% (8/8), done.\nChecking connectivity... done\npath:\nlen: 0\npath: dummy 1-long-long-long-dirname/dummy 2-long-long\n-long-dirname/dummy 3-long-long-long-dirname/dummy 4-l\nong-long-long-dirname/dummy 5-long-long-long-dirname/aaaaaaaaaaaa.txt\nlen: 302\npath_max: 259\n\nWarning: Your console font probably doesn't support Unicode. If you experience s\ntrange characters in the output, consider switching to a TrueType font such as L\nucida Console!\n\nworks fine.\n\n------------------------------------------------------------------------------------\n\nthis result means actual path byte length over run path buffer?\n\n\tstatic char path[PATH_MAX + 1];\n\nhmmm...\n\nI'm not sure why -fno-inline-small-functions works.\n\n\n(2013/10/04 2:36), Erik Faye-Lund wrote:\n> On Thu, Oct 3, 2013 at 7:25 PM, Antoine Pelisse wrote:\n>> I've not followed the thread so much but, in that\n>> entry.c::checkout_entry,() we do:\n>>\n>> memcpy(path, state->base_dir, len);\n>> strcpy(path + len, ce->name);\n>>\n>> which can of course result in memory violation if PATH is not long enough.\n>>\n>\n> ...aaand you're spot on. The following patch illustrates it:\n>\n> $ /git/git-clone.exe mingw-checkout-crash.git\n> Cloning into 'mingw-checkout-crash'...\n> done.\n> fatal: argh, this won't work!\n> warning: Clone succeeded, but checkout failed.\n> You can inspect what was checked out with 'git status'\n> and retry the checkout with 'git checkout -f HEAD'\n>\n> ---\n>\n> diff --git a/entry.c b/entry.c\n> index acc892f..505638e 100644\n> --- a/entry.c\n> +++ b/entry.c\n> @@ -244,6 +244,9 @@ int checkout_entry(struct cache_entry *ce,\n> if (topath)\n> return write_entry(ce, topath, state, 1);\n>\n> + if (len > PATH_MAX || len + strlen(ce->name) > PATH_MAX)\n> + die(\"argh, this won't work!\");\n> +\n> memcpy(path, state->base_dir, len);\n> strcpy(path + len, ce->name);\n> len += ce_namelen(ce);\n>\n>\n>> On Thu, Oct 3, 2013 at 12:26 AM, Wataru Noguchi wrote:\n>>> Hi,\n>>>\n>>> At last, I foundfollowing Makefile optimization suppression works fine in my\n>>> case.\n>>>\n>>> CFLAGS = -g -O2 -fno-inline-small-functions -Wall\n>>>\n>>> Following optimization option cause crash,\n>>>\n>>> -finline-small-functions\n>> --\n>> To unsubscribe from this list: send the line \"unsubscribe git\" in\n>> the body of a message to majordomo@vger.kernel.org\n>> More majordomo info at http://vger.kernel.org/majordomo-info.html\n\n\n-- \n================================\n Wataru Noguchi\n wnoguchi.0727@gmail.com\n http://wnoguchi.github.io/\n================================\n\n-- \n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n\n--- \nYou received this message because you are subscribed to the Google Groups \"msysGit\" group.\nTo unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.\nFor more options, visit https://groups.google.com/groups/opt_out.\n"},{"id":"229213","messageId":"1382179954-5169-1-git-send-email-apelisse@gmail.com","threadId":"35042","inReplyTo":"CABPQNSaqjKPGAQ4EKBSk+bQP2WMksc6M0YQxSkB91UrnFF28xQ@mail.gmail.com","subject":"[PATCH] Prevent buffer overflows when path is too big","fromName":"Antoine Pelisse","fromEmail":"apelisse@gmail.com","sentAt":"2013-10-19T10:52:34Z","receivedAt":"2013-10-19T10:52:34Z","isPatch":true,"sender":{"key":"apelisse@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1929644?v=4"},"body":"Currently, most buffers created with PATH_MAX length, are not checked\nwhen being written, and can overflow if PATH_MAX is not big enough to\nhold the path.\n\nFix that by using strlcpy() where strcpy() was used, and also run some\nextra checks when copy is done with memcpy().\n\nReported-by: Wataru Noguchi \nSigned-off-by: Antoine Pelisse \n---\n abspath.c | 10 +++++++---\n diffcore-order.c | 2 +-\n entry.c | 14 ++++++++++----\n unpack-trees.c | 2 ++\n 4 files changed, 20 insertions(+), 8 deletions(-)\n\ndiff --git a/abspath.c b/abspath.c\nindex 64adbe2..0e60ba4 100644\n--- a/abspath.c\n+++ b/abspath.c\n@@ -216,11 +216,15 @@ const char *absolute_path(const char *path)\n const char *prefix_filename(const char *pfx, int pfx_len, const char *arg)\n {\n \tstatic char path[PATH_MAX];\n+\n+\tif (pfx_len > PATH_MAX)\n+\t\tdie(\"Too long prefix path: %s\", pfx);\n+\n #ifndef GIT_WINDOWS_NATIVE\n \tif (!pfx_len || is_absolute_path(arg))\n \t\treturn arg;\n \tmemcpy(path, pfx, pfx_len);\n-\tstrcpy(path + pfx_len, arg);\n+\tstrlcpy(path + pfx_len, arg, PATH_MAX - pfx_len);\n #else\n \tchar *p;\n \t/* don't add prefix to absolute paths, but still replace '\\' by '/' */\n@@ -228,8 +232,8 @@ const char *prefix_filename(const char *pfx, int pfx_len, const char *arg)\n \t\tpfx_len = 0;\n \telse if (pfx_len)\n \t\tmemcpy(path, pfx, pfx_len);\n-\tstrcpy(path + pfx_len, arg);\n-\tfor (p = path + pfx_len; *p; p++)\n+\tstrlcpy(path + pfx_len, arg, PATH_MAX - pfx_len);\n+\tfor (p = path + pfx_len; p < path + PATH_MAX && *p; p++)\n \t\tif (*p == '\\\\')\n \t\t\t*p = '/';\n #endif\ndiff --git a/diffcore-order.c b/diffcore-order.c\nindex 23e9385..f083c82 100644\n--- a/diffcore-order.c\n+++ b/diffcore-order.c\n@@ -76,7 +76,7 @@ static int match_order(const char *path)\n \tchar p[PATH_MAX];\n \n \tfor (i = 0; i < order_cnt; i++) {\n-\t\tstrcpy(p, path);\n+\t\tstrlcpy(p, path, PATH_MAX);\n \t\twhile (p[0]) {\n \t\t\tchar *cp;\n \t\t\tif (!fnmatch(order[i], p, 0))\ndiff --git a/entry.c b/entry.c\nindex acc892f..39bee42 100644\n--- a/entry.c\n+++ b/entry.c\n@@ -50,17 +50,20 @@ static void remove_subtree(const char *path)\n \tstruct dirent *de;\n \tchar pathbuf[PATH_MAX];\n \tchar *name;\n+\tsize_t pathlen;\n \n \tif (!dir)\n \t\tdie_errno(\"cannot opendir '%s'\", path);\n-\tstrcpy(pathbuf, path);\n-\tname = pathbuf + strlen(path);\n+\tstrlcpy(pathbuf, path, PATH_MAX);\n+\tpathlen = strlen(path);\n+\tname = pathbuf + pathlen;\n \t*name++ = '/';\n+\tpathlen++;\n \twhile ((de = readdir(dir)) != NULL) {\n \t\tstruct stat st;\n \t\tif (is_dot_or_dotdot(de->d_name))\n \t\t\tcontinue;\n-\t\tstrcpy(name, de->d_name);\n+\t\tstrlcpy(name, de->d_name, PATH_MAX - pathlen);\n \t\tif (lstat(pathbuf, &st))\n \t\t\tdie_errno(\"cannot lstat '%s'\", pathbuf);\n \t\tif (S_ISDIR(st.st_mode))\n@@ -244,8 +247,11 @@ int checkout_entry(struct cache_entry *ce,\n \tif (topath)\n \t\treturn write_entry(ce, topath, state, 1);\n \n+\tif (len > PATH_MAX + 1)\n+\t\tdie(\"Too long path: %s\", state->base_dir);\n+\n \tmemcpy(path, state->base_dir, len);\n-\tstrcpy(path + len, ce->name);\n+\tstrlcpy(path + len, ce->name, PATH_MAX + 1 - len);\n \tlen += ce_namelen(ce);\n \n \tif (!check_path(path, len, &st, state->base_dir_len)) {\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex 1a61e6f..85473b1 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -918,6 +918,8 @@ static int clear_ce_flags_1(struct cache_entry **cache, int nr,\n \t\t\tint processed;\n \n \t\t\tlen = slash - name;\n+\t\t\tif (len + prefix_len >= PATH_MAX)\n+\t\t\t\tlen = PATH_MAX - prefix_len - 1;\n \t\t\tmemcpy(prefix + prefix_len, name, len);\n \n \t\t\t/*\n-- \n1.8.4.1.507.g9768648.dirty\n\n-- \n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n\n--- \nYou received this message because you are subscribed to the Google Groups \"msysGit\" group.\nTo unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.\nFor more options, visit https://groups.google.com/groups/opt_out.\n"},{"id":"229231","messageId":"52636E5A.1080909@web.de","threadId":"35042","inReplyTo":"1382179954-5169-1-git-send-email-apelisse@gmail.com","subject":"Re: [PATCH] Prevent buffer overflows when path is too big","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2013-10-20T05:47:06Z","receivedAt":"2013-10-20T05:47:06Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"(may be s/path is too big/path is too long/ ?)\n\nOn 19.10.13 12:52, Antoine Pelisse wrote:\n> Currently, most buffers created with PATH_MAX length, are not checked\n> when being written, and can overflow if PATH_MAX is not big enough to\n> hold the path.\n> \n> Fix that by using strlcpy() where strcpy() was used, and also run some\n> extra checks when copy is done with memcpy().\n> \n> Reported-by: Wataru Noguchi \n> Signed-off-by: Antoine Pelisse \n> ---\n> abspath.c | 10 +++++++---\n> diffcore-order.c | 2 +-\n> entry.c | 14 ++++++++++----\n> unpack-trees.c | 2 ++\n> 4 files changed, 20 insertions(+), 8 deletions(-)\n> \n> diff --git a/abspath.c b/abspath.c\n> index 64adbe2..0e60ba4 100644\n> --- a/abspath.c\n> +++ b/abspath.c\n> @@ -216,11 +216,15 @@ const char *absolute_path(const char *path)\n> const char *prefix_filename(const char *pfx, int pfx_len, const char *arg)\n> {\n> \tstatic char path[PATH_MAX];\n> +\n> +\tif (pfx_len > PATH_MAX)\nI think this should be \nif (pfx_len > PATH_MAX-1) /* Keep 1 char for '\\0'\n> +\t\tdie(\"Too long prefix path: %s\", pfx);\n> +\n> #ifndef GIT_WINDOWS_NATIVE\n> \tif (!pfx_len || is_absolute_path(arg))\n> \t\treturn arg;\n> \tmemcpy(path, pfx, pfx_len);\n> -\tstrcpy(path + pfx_len, arg);\n> +\tstrlcpy(path + pfx_len, arg, PATH_MAX - pfx_len);\n\nI'm not sure how to handle overlong path in general, there are several ways:\na) Silently overwrite memory (with help of memcpy() and/or strcpy()\nb) Silently shorten the path using strlcpy() instead of strcpy()\nc) Avoid the overwriting and call die().\nd) Prepare a longer buffer using xmalloc()\n\nToday we do a), this is not a good thing and the worst choice.\n\n\nA little side note:\n It would be good to have test cases for either b), c) or d).\n\n As PATH_MAX is OS dependend, we need both a main program written in c\n and a test case written in t/txxxx.sh.\n Some existing code can be used for inspiration, e.g. \n test-wildmatch.c in combination with t/t3070-wildmatch.sh\n This willl allow us to reproduce the error, and define how git should behave.\n\nEnd of the side note, let's look closer at the suggested patch, implementing b)\n\nSilently shortening an overlong path like\n\"/foo/bar/baz\" could result something like\n\n\"/foo/bar/ba\" /* That filename may be part of the repo too */\nor\n\"/foo/bar/\" /* This is a directory, not a file name */\n\nIn either case the end user has no idea why git choose another file name.\nAnd this could be hard to debug.\nAfter a couple of hours she/he may send a message asking for help to the mailing list,\nand we end up in more people doing debugging.\n\nc) Is much easier to debug:\n Git can not handle this situation, and we print out the parameters in die()\n\nI would prefer c) over b), make clear that git can't handle that situation.\n\nd) Would mean some more re-factoring: Check all callers to prefix_filename().\nSome of them call xstrdup() after prefix_filename(), which mean that we could\nchange prefix_filename() to always return new string which is long enough via xmalloc(),\nand not a static buffer.\n\nSo we come to the next point (and this is my personal experience,\nso please don't get me wrong):\nhow much time can you spend on this?\n\nIf the answer is kind of \"very little\", I would go for c)\n Avoid the silent memory corruption, and say to the user \"we can not handle this\"\n\nIf the answer is kind of \"little\", I would go for c) and a test program,\n covering all the different code path in abspath()\n (WHich may deserve a refactoring as well, since the code for GIT_WINDOWS_NATIVE\n is very similar to the non-GIT_WINDOWS_NATIVE)\n\nIf the answer is kind of \"more than little\", a different strategie may be better:\n Start sending a patch for c)\n I think we have enough volunteers here for a review, so we can life without the test code.\n On top of that, some volunteer can develop d).\n \nSo far I have only looked at abspath(), and your patch touches more places.\nI think more and more that calling die()\nwith all information included why we call die() is a good starting point.\n\nIt will allow the users to see what is going on.\nMay be the repo can be re-arranged to use shorter path names than what we can handle.\n[snip]\n \n/Torsten\n\n-- \n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n\n--- \nYou received this message because you are subscribed to the Google Groups \"msysGit\" group.\nTo unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.\nFor more options, visit https://groups.google.com/groups/opt_out.\n"},{"id":"229232","messageId":"20131020060517.GA8436@domone.podge","threadId":"35042","inReplyTo":"52636E5A.1080909@web.de","subject":"Re: [msysGit] [PATCH] Prevent buffer overflows when path is too big","fromName":"Ondřej Bílka","fromEmail":"neleai@seznam.cz","sentAt":"2013-10-20T06:05:17Z","receivedAt":"2013-10-20T06:05:17Z","isPatch":true,"sender":{"key":"neleai@seznam.cz","avatar":"https://avatars.githubusercontent.com/u/48067?v=4"},"body":"On Sun, Oct 20, 2013 at 07:47:06AM +0200, Torsten Bögershausen wrote:\n> (may be s/path is too big/path is too long/ ?)\n> \n> On 19.10.13 12:52, Antoine Pelisse wrote:\n> > Currently, most buffers created with PATH_MAX length, are not checked\n> > when being written, and can overflow if PATH_MAX is not big enough to\n> > hold the path.\n> > \n> > Fix that by using strlcpy() where strcpy() was used, and also run some\n> > extra checks when copy is done with memcpy().\n> > \n> > Reported-by: Wataru Noguchi \n> > Signed-off-by: Antoine Pelisse \n> > ---\n> > diff --git a/abspath.c b/abspath.c\n> > index 64adbe2..0e60ba4 100644\n> > --- a/abspath.c\n> > +++ b/abspath.c\n> > @@ -216,11 +216,15 @@ const char *absolute_path(const char *path)\n> > const char *prefix_filename(const char *pfx, int pfx_len, const char *arg)\n> > {\n> > \tstatic char path[PATH_MAX];\n\nWhy do you need static there?\n> > +\n> > +\tif (pfx_len > PATH_MAX)\n> I think this should be \n> if (pfx_len > PATH_MAX-1) /* Keep 1 char for '\\0'\n> > +\t\tdie(\"Too long prefix path: %s\", pfx);\n> > +\n> > #ifndef GIT_WINDOWS_NATIVE\n> > \tif (!pfx_len || is_absolute_path(arg))\n> > \t\treturn arg;\n> > \tmemcpy(path, pfx, pfx_len);\n> > -\tstrcpy(path + pfx_len, arg);\n> > +\tstrlcpy(path + pfx_len, arg, PATH_MAX - pfx_len);\n> \n> I'm not sure how to handle overlong path in general, there are several ways:\n> a) Silently overwrite memory (with help of memcpy() and/or strcpy()\n> b) Silently shorten the path using strlcpy() instead of strcpy()\n> c) Avoid the overwriting and call die().\n> d) Prepare a longer buffer using xmalloc()\n> \nThere is also\ne) modify allocation to place write protected page after buffer end.\n"},{"id":"229233","messageId":"526377C1.8020907@web.de","threadId":"35042","inReplyTo":"20131020060517.GA8436@domone.podge","subject":"Re: [PATCH] Prevent buffer overflows when path is too big","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2013-10-20T06:27:13Z","receivedAt":"2013-10-20T06:27:13Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 20.10.13 08:05, Ondřej Bílka wrote:\n> On Sun, Oct 20, 2013 at 07:47:06AM +0200, Torsten Bögershausen wrote:\n>> (may be s/path is too big/path is too long/ ?)\n>>\n>> On 19.10.13 12:52, Antoine Pelisse wrote:\n>>> Currently, most buffers created with PATH_MAX length, are not checked\n>>> when being written, and can overflow if PATH_MAX is not big enough to\n>>> hold the path.\n>>>\n>>> Fix that by using strlcpy() where strcpy() was used, and also run some\n>>> extra checks when copy is done with memcpy().\n>>>\n>>> Reported-by: Wataru Noguchi \n>>> Signed-off-by: Antoine Pelisse \n>>> ---\n>>> diff --git a/abspath.c b/abspath.c\n>>> index 64adbe2..0e60ba4 100644\n>>> --- a/abspath.c\n>>> +++ b/abspath.c\n>>> @@ -216,11 +216,15 @@ const char *absolute_path(const char *path)\n>>> const char *prefix_filename(const char *pfx, int pfx_len, const char *arg)\n>>> {\n>>> \tstatic char path[PATH_MAX];\n> \n> Why do you need static there?\nGood point.\nget_pathname() from path.c may be better.\n\n>>> +\n>>> +\tif (pfx_len > PATH_MAX)\n>> I think this should be \n>> if (pfx_len > PATH_MAX-1) /* Keep 1 char for '\\0'\n>>> +\t\tdie(\"Too long prefix path: %s\", pfx);\n>>> +\n>>> #ifndef GIT_WINDOWS_NATIVE\n>>> \tif (!pfx_len || is_absolute_path(arg))\n>>> \t\treturn arg;\n>>> \tmemcpy(path, pfx, pfx_len);\n>>> -\tstrcpy(path + pfx_len, arg);\n>>> +\tstrlcpy(path + pfx_len, arg, PATH_MAX - pfx_len);\n>>\n>> I'm not sure how to handle overlong path in general, there are several ways:\n>> a) Silently overwrite memory (with help of memcpy() and/or strcpy()\n>> b) Silently shorten the path using strlcpy() instead of strcpy()\n>> c) Avoid the overwriting and call die().\n>> d) Prepare a longer buffer using xmalloc()\n>>\n> There is also\n> e) modify allocation to place write protected page after buffer end.\n\nYes, I think this is what electric fence, DUMA or valgrind do:\n\nhttp://sourceforge.jp/projects/freshmeat_efence/\nhttp://duma.sourceforge.net/\nhttp://valgrind.sourceforge.net/\n\nTheses are very good tools for developers, finding memory corruption\n(or other bugs like using uninitialized memory).\n\nOne of the motivation I asked for test cases is that a git developer can\nrun these test cases under valgrind and can verify that we are never out of range.\n\nFor an end user a git \"crash\" caused by trying to write to a write protected page\nis better than silently corrupting memory.\n\nAnd a range check, followed by die(), is even easier to debug.\nFor an end user.\n/Torsten\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n-- \n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n\n--- \nYou received this message because you are subscribed to the Google Groups \"msysGit\" group.\nTo unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.\nFor more options, visit https://groups.google.com/groups/opt_out.\n"},{"id":"229234","messageId":"20131020073905.GA10718@domone.podge","threadId":"35042","inReplyTo":"526377C1.8020907@web.de","subject":"Re: [msysGit] [PATCH] Prevent buffer overflows when path is too big","fromName":"Ondřej Bílka","fromEmail":"neleai@seznam.cz","sentAt":"2013-10-20T07:39:05Z","receivedAt":"2013-10-20T07:39:05Z","isPatch":true,"sender":{"key":"neleai@seznam.cz","avatar":"https://avatars.githubusercontent.com/u/48067?v=4"},"body":"On Sun, Oct 20, 2013 at 08:27:13AM +0200, Torsten Bögershausen wrote:\n> Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on\n> \tpopelka.ms.mff.cuni.cz\n> Status: O\n> Content-Length: 2690\n> Lines: 89\n> \n> On 20.10.13 08:05, Ondřej Bílka wrote:\n> > On Sun, Oct 20, 2013 at 07:47:06AM +0200, Torsten Bögershausen wrote:\n> >> (may be s/path is too big/path is too long/ ?)\n> >>\n> >> On 19.10.13 12:52, Antoine Pelisse wrote:\n> >>> Currently, most buffers created with PATH_MAX length, are not checked\n> >>> when being written, and can overflow if PATH_MAX is not big enough to\n> >>> hold the path.\n> >>>\n> >>> Fix that by using strlcpy() where strcpy() was used, and also run some\n> >>> extra checks when copy is done with memcpy().\n> >>>\n> >>> Reported-by: Wataru Noguchi \n> >>> Signed-off-by: Antoine Pelisse \n> >>> ---\n> >>> diff --git a/abspath.c b/abspath.c\n> >>> index 64adbe2..0e60ba4 100644\n> >>> --- a/abspath.c\n> >>> +++ b/abspath.c\n> >>> @@ -216,11 +216,15 @@ const char *absolute_path(const char *path)\n> >>> const char *prefix_filename(const char *pfx, int pfx_len, const char *arg)\n> >>> {\n> >>> \tstatic char path[PATH_MAX];\n> > \n> > Why do you need static there?\n> Good point.\n> get_pathname() from path.c may be better.\n> \n> >>> +\n> >>> +\tif (pfx_len > PATH_MAX)\n> >> I think this should be \n> >> if (pfx_len > PATH_MAX-1) /* Keep 1 char for '\\0'\n> >>> +\t\tdie(\"Too long prefix path: %s\", pfx);\n> >>> +\n> >>> #ifndef GIT_WINDOWS_NATIVE\n> >>> \tif (!pfx_len || is_absolute_path(arg))\n> >>> \t\treturn arg;\n> >>> \tmemcpy(path, pfx, pfx_len);\n> >>> -\tstrcpy(path + pfx_len, arg);\n> >>> +\tstrlcpy(path + pfx_len, arg, PATH_MAX - pfx_len);\n> >>\n> >> I'm not sure how to handle overlong path in general, there are several ways:\n> >> a) Silently overwrite memory (with help of memcpy() and/or strcpy()\n> >> b) Silently shorten the path using strlcpy() instead of strcpy()\n> >> c) Avoid the overwriting and call die().\n> >> d) Prepare a longer buffer using xmalloc()\n> >>\n> > There is also\n> > e) modify allocation to place write protected page after buffer end.\n> \n> Yes, I think this is what electric fence, DUMA or valgrind do:\n> \nYou need to be selective which buffers are important.\n\n> http://sourceforge.jp/projects/freshmeat_efence/\n> http://duma.sourceforge.net/\n\nThese are toys, this comes with fact that they need a 8kb of space for\neach 8byte malloc. Just run a git diff and differences are huge.\n\n$ /usr/bin/time git diff HEAD^ > x\n0.06user 0.01system 0:00.07elapsed 97%CPU (0avgtext+0avgdata\n19172maxresident)k\n0inputs+8outputs (0major+5591minor)pagefaults 0swaps\n$ LD_PRELOAD=libefence.so /usr/bin/time git diff HEAD^ > x\n\n Electric Fence 2.2 Copyright (C) 1987-1999 Bruce Perens\n\n3.49user 0.94system 0:04.45elapsed 99%CPU (0avgtext+0avgdata\n91920maxresident)k\n0inputs+8outputs (0major+118069minor)pagefaults 0swaps\n\n> http://valgrind.sourceforge.net/\n> \n> Theses are very good tools for developers, finding memory corruption\n> (or other bugs like using uninitialized memory).\n> \n> One of the motivation I asked for test cases is that a git developer can\n> run these test cases under valgrind and can verify that we are never out of range.\n> \n> For an end user a git \"crash\" caused by trying to write to a write protected page\n> is better than silently corrupting memory.\n> \n> And a range check, followed by die(), is even easier to debug.\n> For an end user.\n> /Torsten\n> \n"},{"id":"229236","messageId":"CACsJy8AXV=KJtTWxp6dpfa_Pr81h3YwW5EK=c_dV=F7tr7ChWQ@mail.gmail.com","threadId":"35042","inReplyTo":"52636E5A.1080909@web.de","subject":"Re: [PATCH] Prevent buffer overflows when path is too big","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2013-10-20T10:33:47Z","receivedAt":"2013-10-20T10:33:47Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sun, Oct 20, 2013 at 12:47 PM, Torsten Bögershausen wrote:\n> I'm not sure how to handle overlong path in general, there are several ways:\n> a) Silently overwrite memory (with help of memcpy() and/or strcpy()\n> b) Silently shorten the path using strlcpy() instead of strcpy()\n> c) Avoid the overwriting and call die().\n> d) Prepare a longer buffer using xmalloc()\n\nd+) Use strbuf\n-- \nDuy\n\n-- \n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n\n--- \nYou received this message because you are subscribed to the Google Groups \"msysGit\" group.\nTo unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.\nFor more options, visit https://groups.google.com/groups/opt_out.\n"},{"id":"229238","messageId":"CALWbr2z1RmLLNa3Cj+n6g=zu5FB4VZSmRTU1qYb86pXLfYGJGg@mail.gmail.com","threadId":"35042","inReplyTo":"CACsJy8AXV=KJtTWxp6dpfa_Pr81h3YwW5EK=c_dV=F7tr7ChWQ@mail.gmail.com","subject":"Re: [PATCH] Prevent buffer overflows when path is too big","fromName":"Antoine Pelisse","fromEmail":"apelisse@gmail.com","sentAt":"2013-10-20T17:57:28Z","receivedAt":"2013-10-20T17:57:28Z","isPatch":true,"sender":{"key":"apelisse@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1929644?v=4"},"body":"My main motive was to not *stop* the process when a long path is met.\nBecause somebody created a repository on Linux with a long file-name\ndoesn't mean you should not be able to clone it *at all* on Windows.\n\nOn Sun, Oct 20, 2013 at 12:33 PM, Duy Nguyen wrote:\n> On Sun, Oct 20, 2013 at 12:47 PM, Torsten Bögershausen wrote:\n>> I'm not sure how to handle overlong path in general, there are several ways:\n>> a) Silently overwrite memory (with help of memcpy() and/or strcpy()\n\nThis one stop the process, as the application crashes :-)\n\n>> b) Silently shorten the path using strlcpy() instead of strcpy()\n\nI was expecting this solution to fail later in a non-blocking way\n(e.g. \"Can't checkout file $truncated_path, continuing with other\nfiles\"). Maybe it would be better to look at each specific call site\nand see if there is a way to report a problem ('return error(\"Can't\ncheckout %s: path too long\")')\n\n>> c) Avoid the overwriting and call die().\n\nThis one also stops the process, with an error (of course that's\nbetter than point a)\n\n>> d) Prepare a longer buffer using xmalloc()\n> d+) Use strbuf\n\nThis of course looks like the best solution, but I believe PATH_MAX\nexists for a reason, and maybe we can't simply ignore that value ?\n\n-- \n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n\n--- \nYou received this message because you are subscribed to the Google Groups \"msysGit\" group.\nTo unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.\nFor more options, visit https://groups.google.com/groups/opt_out.\n"},{"id":"229241","messageId":"CACsJy8BQ=qzXbMT9OSeQ+aDjLe5ogkUMZzdUhK0ObJP+VHkYvQ@mail.gmail.com","threadId":"35042","inReplyTo":"CALWbr2z1RmLLNa3Cj+n6g=zu5FB4VZSmRTU1qYb86pXLfYGJGg@mail.gmail.com","subject":"Re: [PATCH] Prevent buffer overflows when path is too big","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2013-10-21T01:31:09Z","receivedAt":"2013-10-21T01:31:09Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Mon, Oct 21, 2013 at 12:57 AM, Antoine Pelisse wrote:\n> My main motive was to not *stop* the process when a long path is met.\n> Because somebody created a repository on Linux with a long file-name\n> doesn't mean you should not be able to clone it *at all* on Windows.\n\nThat should be handled at the Windows compatibility layer if Windows\ncannot handle long paths as Linux (or maybe at higher level to skip\nchecking out those paths). For PATH_MAX value, see [1].\n\n[1] http://thread.gmane.org/gmane.comp.version-control.git/169012/focus=169310\n-- \nDuy\n\n-- \n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n\n--- \nYou received this message because you are subscribed to the Google Groups \"msysGit\" group.\nTo unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.\nFor more options, visit https://groups.google.com/groups/opt_out.\n"},{"id":"229260","messageId":"52657A3D.8090609@kdbg.org","threadId":"35042","inReplyTo":"CACsJy8BQ=qzXbMT9OSeQ+aDjLe5ogkUMZzdUhK0ObJP+VHkYvQ@mail.gmail.com","subject":"Re: [PATCH] Prevent buffer overflows when path is too big","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2013-10-21T19:02:21Z","receivedAt":"2013-10-21T19:02:21Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 21.10.2013 03:31, schrieb Duy Nguyen:\n> On Mon, Oct 21, 2013 at 12:57 AM, Antoine Pelisse wrote:\n>> My main motive was to not *stop* the process when a long path is met.\n>> Because somebody created a repository on Linux with a long file-name\n>> doesn't mean you should not be able to clone it *at all* on Windows.\n> \n> That should be handled at the Windows compatibility layer if Windows\n> cannot handle long paths as Linux\n\nNaah... PATH_MAX is a silly, arbitrary limit. The Git data model does\nnot forbid paths longer than PATH_MAX, be it 4096 or 260. A generic\nsolution is needed.\n\n> (or maybe at higher level to skip\n> checking out those paths).\n\nMore like this, yeah.\n\n-- Hannes\n\n-- \n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n\n--- \nYou received this message because you are subscribed to the Google Groups \"msysGit\" group.\nTo unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.\nFor more options, visit https://groups.google.com/groups/opt_out.\n"},{"id":"229261","messageId":"CABPQNSZku9RtwKe2r=zpGrNcHRDD_Ct7C+=x8UcNhJeJDn-oqQ@mail.gmail.com","threadId":"35042","inReplyTo":"52657A3D.8090609@kdbg.org","subject":"Re: [PATCH] Prevent buffer overflows when path is too big","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2013-10-21T19:07:26Z","receivedAt":"2013-10-21T19:07:26Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Mon, Oct 21, 2013 at 9:02 PM, Johannes Sixt wrote:\n> Am 21.10.2013 03:31, schrieb Duy Nguyen:\n>> On Mon, Oct 21, 2013 at 12:57 AM, Antoine Pelisse wrote:\n>>> My main motive was to not *stop* the process when a long path is met.\n>>> Because somebody created a repository on Linux with a long file-name\n>>> doesn't mean you should not be able to clone it *at all* on Windows.\n>>\n>> That should be handled at the Windows compatibility layer if Windows\n>> cannot handle long paths as Linux\n>\n> Naah... PATH_MAX is a silly, arbitrary limit. The Git data model does\n> not forbid paths longer than PATH_MAX, be it 4096 or 260. A generic\n> solution is needed.\n>\n>> (or maybe at higher level to skip\n>> checking out those paths).\n>\n> More like this, yeah.\n\nI would argue that this is probably even a bug on Linux, only harder\n(if not impossible) to trigger by accident as there's probably no\ngit-client that will generate such trees. But a \"malicious\" client\nmight.\n\n-- \n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n\n--- \nYou received this message because you are subscribed to the Google Groups \"msysGit\" group.\nTo unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.\nFor more options, visit https://groups.google.com/groups/opt_out.\n"},{"id":"229262","messageId":"20131021191439.GB29681@sigill.intra.peff.net","threadId":"35042","inReplyTo":"CABPQNSZku9RtwKe2r=zpGrNcHRDD_Ct7C+=x8UcNhJeJDn-oqQ@mail.gmail.com","subject":"Re: [PATCH] Prevent buffer overflows when path is too big","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-10-21T19:14:39Z","receivedAt":"2013-10-21T19:14:39Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Oct 21, 2013 at 09:07:26PM +0200, Erik Faye-Lund wrote:\n\n> I would argue that this is probably even a bug on Linux, only harder\n> (if not impossible) to trigger by accident as there's probably no\n> git-client that will generate such trees. But a \"malicious\" client\n> might.\n\nI've just been poking through the impacts of these overflows, for that\nexact reason. I don't think any of them are easily triggerable by\nsomebody sending you a malicious tree (e.g., the `remove_subtree` one\nonly triggers when we have seen that tree in the filesystem, so it must\nbe limited to `PATH_MAX`). Some of them are triggerable if you use\nparticular options (e.g., the one in `match_order` is easy to trigger if\nyou use `diff -O`).\n\nStill, they should all be fixed, even for Linux. I shouldn't have to\ntrace the provenance of the data back through 10 functions just to find\nout a buffer overflow isn't easily exploitable. :)\n\n-Peff\n\n-- \n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n\n--- \nYou received this message because you are subscribed to the Google Groups \"msysGit\" group.\nTo unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.\nFor more options, visit https://groups.google.com/groups/opt_out.\n"},{"id":"229263","messageId":"20131021193223.GC29681@sigill.intra.peff.net","threadId":"35042","inReplyTo":"20131021191439.GB29681@sigill.intra.peff.net","subject":"Re: [PATCH] Prevent buffer overflows when path is too big","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-10-21T19:32:23Z","receivedAt":"2013-10-21T19:32:23Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Oct 21, 2013 at 03:14:39PM -0400, Jeff King wrote:\n\n> On Mon, Oct 21, 2013 at 09:07:26PM +0200, Erik Faye-Lund wrote:\n> \n> > I would argue that this is probably even a bug on Linux, only harder\n> > (if not impossible) to trigger by accident as there's probably no\n> > git-client that will generate such trees. But a \"malicious\" client\n> > might.\n> \n> I've just been poking through the impacts of these overflows, for that\n> exact reason. I don't think any of them are easily triggerable by\n> somebody sending you a malicious tree (e.g., the `remove_subtree` one\n> only triggers when we have seen that tree in the filesystem, so it must\n> be limited to `PATH_MAX`). Some of them are triggerable if you use\n> particular options (e.g., the one in `match_order` is easy to trigger if\n> you use `diff -O`).\n\nActually, I take that back. The one in checkout_entry is quite easy to\ntrigger if the victim checks out your tree. The rest are much harder,\nthough.\n\n-Peff\n\n-- \n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n\n--- \nYou received this message because you are subscribed to the Google Groups \"msysGit\" group.\nTo unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.\nFor more options, visit https://groups.google.com/groups/opt_out.\n"},{"id":"229320","messageId":"1382532907-30561-1-git-send-email-pclouds@gmail.com","threadId":"35042","inReplyTo":"20131021193223.GC29681@sigill.intra.peff.net","subject":"[PATCH 1/2] entry.c: convert checkout_entry to use strbuf","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2013-10-23T12:55:06Z","receivedAt":"2013-10-23T12:55:06Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"The old code does not do boundary check so any paths longer than\nPATH_MAX can cause buffer overflow. Replace it with strbuf to handle\npaths of arbitrary length.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n To get this topic going again. These two patches kill PATH_MAX in\n entry.c and builtin/checkout-index.c\n\n entry.c | 12 +++++++-----\n 1 file changed, 7 insertions(+), 5 deletions(-)\n\ndiff --git a/entry.c b/entry.c\nindex acc892f..d955af5 100644\n--- a/entry.c\n+++ b/entry.c\n@@ -237,16 +237,18 @@ static int check_path(const char *path, int len, struct stat *st, int skiplen)\n int checkout_entry(struct cache_entry *ce,\n \t\t const struct checkout *state, char *topath)\n {\n-\tstatic char path[PATH_MAX + 1];\n+\tstatic struct strbuf path_buf = STRBUF_INIT;\n+\tchar *path;\n \tstruct stat st;\n-\tint len = state->base_dir_len;\n+\tint len;\n \n \tif (topath)\n \t\treturn write_entry(ce, topath, state, 1);\n \n-\tmemcpy(path, state->base_dir, len);\n-\tstrcpy(path + len, ce->name);\n-\tlen += ce_namelen(ce);\n+\tstrbuf_reset(&path_buf);\n+\tstrbuf_addf(&path_buf, \"%.*s%s\", state->base_dir_len, state->base_dir, ce->name);\n+\tpath = path_buf.buf;\n+\tlen = path_buf.len;\n \n \tif (!check_path(path, len, &st, state->base_dir_len)) {\n \t\tunsigned changed = ce_match_stat(ce, &st, CE_MATCH_IGNORE_VALID|CE_MATCH_IGNORE_SKIP_WORKTREE);\n-- \n1.8.2.83.gc99314b\n\n-- \n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n\n--- \nYou received this message because you are subscribed to the Google Groups \"msysGit\" group.\nTo unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.\nFor more options, visit https://groups.google.com/groups/opt_out.\n"},{"id":"229321","messageId":"1382532907-30561-2-git-send-email-pclouds@gmail.com","threadId":"35042","inReplyTo":"1382532907-30561-1-git-send-email-pclouds@gmail.com","subject":"[PATCH 2/2] entry.c: convert write_entry to use strbuf","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2013-10-23T12:55:07Z","receivedAt":"2013-10-23T12:55:07Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"The strcpy call in open_output_fd() implies that the output buffer\nmust be at least 25 chars long. And it's true. The only caller that\ncan trigger that code is checkout-index, which has the buffer of\nPATH_MAX chars (and any systems that have PATH_MAX shorter than 25\nchars are just insane).\n\nBut in order to say that, one has to walk through a dozen of\nfunctions. Just convert it to strbuf to avoid the constraint and\nconfusion.\n\nAlthough my original motivation was simpler than that. I just wanted\nto change \"char *path\" to \"const char *path\" in checkout_entry() to\nmake sure no funny business regarding \"path\" in that function.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n builtin/checkout-index.c | 19 ++++++++++++-------\n cache.h | 2 +-\n entry.c | 29 ++++++++++++++++-------------\n 3 files changed, 29 insertions(+), 21 deletions(-)\n\ndiff --git a/builtin/checkout-index.c b/builtin/checkout-index.c\nindex 69e167b..6d88c0c 100644\n--- a/builtin/checkout-index.c\n+++ b/builtin/checkout-index.c\n@@ -14,7 +14,12 @@\n static int line_termination = '\\n';\n static int checkout_stage; /* default to checkout stage0 */\n static int to_tempfile;\n-static char topath[4][PATH_MAX + 1];\n+static struct strbuf topath[4] = {\n+\tSTRBUF_INIT,\n+\tSTRBUF_INIT,\n+\tSTRBUF_INIT,\n+\tSTRBUF_INIT\n+};\n \n static struct checkout state;\n \n@@ -26,19 +31,19 @@ static void write_tempfile_record(const char *name, int prefix_length)\n \t\tfor (i = 1; i < 4; i++) {\n \t\t\tif (i > 1)\n \t\t\t\tputchar(' ');\n-\t\t\tif (topath[i][0])\n-\t\t\t\tfputs(topath[i], stdout);\n+\t\t\tif (topath[i].len)\n+\t\t\t\tfputs(topath[i].buf, stdout);\n \t\t\telse\n \t\t\t\tputchar('.');\n \t\t}\n \t} else\n-\t\tfputs(topath[checkout_stage], stdout);\n+\t\tfputs(topath[checkout_stage].buf, stdout);\n \n \tputchar('\\t');\n \twrite_name_quoted(name + prefix_length, stdout, line_termination);\n \n \tfor (i = 0; i < 4; i++) {\n-\t\ttopath[i][0] = 0;\n+\t\tstrbuf_reset(&topath[i]);\n \t}\n }\n \n@@ -65,7 +70,7 @@ static int checkout_file(const char *name, int prefix_length)\n \t\t\tcontinue;\n \t\tdid_checkout = 1;\n \t\tif (checkout_entry(ce, &state,\n-\t\t to_tempfile ? topath[ce_stage(ce)] : NULL) < 0)\n+\t\t to_tempfile ? &topath[ce_stage(ce)] : NULL) < 0)\n \t\t\terrs++;\n \t}\n \n@@ -109,7 +114,7 @@ static void checkout_all(const char *prefix, int prefix_length)\n \t\t\t\twrite_tempfile_record(last_ce->name, prefix_length);\n \t\t}\n \t\tif (checkout_entry(ce, &state,\n-\t\t to_tempfile ? topath[ce_stage(ce)] : NULL) < 0)\n+\t\t to_tempfile ? &topath[ce_stage(ce)] : NULL) < 0)\n \t\t\terrs++;\n \t\tlast_ce = ce;\n \t}\ndiff --git a/cache.h b/cache.h\nindex 51d6602..276182f 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -962,7 +962,7 @@ struct checkout {\n \t\t refresh_cache:1;\n };\n \n-extern int checkout_entry(struct cache_entry *ce, const struct checkout *state, char *topath);\n+extern int checkout_entry(struct cache_entry *ce, const struct checkout *state, struct strbuf *topath);\n \n struct cache_def {\n \tchar path[PATH_MAX + 1];\ndiff --git a/entry.c b/entry.c\nindex d955af5..a76942d 100644\n--- a/entry.c\n+++ b/entry.c\n@@ -92,15 +92,15 @@ static void *read_blob_entry(const struct cache_entry *ce, unsigned long *size)\n \treturn NULL;\n }\n \n-static int open_output_fd(char *path, const struct cache_entry *ce, int to_tempfile)\n+static int open_output_fd(struct strbuf *path, const struct cache_entry *ce, int to_tempfile)\n {\n \tint symlink = (ce->ce_mode & S_IFMT) != S_IFREG;\n \tif (to_tempfile) {\n-\t\tstrcpy(path, symlink\n-\t\t ? \".merge_link_XXXXXX\" : \".merge_file_XXXXXX\");\n-\t\treturn mkstemp(path);\n+\t\tstrbuf_reset(path);\n+\t\tstrbuf_addstr(path, symlink ? \".merge_link_XXXXXX\" : \".merge_file_XXXXXX\");\n+\t\treturn mkstemp(path->buf);\n \t} else {\n-\t\treturn create_file(path, !symlink ? ce->ce_mode : 0666);\n+\t\treturn create_file(path->buf, !symlink ? ce->ce_mode : 0666);\n \t}\n }\n \n@@ -115,7 +115,7 @@ static int fstat_output(int fd, const struct checkout *state, struct stat *st)\n \treturn 0;\n }\n \n-static int streaming_write_entry(const struct cache_entry *ce, char *path,\n+static int streaming_write_entry(const struct cache_entry *ce, struct strbuf *path,\n \t\t\t\t struct stream_filter *filter,\n \t\t\t\t const struct checkout *state, int to_tempfile,\n \t\t\t\t int *fstat_done, struct stat *statbuf)\n@@ -132,12 +132,12 @@ static int streaming_write_entry(const struct cache_entry *ce, char *path,\n \tresult |= close(fd);\n \n \tif (result)\n-\t\tunlink(path);\n+\t\tunlink(path->buf);\n \treturn result;\n }\n \n static int write_entry(struct cache_entry *ce,\n-\t\t char *path, const struct checkout *state, int to_tempfile)\n+\t\t struct strbuf *path_buf, const struct checkout *state, int to_tempfile)\n {\n \tunsigned int ce_mode_s_ifmt = ce->ce_mode & S_IFMT;\n \tint fd, ret, fstat_done = 0;\n@@ -146,15 +146,17 @@ static int write_entry(struct cache_entry *ce,\n \tunsigned long size;\n \tsize_t wrote, newsize = 0;\n \tstruct stat st;\n+\tconst char *path;\n \n \tif (ce_mode_s_ifmt == S_IFREG) {\n \t\tstruct stream_filter *filter = get_stream_filter(ce->name, ce->sha1);\n \t\tif (filter &&\n-\t\t !streaming_write_entry(ce, path, filter,\n+\t\t !streaming_write_entry(ce, path_buf, filter,\n \t\t\t\t\t state, to_tempfile,\n \t\t\t\t\t &fstat_done, &st))\n \t\t\tgoto finish;\n \t}\n+\tpath = path_buf->buf;\n \n \tswitch (ce_mode_s_ifmt) {\n \tcase S_IFREG:\n@@ -183,7 +185,8 @@ static int write_entry(struct cache_entry *ce,\n \t\t\tsize = newsize;\n \t\t}\n \n-\t\tfd = open_output_fd(path, ce, to_tempfile);\n+\t\tfd = open_output_fd(path_buf, ce, to_tempfile);\n+\t\tpath = path_buf->buf;\n \t\tif (fd < 0) {\n \t\t\tfree(new);\n \t\t\treturn error(\"unable to create file %s (%s)\",\n@@ -235,10 +238,10 @@ static int check_path(const char *path, int len, struct stat *st, int skiplen)\n }\n \n int checkout_entry(struct cache_entry *ce,\n-\t\t const struct checkout *state, char *topath)\n+\t\t const struct checkout *state, struct strbuf *topath)\n {\n \tstatic struct strbuf path_buf = STRBUF_INIT;\n-\tchar *path;\n+\tconst char *path;\n \tstruct stat st;\n \tint len;\n \n@@ -278,5 +281,5 @@ int checkout_entry(struct cache_entry *ce,\n \t} else if (state->not_new)\n \t\treturn 0;\n \tcreate_directories(path, len, state);\n-\treturn write_entry(ce, path, state, 0);\n+\treturn write_entry(ce, &path_buf, state, 0);\n }\n-- \n1.8.2.83.gc99314b\n\n-- \n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n\n--- \nYou received this message because you are subscribed to the Google Groups \"msysGit\" group.\nTo unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.\nFor more options, visit https://groups.google.com/groups/opt_out.\n"},{"id":"229322","messageId":"CACsJy8AooiUNRfnqDLBmx=KPnztjdNuF4bYY2b=Egs3gdiW6KA@mail.gmail.com","threadId":"35042","inReplyTo":"52657A3D.8090609@kdbg.org","subject":"Re: [PATCH] Prevent buffer overflows when path is too big","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2013-10-23T12:55:09Z","receivedAt":"2013-10-23T12:55:09Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, Oct 22, 2013 at 2:02 AM, Johannes Sixt wrote:\n>> (or maybe at higher level to skip checking out those paths).\n>\n> More like this, yeah.\n\nThe good thing is we do not stop checking out if one entry fails. But\ndue to the lack of worktree entries, one may accidentally remove files\nin new commits. So setting CE_VALID on failed-to-checkout entries\nmight help. I'm not sure. But I won't pursue because Windows is not\nreally my itch.\n-- \nDuy\n\n-- \n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n\n--- \nYou received this message because you are subscribed to the Google Groups \"msysGit\" group.\nTo unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.\nFor more options, visit https://groups.google.com/groups/opt_out.\n"},{"id":"229323","messageId":"CALWbr2z90_LysnZiPaGr-X98EceX_d0yJ6_y_te16bC818xAEQ@mail.gmail.com","threadId":"35042","inReplyTo":"1382532907-30561-1-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH 1/2] entry.c: convert checkout_entry to use strbuf","fromName":"Antoine Pelisse","fromEmail":"apelisse@gmail.com","sentAt":"2013-10-23T12:58:08Z","receivedAt":"2013-10-23T12:58:08Z","isPatch":true,"sender":{"key":"apelisse@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1929644?v=4"},"body":"On Wed, Oct 23, 2013 at 2:55 PM, Nguyễn Thái Ngọc Duy wrote:\n> The old code does not do boundary check so any paths longer than\n> PATH_MAX can cause buffer overflow. Replace it with strbuf to handle\n> paths of arbitrary length.\n>\n> Signed-off-by: Nguyễn Thái Ngọc Duy \n> ---\n> To get this topic going again. These two patches kill PATH_MAX in\n> entry.c and builtin/checkout-index.c\n\nThanks !\n\n> diff --git a/entry.c b/entry.c\n> index acc892f..d955af5 100644\n> --- a/entry.c\n> +++ b/entry.c\n> @@ -237,16 +237,18 @@ static int check_path(const char *path, int len, struct stat *st, int skiplen)\n> int checkout_entry(struct cache_entry *ce,\n> const struct checkout *state, char *topath)\n> {\n> - static char path[PATH_MAX + 1];\n> + static struct strbuf path_buf = STRBUF_INIT;\n> + char *path;\n> struct stat st;\n> - int len = state->base_dir_len;\n> + int len;\n>\n> if (topath)\n> return write_entry(ce, topath, state, 1);\n>\n> - memcpy(path, state->base_dir, len);\n> - strcpy(path + len, ce->name);\n> - len += ce_namelen(ce);\n> + strbuf_reset(&path_buf);\n\nI think this is not required\n\n-- \n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n\n--- \nYou received this message because you are subscribed to the Google Groups \"msysGit\" group.\nTo unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.\nFor more options, visit https://groups.google.com/groups/opt_out.\n"},{"id":"229325","messageId":"CACsJy8Cnf6o=hLcMOT3iXP+q=b8E4pE=68whxDi364N4MNgUYQ@mail.gmail.com","threadId":"35042","inReplyTo":"CALWbr2z90_LysnZiPaGr-X98EceX_d0yJ6_y_te16bC818xAEQ@mail.gmail.com","subject":"Re: [PATCH 1/2] entry.c: convert checkout_entry to use strbuf","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2013-10-23T13:04:03Z","receivedAt":"2013-10-23T13:04:03Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, Oct 23, 2013 at 7:58 PM, Antoine Pelisse wrote:\n>> diff --git a/entry.c b/entry.c\n>> index acc892f..d955af5 100644\n>> --- a/entry.c\n>> +++ b/entry.c\n>> @@ -237,16 +237,18 @@ static int check_path(const char *path, int len, struct stat *st, int skiplen)\n>> int checkout_entry(struct cache_entry *ce,\n>> const struct checkout *state, char *topath)\n>> {\n>> - static char path[PATH_MAX + 1];\n>> + static struct strbuf path_buf = STRBUF_INIT;\n>> + char *path;\n>> struct stat st;\n>> - int len = state->base_dir_len;\n>> + int len;\n>>\n>> if (topath)\n>> return write_entry(ce, topath, state, 1);\n>>\n>> - memcpy(path, state->base_dir, len);\n>> - strcpy(path + len, ce->name);\n>> - len += ce_namelen(ce);\n>> + strbuf_reset(&path_buf);\n>\n> I think this is not required\n\nIf you mean strbuf_reset, I think it is. path_buf is still static (I\ndon't want to remove that because it'll add a lot more strbuf_release)\nso we can't be sure what it contains from the second checkout_entry()\ncall.\n-- \nDuy\n\n-- \n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n\n--- \nYou received this message because you are subscribed to the Google Groups \"msysGit\" group.\nTo unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.\nFor more options, visit https://groups.google.com/groups/opt_out.\n"},{"id":"229326","messageId":"CALWbr2wypVuPQWEiM8ci_5RK_+R9c7O48=hXs4XrUiRUp+yf0A@mail.gmail.com","threadId":"35042","inReplyTo":"CACsJy8Cnf6o=hLcMOT3iXP+q=b8E4pE=68whxDi364N4MNgUYQ@mail.gmail.com","subject":"Re: [PATCH 1/2] entry.c: convert checkout_entry to use strbuf","fromName":"Antoine Pelisse","fromEmail":"apelisse@gmail.com","sentAt":"2013-10-23T13:06:16Z","receivedAt":"2013-10-23T13:06:16Z","isPatch":true,"sender":{"key":"apelisse@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1929644?v=4"},"body":"On Wed, Oct 23, 2013 at 3:04 PM, Duy Nguyen wrote:\n> On Wed, Oct 23, 2013 at 7:58 PM, Antoine Pelisse wrote:\n>>> diff --git a/entry.c b/entry.c\n>>> index acc892f..d955af5 100644\n>>> --- a/entry.c\n>>> +++ b/entry.c\n>>> @@ -237,16 +237,18 @@ static int check_path(const char *path, int len, struct stat *st, int skiplen)\n>>> int checkout_entry(struct cache_entry *ce,\n>>> const struct checkout *state, char *topath)\n>>> {\n>>> - static char path[PATH_MAX + 1];\n>>> + static struct strbuf path_buf = STRBUF_INIT;\n>>> + char *path;\n>>> struct stat st;\n>>> - int len = state->base_dir_len;\n>>> + int len;\n>>>\n>>> if (topath)\n>>> return write_entry(ce, topath, state, 1);\n>>>\n>>> - memcpy(path, state->base_dir, len);\n>>> - strcpy(path + len, ce->name);\n>>> - len += ce_namelen(ce);\n>>> + strbuf_reset(&path_buf);\n>>\n>> I think this is not required\n>\n> If you mean strbuf_reset, I think it is. path_buf is still static (I\n> don't want to remove that because it'll add a lot more strbuf_release)\n> so we can't be sure what it contains from the second checkout_entry()\n> call.\n\nOf course, I forgot about the static,\nThanks :-)\n\n-- \n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n\n--- \nYou received this message because you are subscribed to the Google Groups \"msysGit\" group.\nTo unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.\nFor more options, visit https://groups.google.com/groups/opt_out.\n"},{"id":"229351","messageId":"20131023172914.GA6824@sigill.intra.peff.net","threadId":"35042","inReplyTo":"1382532907-30561-1-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH 1/2] entry.c: convert checkout_entry to use strbuf","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-10-23T17:29:14Z","receivedAt":"2013-10-23T17:29:14Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Oct 23, 2013 at 07:55:06PM +0700, Nguyen Thai Ngoc Duy wrote:\n\n> The old code does not do boundary check so any paths longer than\n> PATH_MAX can cause buffer overflow. Replace it with strbuf to handle\n> paths of arbitrary length.\n\nI think this is a reasonable solution. If we have such a long path, we\nare probably about to feed it to open() or another syscall, and we will\njust get ENAMETOOLONG there anyway. But certainly we need to fix the\nbuffer overflow, and we are probably better off letting the syscall\nreport failure than calling die(), because we generally handle the\nsyscall failure more gracefully (e.g., by reporting the failed path but\ncontinuing).\n\n> -\tmemcpy(path, state->base_dir, len);\n> -\tstrcpy(path + len, ce->name);\n> -\tlen += ce_namelen(ce);\n> +\tstrbuf_reset(&path_buf);\n> +\tstrbuf_addf(&path_buf, \"%.*s%s\", state->base_dir_len, state->base_dir, ce->name);\n> +\tpath = path_buf.buf;\n> +\tlen = path_buf.len;\n\nThis is not something you introduced, but while we are here, you may\nwant to use ce->namelen, which would be a little faster than treating it\nas a string (especially for strbuf, as it can then know up front how big\nthe size is).\n\nI doubt it's measurable, though (especially as the growth cost is\namortized due to the static buffer).\n\n-Peff\n\n-- \n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n\n--- \nYou received this message because you are subscribed to the Google Groups \"msysGit\" group.\nTo unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.\nFor more options, visit https://groups.google.com/groups/opt_out.\n"},{"id":"229352","messageId":"CABPQNSZgFa1Roq=aEg8CBpo320hP7bFEOq2RK8xY3fESdYLdTg@mail.gmail.com","threadId":"35042","inReplyTo":"20131023172914.GA6824@sigill.intra.peff.net","subject":"Re: [PATCH 1/2] entry.c: convert checkout_entry to use strbuf","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2013-10-23T17:34:18Z","receivedAt":"2013-10-23T17:34:18Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Wed, Oct 23, 2013 at 7:29 PM, Jeff King wrote:\n> On Wed, Oct 23, 2013 at 07:55:06PM +0700, Nguyen Thai Ngoc Duy wrote:\n>\n>> The old code does not do boundary check so any paths longer than\n>> PATH_MAX can cause buffer overflow. Replace it with strbuf to handle\n>> paths of arbitrary length.\n>\n> I think this is a reasonable solution. If we have such a long path, we\n> are probably about to feed it to open() or another syscall, and we will\n> just get ENAMETOOLONG there anyway. But certainly we need to fix the\n> buffer overflow, and we are probably better off letting the syscall\n> report failure than calling die(), because we generally handle the\n> syscall failure more gracefully (e.g., by reporting the failed path but\n> continuing).\n>\n>> - memcpy(path, state->base_dir, len);\n>> - strcpy(path + len, ce->name);\n>> - len += ce_namelen(ce);\n>> + strbuf_reset(&path_buf);\n>> + strbuf_addf(&path_buf, \"%.*s%s\", state->base_dir_len, state->base_dir, ce->name);\n>> + path = path_buf.buf;\n>> + len = path_buf.len;\n>\n> This is not something you introduced, but while we are here, you may\n> want to use ce->namelen, which would be a little faster than treating it\n> as a string (especially for strbuf, as it can then know up front how big\n> the size is).\n>\n> I doubt it's measurable, though (especially as the growth cost is\n> amortized due to the static buffer).\n\nI somehow feel that:\n\nstrbuf_reset(&path_buf);\nstrbuf_add(&path_buf, state->base_dir, state->base_dir_len);\nstrbuf_addch(&path_buf, '/');\nstrbuf_add(&path_buf, state->name, state->name_len);\n\nfeels a bit neater than using strbuf_addf. But that might just be me.\n\n-- \n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n\n--- \nYou received this message because you are subscribed to the Google Groups \"msysGit\" group.\nTo unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.\nFor more options, visit https://groups.google.com/groups/opt_out.\n"},{"id":"229353","messageId":"20131023175229.GB6824@sigill.intra.peff.net","threadId":"35042","inReplyTo":"CABPQNSZgFa1Roq=aEg8CBpo320hP7bFEOq2RK8xY3fESdYLdTg@mail.gmail.com","subject":"Re: [PATCH 1/2] entry.c: convert checkout_entry to use strbuf","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-10-23T17:52:29Z","receivedAt":"2013-10-23T17:52:29Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Oct 23, 2013 at 07:34:18PM +0200, Erik Faye-Lund wrote:\n\n> >> - memcpy(path, state->base_dir, len);\n> >> - strcpy(path + len, ce->name);\n> >> - len += ce_namelen(ce);\n> >> + strbuf_reset(&path_buf);\n> >> + strbuf_addf(&path_buf, \"%.*s%s\", state->base_dir_len, state->base_dir, ce->name);\n> >> + path = path_buf.buf;\n> >> + len = path_buf.len;\n> >\n> > This is not something you introduced, but while we are here, you may\n> > want to use ce->namelen, which would be a little faster than treating it\n> > as a string (especially for strbuf, as it can then know up front how big\n> > the size is).\n> >\n> > I doubt it's measurable, though (especially as the growth cost is\n> > amortized due to the static buffer).\n> \n> I somehow feel that:\n> \n> strbuf_reset(&path_buf);\n> strbuf_add(&path_buf, state->base_dir, state->base_dir_len);\n> strbuf_addch(&path_buf, '/');\n> strbuf_add(&path_buf, state->name, state->name_len);\n> \n> feels a bit neater than using strbuf_addf. But that might just be me.\n\nI agree. But note that your addch is a bug. :)\n\n-Peff\n\n-- \n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n\n--- \nYou received this message because you are subscribed to the Google Groups \"msysGit\" group.\nTo unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.\nFor more options, visit https://groups.google.com/groups/opt_out.\n"},{"id":"229354","messageId":"xmqqeh7bri1h.fsf@gitster.dls.corp.google.com","threadId":"35042","inReplyTo":"1382532907-30561-2-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH 2/2] entry.c: convert write_entry to use strbuf","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-10-23T17:52:42Z","receivedAt":"2013-10-23T17:52:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy writes:\n\n> The strcpy call in open_output_fd() implies that the output buffer\n> must be at least 25 chars long.\n\nHmph, where does that 25 come from?\n\n> And it's true. The only caller that\n> can trigger that code is checkout-index, which has the buffer of\n> PATH_MAX chars (and any systems that have PATH_MAX shorter than 25\n> chars are just insane).\n>\n> But in order to say that, one has to walk through a dozen of\n> functions. Just convert it to strbuf to avoid the constraint and\n> confusion.\n\nWouldn't it be far clearer to document what is going on especially\naround the topath parameter to checkout_entry(), than to introduce\nunnecessary strbuf overhead?\n\nAt first glance, it might appear that the caller of checkout_entry()\ncan specify to which path the contents are written out, but in\nreality topath[] is to point at the buffer to store the temporary\npath generated by the lower guts of write_entry(). It is unclear in\nthe original code and that is worth an in-code comment.\n\nAnd when describing that API requirement, we would need to say how\nbig a buffer the caller must allocate for topath[] in the comment.\nThat size does not have to be platform-dependent PATH_MAX.\n\nSomething like this?\n\n builtin/checkout-index.c | 2 +-\n cache.h | 1 +\n entry.c | 8 ++++++++\n 3 files changed, 10 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/checkout-index.c b/builtin/checkout-index.c\nindex b1feda7..4ed6b23 100644\n--- a/builtin/checkout-index.c\n+++ b/builtin/checkout-index.c\n@@ -14,7 +14,7 @@\n static int line_termination = '\\n';\n static int checkout_stage; /* default to checkout stage0 */\n static int to_tempfile;\n-static char topath[4][PATH_MAX + 1];\n+static char topath[4][TEMPORARY_FILENAME_LENGTH + 1];\n \n static struct checkout state;\n \ndiff --git a/cache.h b/cache.h\nindex 85b544f..3118b7f 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -975,6 +975,7 @@ struct checkout {\n \t\t refresh_cache:1;\n };\n \n+#define TEMPORARY_FILENAME_LENGTH 25\n extern int checkout_entry(struct cache_entry *ce, const struct checkout *state, char *topath);\n \n struct cache_def {\ndiff --git a/entry.c b/entry.c\nindex d955af5..2df4ee1 100644\n--- a/entry.c\n+++ b/entry.c\n@@ -234,6 +234,14 @@ static int check_path(const char *path, int len, struct stat *st, int skiplen)\n \treturn lstat(path, st);\n }\n \n+/*\n+ * Write the contents from ce out to the working tree.\n+ *\n+ * When topath[] is not NULL, instead of writing to the working tree\n+ * file named by ce, a temporary file is created by this function and\n+ * its name is returned in topath[], which must be able to hold at\n+ * least TEMPORARY_FILENAME_LENGTH bytes long.\n+ */\n int checkout_entry(struct cache_entry *ce,\n \t\t const struct checkout *state, char *topath)\n {\n\n-- \n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n\n--- \nYou received this message because you are subscribed to the Google Groups \"msysGit\" group.\nTo unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.\nFor more options, visit https://groups.google.com/groups/opt_out.\n"},{"id":"229355","messageId":"xmqqa9hzrh9k.fsf@gitster.dls.corp.google.com","threadId":"35042","inReplyTo":"20131023172914.GA6824@sigill.intra.peff.net","subject":"Re: [PATCH 1/2] entry.c: convert checkout_entry to use strbuf","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-10-23T18:09:27Z","receivedAt":"2013-10-23T18:09:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Wed, Oct 23, 2013 at 07:55:06PM +0700, Nguyen Thai Ngoc Duy wrote:\n> ...\n>> -\tmemcpy(path, state->base_dir, len);\n>> -\tstrcpy(path + len, ce->name);\n>> -\tlen += ce_namelen(ce);\n>> +\tstrbuf_reset(&path_buf);\n>> +\tstrbuf_addf(&path_buf, \"%.*s%s\", state->base_dir_len, state->base_dir, ce->name);\n>> +\tpath = path_buf.buf;\n>> +\tlen = path_buf.len;\n>\n> This is not something you introduced, but while we are here, you may\n> want to use ce->namelen, which would be a little faster than treating it\n> as a string (especially for strbuf, as it can then know up front how big\n> the size is).\n\nHmmmm, do you mean something like this on top?\n\ndiff --git a/entry.c b/entry.c\nindex d955af5..0d48292 100644\n--- a/entry.c\n+++ b/entry.c\n@@ -246,7 +246,9 @@ int checkout_entry(struct cache_entry *ce,\n \t\treturn write_entry(ce, topath, state, 1);\n \n \tstrbuf_reset(&path_buf);\n-\tstrbuf_addf(&path_buf, \"%.*s%s\", state->base_dir_len, state->base_dir, ce->name);\n+\tstrbuf_addf(&path_buf, \"%.*s%.*s\",\n+\t\t state->base_dir_len, state->base_dir,\n+\t\t ce_namelen(ce), ce->name);\n \tpath = path_buf.buf;\n \tlen = path_buf.len;\n \n\n-- \n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n\n--- \nYou received this message because you are subscribed to the Google Groups \"msysGit\" group.\nTo unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.\nFor more options, visit https://groups.google.com/groups/opt_out.\n"},{"id":"229356","messageId":"20131023181057.GA7950@sigill.intra.peff.net","threadId":"35042","inReplyTo":"xmqqa9hzrh9k.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 1/2] entry.c: convert checkout_entry to use strbuf","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-10-23T18:10:58Z","receivedAt":"2013-10-23T18:10:58Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Oct 23, 2013 at 11:09:27AM -0700, Junio C Hamano wrote:\n\n> > This is not something you introduced, but while we are here, you may\n> > want to use ce->namelen, which would be a little faster than treating it\n> > as a string (especially for strbuf, as it can then know up front how big\n> > the size is).\n> \n> Hmmmm, do you mean something like this on top?\n> \n> diff --git a/entry.c b/entry.c\n> index d955af5..0d48292 100644\n> --- a/entry.c\n> +++ b/entry.c\n> @@ -246,7 +246,9 @@ int checkout_entry(struct cache_entry *ce,\n> \t\treturn write_entry(ce, topath, state, 1);\n> \n> \tstrbuf_reset(&path_buf);\n> -\tstrbuf_addf(&path_buf, \"%.*s%s\", state->base_dir_len, state->base_dir, ce->name);\n> +\tstrbuf_addf(&path_buf, \"%.*s%.*s\",\n> +\t\t state->base_dir_len, state->base_dir,\n> +\t\t ce_namelen(ce), ce->name);\n> \tpath = path_buf.buf;\n> \tlen = path_buf.len;\n\nYes, though I actually find Erik's version with two separate strbuf_add\ninvocations slightly more readable (it _could_ result in two\nallocations, but again, we are amortizing the growth over many calls\nanyway, so most of them will not need to grow the buffer at all).\n\n-Peff\n\n-- \n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n\n--- \nYou received this message because you are subscribed to the Google Groups \"msysGit\" group.\nTo unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.\nFor more options, visit https://groups.google.com/groups/opt_out.\n"},{"id":"229373","messageId":"CACsJy8Ap005pUBZH0k=7F9nON9Vb5SoO-1McPAwRkLCC0YoPMQ@mail.gmail.com","threadId":"35042","inReplyTo":"xmqqeh7bri1h.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 2/2] entry.c: convert write_entry to use strbuf","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2013-10-24T01:23:41Z","receivedAt":"2013-10-24T01:23:41Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Thu, Oct 24, 2013 at 12:52 AM, Junio C Hamano wrote:\n> Nguyễn Thái Ngọc Duy writes:\n>\n>> The strcpy call in open_output_fd() implies that the output buffer\n>> must be at least 25 chars long.\n>\n> Hmph, where does that 25 come from?\n>\n> [snipped]\n\nMuch better. Thanks.\n-- \nDuy\n\n-- \n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n\n--- \nYou received this message because you are subscribed to the Google Groups \"msysGit\" group.\nTo unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.\nFor more options, visit https://groups.google.com/groups/opt_out.\n"},{"id":"229374","messageId":"1382579735-21893-1-git-send-email-pclouds@gmail.com","threadId":"35042","inReplyTo":"1382532907-30561-1-git-send-email-pclouds@gmail.com","subject":"[PATCH v2] entry.c: convert checkout_entry to use strbuf","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2013-10-24T01:55:35Z","receivedAt":"2013-10-24T01:55:35Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"The old code does not do boundary check so any paths longer than\nPATH_MAX can cause buffer overflow. Replace it with strbuf to handle\npaths of arbitrary length.\n\nThe OS may reject if the path is too long though. But in that case we\nreport the cause (e.g. name too long) and usually move on to checking\nout the next entry.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n v2 does two strbuf_add() instead of one hard-to-read strbuf_addf()\n\n entry.c | 13 ++++++++-----\n 1 file changed, 8 insertions(+), 5 deletions(-)\n\ndiff --git a/entry.c b/entry.c\nindex acc892f..fbb4863 100644\n--- a/entry.c\n+++ b/entry.c\n@@ -237,16 +237,19 @@ static int check_path(const char *path, int len, struct stat *st, int skiplen)\n int checkout_entry(struct cache_entry *ce,\n \t\t const struct checkout *state, char *topath)\n {\n-\tstatic char path[PATH_MAX + 1];\n+\tstatic struct strbuf path_buf = STRBUF_INIT;\n+\tchar *path;\n \tstruct stat st;\n-\tint len = state->base_dir_len;\n+\tint len;\n \n \tif (topath)\n \t\treturn write_entry(ce, topath, state, 1);\n \n-\tmemcpy(path, state->base_dir, len);\n-\tstrcpy(path + len, ce->name);\n-\tlen += ce_namelen(ce);\n+\tstrbuf_reset(&path_buf);\n+\tstrbuf_add(&path_buf, state->base_dir, state->base_dir_len);\n+\tstrbuf_add(&path_buf, ce->name, ce_namelen(ce));\n+\tpath = path_buf.buf;\n+\tlen = path_buf.len;\n \n \tif (!check_path(path, len, &st, state->base_dir_len)) {\n \t\tunsigned changed = ce_match_stat(ce, &st, CE_MATCH_IGNORE_VALID|CE_MATCH_IGNORE_SKIP_WORKTREE);\n-- \n1.8.2.82.gc24b958\n\n-- \n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n\n--- \nYou received this message because you are subscribed to the Google Groups \"msysGit\" group.\nTo unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.\nFor more options, visit https://groups.google.com/groups/opt_out.\n"},{"id":"229427","messageId":"xmqq8uxio3ed.fsf@gitster.dls.corp.google.com","threadId":"35042","inReplyTo":"CACsJy8Ap005pUBZH0k=7F9nON9Vb5SoO-1McPAwRkLCC0YoPMQ@mail.gmail.com","subject":"Re: [PATCH 2/2] entry.c: convert write_entry to use strbuf","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-10-24T19:49:30Z","receivedAt":"2013-10-24T19:49:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Duy Nguyen writes:\n\n> On Thu, Oct 24, 2013 at 12:52 AM, Junio C Hamano wrote:\n>> Nguyễn Thái Ngọc Duy writes:\n>>\n>>> The strcpy call in open_output_fd() implies that the output buffer\n>>> must be at least 25 chars long.\n>>\n>> Hmph, where does that 25 come from?\n>>\n>> [snipped]\n>\n> Much better. Thanks.\n\nSo where does that 25 come from?\n\nWe strcpy \".merge_link_XXXXXX\" or \".merge_file_XXXXXX\" into path[]\nand run mkstemp() on it, and these templates are 18 bytes long, so I\nam puzzled.\n\nIs 25 \"just a small random number that is surely longer than these\ntemplates--did not bother to count how long the templates are\"?\nThat's fine by me; I am just trying to make sure I am not missing\nanything that turns these templates into a longer filename.\n\nThanks.\n\n-- \n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n\n--- \nYou received this message because you are subscribed to the Google Groups \"msysGit\" group.\nTo unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.\nFor more options, visit https://groups.google.com/groups/opt_out.\n"},{"id":"229453","messageId":"CACsJy8D0PiY487N-OB5KuJOvz26vyu8dtUtbxticqZVScomaDA@mail.gmail.com","threadId":"35042","inReplyTo":"xmqq8uxio3ed.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 2/2] entry.c: convert write_entry to use strbuf","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2013-10-24T23:47:08Z","receivedAt":"2013-10-24T23:47:08Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Fri, Oct 25, 2013 at 2:49 AM, Junio C Hamano wrote:\n> Duy Nguyen writes:\n>\n>> On Thu, Oct 24, 2013 at 12:52 AM, Junio C Hamano wrote:\n>>> Nguyễn Thái Ngọc Duy writes:\n>>>\n>>>> The strcpy call in open_output_fd() implies that the output buffer\n>>>> must be at least 25 chars long.\n>>>\n>>> Hmph, where does that 25 come from?\n>>>\n>>> [snipped]\n>>\n>> Much better. Thanks.\n>\n> So where does that 25 come from?\n>\n> We strcpy \".merge_link_XXXXXX\" or \".merge_file_XXXXXX\" into path[]\n> and run mkstemp() on it, and these templates are 18 bytes long, so I\n> am puzzled.\n>\n> Is 25 \"just a small random number that is surely longer than these\n> templates--did not bother to count how long the templates are\"?\n\nYes. I was too lazy to subtract precisely the column number from\nbetween the quotes, so I just made sure the number is large enough to\ncover the columns..\n\n> That's fine by me; I am just trying to make sure I am not missing\n> anything that turns these templates into a longer filename.\n-- \nDuy\n\n-- \n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n\n--- \nYou received this message because you are subscribed to the Google Groups \"msysGit\" group.\nTo unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.\nFor more options, visit https://groups.google.com/groups/opt_out.\n"},{"id":"231138","messageId":"1385491163-18407-1-git-send-email-apelisse@gmail.com","threadId":"35042","inReplyTo":"CACsJy8AooiUNRfnqDLBmx=KPnztjdNuF4bYY2b=Egs3gdiW6KA@mail.gmail.com","subject":"[PATCH] Prevent buffer overflows when path is too long","fromName":"Antoine Pelisse","fromEmail":"apelisse@gmail.com","sentAt":"2013-11-26T18:39:23Z","receivedAt":"2013-11-26T18:39:23Z","isPatch":true,"sender":{"key":"apelisse@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1929644?v=4"},"body":"Some buffers created with PATH_MAX length are not checked when being\nwritten, and can overflow if PATH_MAX is not big enough to hold the\npath.\n\nSome of the use-case are probably impossible to reach, and the program\ndies if the path looks too long. When it would be possible for the user\nto use a longer path, simply use strbuf to build it.\n\nReported-by: Wataru Noguchi \nSigned-off-by: Antoine Pelisse \n---\n abspath.c | 10 ++++++++--\n diffcore-order.c | 14 +++++++++-----\n unpack-trees.c | 2 ++\n 3 files changed, 19 insertions(+), 7 deletions(-)\n\ndiff --git a/abspath.c b/abspath.c\nindex e390994..29a5f9d 100644\n--- a/abspath.c\n+++ b/abspath.c\n@@ -216,11 +216,16 @@ const char *absolute_path(const char *path)\n const char *prefix_filename(const char *pfx, int pfx_len, const char *arg)\n {\n \tstatic char path[PATH_MAX];\n+\n+\tif (pfx_len >= PATH_MAX)\n+\t\tdie(\"Too long prefix path: %s\", pfx);\n+\n #ifndef GIT_WINDOWS_NATIVE\n \tif (!pfx_len || is_absolute_path(arg))\n \t\treturn arg;\n \tmemcpy(path, pfx, pfx_len);\n-\tstrcpy(path + pfx_len, arg);\n+\tif (strlcpy(path + pfx_len, arg, PATH_MAX - pfx_len) > PATH_MAX)\n+\t\tdie(\"Too long path: %s\", path);\n #else\n \tchar *p;\n \t/* don't add prefix to absolute paths, but still replace '\\' by '/' */\n@@ -228,7 +233,8 @@ const char *prefix_filename(const char *pfx, int pfx_len, const char *arg)\n \t\tpfx_len = 0;\n \telse if (pfx_len)\n \t\tmemcpy(path, pfx, pfx_len);\n-\tstrcpy(path + pfx_len, arg);\n+\tif (strlcpy(path + pfx_len, arg, PATH_MAX - pfx_len) > PATH_MAX)\n+\t\tdie(\"Too long path: %s\", path);\n \tfor (p = path + pfx_len; *p; p++)\n \t\tif (*p == '\\\\')\n \t\t\t*p = '/';\ndiff --git a/diffcore-order.c b/diffcore-order.c\nindex 23e9385..87193f8 100644\n--- a/diffcore-order.c\n+++ b/diffcore-order.c\n@@ -73,20 +73,24 @@ struct pair_order {\n static int match_order(const char *path)\n {\n \tint i;\n-\tchar p[PATH_MAX];\n+\tstruct strbuf p = STRBUF_INIT;\n \n \tfor (i = 0; i < order_cnt; i++) {\n-\t\tstrcpy(p, path);\n-\t\twhile (p[0]) {\n+\t\tstrbuf_reset(&p);\n+\t\tstrbuf_addstr(&p, path);\n+\t\twhile (p.buf[0]) {\n \t\t\tchar *cp;\n-\t\t\tif (!fnmatch(order[i], p, 0))\n+\t\t\tif (!fnmatch(order[i], p.buf, 0)) {\n+\t\t\t\tstrbuf_release(&p);\n \t\t\t\treturn i;\n-\t\t\tcp = strrchr(p, '/');\n+\t\t\t}\n+\t\t\tcp = strrchr(p.buf, '/');\n \t\t\tif (!cp)\n \t\t\t\tbreak;\n \t\t\t*cp = 0;\n \t\t}\n \t}\n+\tstrbuf_release(&p);\n \treturn order_cnt;\n }\n \ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex 35cb05e..f93565b 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -918,6 +918,8 @@ static int clear_ce_flags_1(struct cache_entry **cache, int nr,\n \t\t\tint processed;\n \n \t\t\tlen = slash - name;\n+\t\t\tif (len + prefix_len >= PATH_MAX)\n+\t\t\t\tdie(\"Too long path: %s\", prefix);\n \t\t\tmemcpy(prefix + prefix_len, name, len);\n \n \t\t\t/*\n-- \n1.8.5.rc3.1.ga0b6b91\n\n-- \n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n\n--- \nYou received this message because you are subscribed to the Google Groups \"msysGit\" group.\nTo unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.\nFor more options, visit https://groups.google.com/groups/opt_out.\n"},{"id":"231143","messageId":"xmqqwqjvuelv.fsf@gitster.dls.corp.google.com","threadId":"35042","inReplyTo":"1385491163-18407-1-git-send-email-apelisse@gmail.com","subject":"Re: [PATCH] Prevent buffer overflows when path is too long","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-11-26T19:50:36Z","receivedAt":"2013-11-26T19:50:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Antoine Pelisse writes:\n\n> Some buffers created with PATH_MAX length are not checked when being\n> written, and can overflow if PATH_MAX is not big enough to hold the\n> path.\n\nPerhaps it is time to update all of them to use strbuf? The callers\nof prefix_filename() aren't that many, and all of them are prepared\nto stash the returned value away when they keep it longer term, so\nthey would not notice if we used \"static struct strbuf path\" and\ngave back \"path.buf\" (without strbuf_detach() on it). The buffer\nused in clear_ce_flags() and passed to clear_ce_flags_{1,dir} are\nnot seen outside the callchain, and can safely become strbuf, I\nthink.\n\n> abspath.c | 10 ++++++++--\n> diffcore-order.c | 14 +++++++++-----\n> unpack-trees.c | 2 ++\n> 3 files changed, 19 insertions(+), 7 deletions(-)\n>\n> diff --git a/abspath.c b/abspath.c\n> index e390994..29a5f9d 100644\n> --- a/abspath.c\n> +++ b/abspath.c\n> @@ -216,11 +216,16 @@ const char *absolute_path(const char *path)\n> const char *prefix_filename(const char *pfx, int pfx_len, const char *arg)\n> {\n> \tstatic char path[PATH_MAX];\n> +\n> +\tif (pfx_len >= PATH_MAX)\n> +\t\tdie(\"Too long prefix path: %s\", pfx);\n\nI do not think this is needed, and will reject a valid input that\nused to be accepted (e.g. arg is absolute so pfx does not matter).\n\n> #ifndef GIT_WINDOWS_NATIVE\n> \tif (!pfx_len || is_absolute_path(arg))\n> \t\treturn arg;\n> \tmemcpy(path, pfx, pfx_len);\n> -\tstrcpy(path + pfx_len, arg);\n> +\tif (strlcpy(path + pfx_len, arg, PATH_MAX - pfx_len) > PATH_MAX)\n> +\t\tdie(\"Too long path: %s\", path);\n\nRather, have that \"too long a prefix?\" check before that memcpy().\n\n> #else\n> \tchar *p;\n> \t/* don't add prefix to absolute paths, but still replace '\\' by '/' */\n> @@ -228,7 +233,8 @@ const char *prefix_filename(const char *pfx, int pfx_len, const char *arg)\n> \t\tpfx_len = 0;\n> \telse if (pfx_len)\n> \t\tmemcpy(path, pfx, pfx_len);\n\n... and around here.\n\n> -\tstrcpy(path + pfx_len, arg);\n> +\tif (strlcpy(path + pfx_len, arg, PATH_MAX - pfx_len) > PATH_MAX)\n> +\t\tdie(\"Too long path: %s\", path);\n> \tfor (p = path + pfx_len; *p; p++)\n> \t\tif (*p == '\\\\')\n> \t\t\t*p = '/';\n\nThe above is curious. Unless we are doing the short-cut for \"no\nprefix so we can just return arg\" codepath, we know that the\nresulting length is always pfx_len + strlen(arg), no?\n\n> diff --git a/diffcore-order.c b/diffcore-order.c\n> index 23e9385..87193f8 100644\n> --- a/diffcore-order.c\n> +++ b/diffcore-order.c\n> @@ -73,20 +73,24 @@ struct pair_order {\n> static int match_order(const char *path)\n> {\n> \tint i;\n> -\tchar p[PATH_MAX];\n> +\tstruct strbuf p = STRBUF_INIT;\n> \n> \tfor (i = 0; i < order_cnt; i++) {\n> -\t\tstrcpy(p, path);\n> -\t\twhile (p[0]) {\n> +\t\tstrbuf_reset(&p);\n> +\t\tstrbuf_addstr(&p, path);\n> +\t\twhile (p.buf[0]) {\n> \t\t\tchar *cp;\n> -\t\t\tif (!fnmatch(order[i], p, 0))\n> +\t\t\tif (!fnmatch(order[i], p.buf, 0)) {\n> +\t\t\t\tstrbuf_release(&p);\n> \t\t\t\treturn i;\n> -\t\t\tcp = strrchr(p, '/');\n> +\t\t\t}\n> +\t\t\tcp = strrchr(p.buf, '/');\n> \t\t\tif (!cp)\n> \t\t\t\tbreak;\n> \t\t\t*cp = 0;\n> \t\t}\n> \t}\n> +\tstrbuf_release(&p);\n> \treturn order_cnt;\n> }\n> \n> diff --git a/unpack-trees.c b/unpack-trees.c\n> index 35cb05e..f93565b 100644\n> --- a/unpack-trees.c\n> +++ b/unpack-trees.c\n> @@ -918,6 +918,8 @@ static int clear_ce_flags_1(struct cache_entry **cache, int nr,\n> \t\t\tint processed;\n> \n> \t\t\tlen = slash - name;\n> +\t\t\tif (len + prefix_len >= PATH_MAX)\n> +\t\t\t\tdie(\"Too long path: %s\", prefix);\n> \t\t\tmemcpy(prefix + prefix_len, name, len);\n> \n> \t\t\t/*\n> -- \n> 1.8.5.rc3.1.ga0b6b91\n>\n> -- \n\n-- \n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n\n--- \nYou received this message because you are subscribed to the Google Groups \"msysGit\" group.\nTo unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.\nFor more options, visit https://groups.google.com/groups/opt_out.\n"},{"id":"231269","messageId":"CALWbr2wNNxkFrzxVvCzkQkm34M=6CiBvmnqjm+p73H1MoFoD7A@mail.gmail.com","threadId":"35042","inReplyTo":"xmqqwqjvuelv.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] Prevent buffer overflows when path is too long","fromName":"Antoine Pelisse","fromEmail":"apelisse@gmail.com","sentAt":"2013-11-29T12:12:02Z","receivedAt":"2013-11-29T12:12:02Z","isPatch":true,"sender":{"key":"apelisse@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1929644?v=4"},"body":"On Tue, Nov 26, 2013 at 8:50 PM, Junio C Hamano wrote:\n> Antoine Pelisse writes:\n>\n>> Some buffers created with PATH_MAX length are not checked when being\n>> written, and can overflow if PATH_MAX is not big enough to hold the\n>> path.\n>\n> Perhaps it is time to update all of them to use strbuf? The callers\n> of prefix_filename() aren't that many, and all of them are prepared\n> to stash the returned value away when they keep it longer term, so\n> they would not notice if we used \"static struct strbuf path\" and\n> gave back \"path.buf\" (without strbuf_detach() on it). The buffer\n> used in clear_ce_flags() and passed to clear_ce_flags_{1,dir} are\n> not seen outside the callchain, and can safely become strbuf, I\n> think.\n\nLet's do that, but shouldn't we also modify those that are currently\nsafe, like absolute_path() just above prefix_filename() ?\n\n>> abspath.c | 10 ++++++++--\n>> diffcore-order.c | 14 +++++++++-----\n>> unpack-trees.c | 2 ++\n>> 3 files changed, 19 insertions(+), 7 deletions(-)\n>>\n>> diff --git a/abspath.c b/abspath.c\n>> index e390994..29a5f9d 100644\n>> --- a/abspath.c\n>> +++ b/abspath.c\n>> @@ -216,11 +216,16 @@ const char *absolute_path(const char *path)\n>> const char *prefix_filename(const char *pfx, int pfx_len, const char *arg)\n>> {\n>> static char path[PATH_MAX];\n>> +\n>> + if (pfx_len >= PATH_MAX)\n>> + die(\"Too long prefix path: %s\", pfx);\n>\n> I do not think this is needed, and will reject a valid input that\n> used to be accepted (e.g. arg is absolute so pfx does not matter).\n\nMy mistake\n\n>> - strcpy(path + pfx_len, arg);\n>> + if (strlcpy(path + pfx_len, arg, PATH_MAX - pfx_len) > PATH_MAX)\n>> + die(\"Too long path: %s\", path);\n>> for (p = path + pfx_len; *p; p++)\n>> if (*p == '\\\\')\n>> *p = '/';\n>\n> The above is curious. Unless we are doing the short-cut for \"no\n> prefix so we can just return arg\" codepath, we know that the\n> resulting length is always pfx_len + strlen(arg), no?\n\nIf you mean that the test should be more like the following:\n+ if (strlcpy(path + pfx_len, arg, PATH_MAX - pfx_len) > PATH_MAX - pfx_len)\n\nThen of course, you are right, that's my mistake.\n\n-- \n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n\n--- \nYou received this message because you are subscribed to the Google Groups \"msysGit\" group.\nTo unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.\nFor more options, visit https://groups.google.com/groups/opt_out.\n"},{"id":"232021","messageId":"1387020676-5569-1-git-send-email-apelisse@gmail.com","threadId":"35042","inReplyTo":"xmqqwqjvuelv.fsf@gitster.dls.corp.google.com","subject":"[PATCH] Prevent buffer overflows when path is too long","fromName":"Antoine Pelisse","fromEmail":"apelisse@gmail.com","sentAt":"2013-12-14T11:31:16Z","receivedAt":"2013-12-14T11:31:16Z","isPatch":true,"sender":{"key":"apelisse@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1929644?v=4"},"body":"Some buffers created with PATH_MAX length are not checked when being\nwritten, and can overflow if PATH_MAX is not big enough to hold the\npath.\n\nReplace those buffers by strbufs so that their size is automatically\ngrown if necessary. They are created as static local variables to avoid\nreallocating memory on each call. Note that prefix_filename() returns\nthis static buffer so each callers should copy or use the string\nimmediately (this is currently true).\n\nReported-by: Wataru Noguchi \nSigned-off-by: Antoine Pelisse \n---\n abspath.c | 16 +++++++++-------\n diffcore-order.c | 11 ++++++-----\n unpack-trees.c | 51 +++++++++++++++++++++++++++------------------------\n 3 files changed, 42 insertions(+), 36 deletions(-)\n\ndiff --git a/abspath.c b/abspath.c\nindex e390994..9c908e3 100644\n--- a/abspath.c\n+++ b/abspath.c\n@@ -215,23 +215,25 @@ const char *absolute_path(const char *path)\n */\n const char *prefix_filename(const char *pfx, int pfx_len, const char *arg)\n {\n-\tstatic char path[PATH_MAX];\n+\tstatic struct strbuf path = STRBUF_INIT;\n #ifndef GIT_WINDOWS_NATIVE\n \tif (!pfx_len || is_absolute_path(arg))\n \t\treturn arg;\n-\tmemcpy(path, pfx, pfx_len);\n-\tstrcpy(path + pfx_len, arg);\n+\tstrbuf_reset(&path);\n+\tstrbuf_add(&path, pfx, pfx_len);\n+\tstrbuf_addstr(&path, arg);\n #else\n \tchar *p;\n \t/* don't add prefix to absolute paths, but still replace '\\' by '/' */\n+\tstrbuf_reset(&path);\n \tif (is_absolute_path(arg))\n \t\tpfx_len = 0;\n \telse if (pfx_len)\n-\t\tmemcpy(path, pfx, pfx_len);\n-\tstrcpy(path + pfx_len, arg);\n-\tfor (p = path + pfx_len; *p; p++)\n+\t\tstrbuf_add(&path, pfx, pfx_len);\n+\tstrbuf_addstr(&path, arg);\n+\tfor (p = path.buf + pfx_len; *p; p++)\n \t\tif (*p == '\\\\')\n \t\t\t*p = '/';\n #endif\n-\treturn path;\n+\treturn path.buf;\n }\ndiff --git a/diffcore-order.c b/diffcore-order.c\nindex 23e9385..50c089b 100644\n--- a/diffcore-order.c\n+++ b/diffcore-order.c\n@@ -73,15 +73,16 @@ struct pair_order {\n static int match_order(const char *path)\n {\n \tint i;\n-\tchar p[PATH_MAX];\n+\tstatic struct strbuf p = STRBUF_INIT;\n \n \tfor (i = 0; i < order_cnt; i++) {\n-\t\tstrcpy(p, path);\n-\t\twhile (p[0]) {\n+\t\tstrbuf_reset(&p);\n+\t\tstrbuf_addstr(&p, path);\n+\t\twhile (p.buf[0]) {\n \t\t\tchar *cp;\n-\t\t\tif (!fnmatch(order[i], p, 0))\n+\t\t\tif (!fnmatch(order[i], p.buf, 0))\n \t\t\t\treturn i;\n-\t\t\tcp = strrchr(p, '/');\n+\t\t\tcp = strrchr(p.buf, '/');\n \t\t\tif (!cp)\n \t\t\t\tbreak;\n \t\t\t*cp = 0;\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex ad3e9a0..164354d 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -830,23 +830,24 @@ static int unpack_callback(int n, unsigned long mask, unsigned long dirmask, str\n }\n \n static int clear_ce_flags_1(struct cache_entry **cache, int nr,\n-\t\t\t char *prefix, int prefix_len,\n+\t\t\t struct strbuf *prefix,\n \t\t\t int select_mask, int clear_mask,\n \t\t\t struct exclude_list *el, int defval);\n \n /* Whole directory matching */\n static int clear_ce_flags_dir(struct cache_entry **cache, int nr,\n-\t\t\t char *prefix, int prefix_len,\n+\t\t\t struct strbuf *prefix,\n \t\t\t char *basename,\n \t\t\t int select_mask, int clear_mask,\n \t\t\t struct exclude_list *el, int defval)\n {\n \tstruct cache_entry **cache_end;\n \tint dtype = DT_DIR;\n-\tint ret = is_excluded_from_list(prefix, prefix_len,\n+\tint ret = is_excluded_from_list(prefix->buf, prefix->len,\n \t\t\t\t\tbasename, &dtype, el);\n+\tint rc;\n \n-\tprefix[prefix_len++] = '/';\n+\tstrbuf_addch(prefix, '/');\n \n \t/* If undecided, use matching result of parent dir in defval */\n \tif (ret < 0)\n@@ -854,7 +855,7 @@ static int clear_ce_flags_dir(struct cache_entry **cache, int nr,\n \n \tfor (cache_end = cache; cache_end != cache + nr; cache_end++) {\n \t\tstruct cache_entry *ce = *cache_end;\n-\t\tif (strncmp(ce->name, prefix, prefix_len))\n+\t\tif (strncmp(ce->name, prefix->buf, prefix->len))\n \t\t\tbreak;\n \t}\n \n@@ -865,10 +866,12 @@ static int clear_ce_flags_dir(struct cache_entry **cache, int nr,\n \t * calling clear_ce_flags_1(). That function will call\n \t * the expensive is_excluded_from_list() on every entry.\n \t */\n-\treturn clear_ce_flags_1(cache, cache_end - cache,\n-\t\t\t\tprefix, prefix_len,\n-\t\t\t\tselect_mask, clear_mask,\n-\t\t\t\tel, ret);\n+\trc = clear_ce_flags_1(cache, cache_end - cache,\n+\t\t\t prefix,\n+\t\t\t select_mask, clear_mask,\n+\t\t\t el, ret);\n+\tstrbuf_setlen(prefix, prefix->len - 1);\n+\treturn rc;\n }\n \n /*\n@@ -887,7 +890,7 @@ static int clear_ce_flags_dir(struct cache_entry **cache, int nr,\n * Top level path has prefix_len zero.\n */\n static int clear_ce_flags_1(struct cache_entry **cache, int nr,\n-\t\t\t char *prefix, int prefix_len,\n+\t\t\t struct strbuf *prefix,\n \t\t\t int select_mask, int clear_mask,\n \t\t\t struct exclude_list *el, int defval)\n {\n@@ -907,10 +910,10 @@ static int clear_ce_flags_1(struct cache_entry **cache, int nr,\n \t\t\tcontinue;\n \t\t}\n \n-\t\tif (prefix_len && strncmp(ce->name, prefix, prefix_len))\n+\t\tif (prefix->len && strncmp(ce->name, prefix->buf, prefix->len))\n \t\t\tbreak;\n \n-\t\tname = ce->name + prefix_len;\n+\t\tname = ce->name + prefix->len;\n \t\tslash = strchr(name, '/');\n \n \t\t/* If it's a directory, try whole directory match first */\n@@ -918,29 +921,26 @@ static int clear_ce_flags_1(struct cache_entry **cache, int nr,\n \t\t\tint processed;\n \n \t\t\tlen = slash - name;\n-\t\t\tmemcpy(prefix + prefix_len, name, len);\n+\t\t\tstrbuf_add(prefix, name, len);\n \n-\t\t\t/*\n-\t\t\t * terminate the string (no trailing slash),\n-\t\t\t * clear_c_f_dir needs it\n-\t\t\t */\n-\t\t\tprefix[prefix_len + len] = '\\0';\n \t\t\tprocessed = clear_ce_flags_dir(cache, cache_end - cache,\n-\t\t\t\t\t\t prefix, prefix_len + len,\n-\t\t\t\t\t\t prefix + prefix_len,\n+\t\t\t\t\t\t prefix,\n+\t\t\t\t\t\t prefix->buf + prefix->len - len,\n \t\t\t\t\t\t select_mask, clear_mask,\n \t\t\t\t\t\t el, defval);\n \n \t\t\t/* clear_c_f_dir eats a whole dir already? */\n \t\t\tif (processed) {\n \t\t\t\tcache += processed;\n+\t\t\t\tstrbuf_setlen(prefix, prefix->len - len);\n \t\t\t\tcontinue;\n \t\t\t}\n \n-\t\t\tprefix[prefix_len + len++] = '/';\n+\t\t\tstrbuf_addch(prefix, '/');\n \t\t\tcache += clear_ce_flags_1(cache, cache_end - cache,\n-\t\t\t\t\t\t prefix, prefix_len + len,\n+\t\t\t\t\t\t prefix,\n \t\t\t\t\t\t select_mask, clear_mask, el, defval);\n+\t\t\tstrbuf_setlen(prefix, prefix->len - len - 1);\n \t\t\tcontinue;\n \t\t}\n \n@@ -961,9 +961,12 @@ static int clear_ce_flags(struct cache_entry **cache, int nr,\n \t\t\t int select_mask, int clear_mask,\n \t\t\t struct exclude_list *el)\n {\n-\tchar prefix[PATH_MAX];\n+\tstatic struct strbuf prefix = STRBUF_INIT;\n+\n+\tstrbuf_reset(&prefix);\n+\n \treturn clear_ce_flags_1(cache, nr,\n-\t\t\t\tprefix, 0,\n+\t\t\t\t&prefix,\n \t\t\t\tselect_mask, clear_mask,\n \t\t\t\tel, 0);\n }\n-- \n1.8.5.1.94.g19422b2\n"}]}