{"thread":{"id":"44737","subject":"[PATCH] mingw: consider that UNICODE_STRING::Length counts bytes","startedAt":"2016-12-19T21:33:57Z","lastAt":"2016-12-20T17:08:37Z","messageCount":5,"participants":["Max Kirillov","Junio C Hamano","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"308099","messageId":"1482183120-21592-1-git-send-email-max@max630.net","threadId":"44737","inReplyTo":null,"subject":"[PATCH] mingw: consider that UNICODE_STRING::Length counts bytes","fromName":"Max Kirillov","fromEmail":"max@max630.net","sentAt":"2016-12-19T21:32:00Z","receivedAt":"2016-12-19T21:33:57Z","isPatch":true,"sender":{"key":"max@max630.net","avatar":"https://avatars.githubusercontent.com/u/381560?v=4"},"body":"UNICODE_STRING::Length field means size of buffer in bytes[1], despite of buffer\nitself being array of wchar_t. Because of that terminating zero is placed twice\nas far. Fix it.\n\n[1] https://msdn.microsoft.com/en-us/library/windows/desktop/aa380518.aspx\n\nSigned-off-by: Max Kirillov \n---\nAccess outside of buffer was very unlikely (for that user needed to redirect\nstandard fd to a file with path longer than ~250 symbols), it still did not\nseem to do any harm, and otherwise it did not break because only substring is\nchecked, but it was still incorrect.\n compat/winansi.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/compat/winansi.c b/compat/winansi.c\nindex 3be60ce..6b4f736 100644\n--- a/compat/winansi.c\n+++ b/compat/winansi.c\n@@ -553,7 +553,7 @@ static void detect_msys_tty(int fd)\n \t\t\tbuffer, sizeof(buffer) - 2, &result)))\n \t\treturn;\n \tname = nameinfo->Name.Buffer;\n-\tname[nameinfo->Name.Length] = 0;\n+\tname[nameinfo->Name.Length / sizeof(*name)] = 0;\n \n \t/* check if this could be a MSYS2 pty pipe ('msys-XXXX-ptyN-XX') */\n \tif (!wcsstr(name, L\"msys-\") || !wcsstr(name, L\"-pty\"))\n-- \n2.3.4.2801.g3d0809b\n\n"},{"id":"308100","messageId":"xmqqtw9zmvjq.fsf@gitster.mtv.corp.google.com","threadId":"44737","inReplyTo":"1482183120-21592-1-git-send-email-max@max630.net","subject":"Re: [PATCH] mingw: consider that UNICODE_STRING::Length counts bytes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-12-19T21:57:29Z","receivedAt":"2016-12-19T21:57:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Max Kirillov writes:\n\n> UNICODE_STRING::Length field means size of buffer in bytes[1], despite of buffer\n> itself being array of wchar_t. Because of that terminating zero is placed twice\n> as far. Fix it.\n>\n> [1] https://msdn.microsoft.com/en-us/library/windows/desktop/aa380518.aspx\n>\n> Signed-off-by: Max Kirillov \n> ---\n\nMax, I see this is a resend from a few days ago. I suspect that we\nare in a slow season, so there may be longer delay until we see\nresponses to a patch.\n\nI will wait until taking any patch to files specific to MS Windows\nplatform (compat/win*, compat/mingw*, and compat/vcbuild*) without\nfirst seeing it reviewed and acked by either of the two Johannes's\n(well, there might be other people in addition to those two, whose\nAcked-by/Reviewed-by I should be trusting; if that is the case, it\nonly shows how unqualified I would be as the first contact to be on\nthe To: line of such a patch).\n\nI do not mind \"see the patch, ping Johanneses as needed, wait and\nthen apply it to my tree\" flow, but I also wonder if the process can\nbe made more efficient. Dscho (one of the Johanneses) gets many\npatches specific to Windows port via the Git-for-Windows project and\nthen \"upstreams\" by forwarding with his sign-off in my direction,\nand I do not mind that flow, either. Whichever one is the most\nefficient for all three parties involved is fine by me, but if one\nis preferred over the other, perhaps I should write it down in the\n\"notes from the maintainer\" or somewhere?\n\nDscho, J6t, what do you think?\n\nThanks.\n\n> Access outside of buffer was very unlikely (for that user needed to redirect\n> standard fd to a file with path longer than ~250 symbols), it still did not\n> seem to do any harm, and otherwise it did not break because only substring is\n> checked, but it was still incorrect.\n> compat/winansi.c | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/compat/winansi.c b/compat/winansi.c\n> index 3be60ce..6b4f736 100644\n> --- a/compat/winansi.c\n> +++ b/compat/winansi.c\n> @@ -553,7 +553,7 @@ static void detect_msys_tty(int fd)\n> \t\t\tbuffer, sizeof(buffer) - 2, &result)))\n> \t\treturn;\n> \tname = nameinfo->Name.Buffer;\n> -\tname[nameinfo->Name.Length] = 0;\n> +\tname[nameinfo->Name.Length / sizeof(*name)] = 0;\n> \n> \t/* check if this could be a MSYS2 pty pipe ('msys-XXXX-ptyN-XX') */\n> \tif (!wcsstr(name, L\"msys-\") || !wcsstr(name, L\"-pty\"))\n"},{"id":"308128","messageId":"20161220052130.GA10028@wheezy.local","threadId":"44737","inReplyTo":"xmqqtw9zmvjq.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] mingw: consider that UNICODE_STRING::Length counts bytes","fromName":"Max Kirillov","fromEmail":"max@max630.net","sentAt":"2016-12-20T05:21:30Z","receivedAt":"2016-12-20T05:30:22Z","isPatch":true,"sender":{"key":"max@max630.net","avatar":"https://avatars.githubusercontent.com/u/381560?v=4"},"body":"On Mon, Dec 19, 2016 at 01:57:29PM -0800, Junio C Hamano wrote:\n> Max, I see this is a resend from a few days ago\n\nSorry about resend. For some reason I don't get the list\ncopy (might be some clever duplicate elimination in my\nforwardings), and marc.info seems to be slow to update, so I\nhad no indication the first message got into list. Now I see\nthey are there in the other archive.\n\nPS: probably http://vger.kernel.org/vger-lists.html#git\nshould be updated because there is no archive at gmane\nanymore.\n\n-- \nMax\n"},{"id":"308145","messageId":"alpine.DEB.2.20.1612201610270.54750@virtualbox","threadId":"44737","inReplyTo":"1482183120-21592-1-git-send-email-max@max630.net","subject":"Re: [PATCH] mingw: consider that UNICODE_STRING::Length counts bytes","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-12-20T15:16:02Z","receivedAt":"2016-12-20T15:16:38Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Max,\n\nOn Mon, 19 Dec 2016, Max Kirillov wrote:\n\n> UNICODE_STRING::Length field means size of buffer in bytes[1], despite\n> of buffer itself being array of wchar_t. Because of that terminating\n> zero is placed twice as far. Fix it.\n\nThis commit message needs to be wrapped at <= 76 columns per row.\n\n> [1] https://msdn.microsoft.com/en-us/library/windows/desktop/aa380518.aspx\n> \n> Signed-off-by: Max Kirillov \n> ---\n> Access outside of buffer was very unlikely (for that user needed to redirect\n> standard fd to a file with path longer than ~250 symbols), it still did not\n> seem to do any harm, and otherwise it did not break because only substring is\n> checked, but it was still incorrect.\n\nVery good, thank you for fixing my mistake! I verified locally that it\ndoes exactly the right thing with your patch.\n\nACK,\nDscho\n"},{"id":"308152","messageId":"xmqqpokmlebc.fsf@gitster.mtv.corp.google.com","threadId":"44737","inReplyTo":"alpine.DEB.2.20.1612201610270.54750@virtualbox","subject":"Re: [PATCH] mingw: consider that UNICODE_STRING::Length counts bytes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-12-20T17:07:19Z","receivedAt":"2016-12-20T17:08:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> Hi Max,\n>\n> On Mon, 19 Dec 2016, Max Kirillov wrote:\n>\n>> UNICODE_STRING::Length field means size of buffer in bytes[1], despite\n>> of buffer itself being array of wchar_t. Because of that terminating\n>> zero is placed twice as far. Fix it.\n>\n> This commit message needs to be wrapped at <= 76 columns per row.\n> ...\n> Very good, thank you for fixing my mistake! I verified locally that it\n> does exactly the right thing with your patch.\n\nThanks, both. I'll queue this like so.\n\n-- >8 --\nFrom: Max Kirillov \nDate: Mon, 19 Dec 2016 23:32:00 +0200\nSubject: [PATCH] mingw: consider that UNICODE_STRING::Length counts bytes\n\nUNICODE_STRING::Length field means size of buffer in bytes[1],\ndespite of buffer itself being array of wchar_t. Because of that\nterminating zero is placed twice as far. Fix it.\n\n[1] https://msdn.microsoft.com/en-us/library/windows/desktop/aa380518.aspx\n\nSigned-off-by: Max Kirillov \nAcked-by: Johannes Schindelin \nSigned-off-by: Junio C Hamano \n---\n compat/winansi.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/compat/winansi.c b/compat/winansi.c\nindex 3be60ce1c6..6b4f736fdc 100644\n--- a/compat/winansi.c\n+++ b/compat/winansi.c\n@@ -553,7 +553,7 @@ static void detect_msys_tty(int fd)\n \t\t\tbuffer, sizeof(buffer) - 2, &result)))\n \t\treturn;\n \tname = nameinfo->Name.Buffer;\n-\tname[nameinfo->Name.Length] = 0;\n+\tname[nameinfo->Name.Length / sizeof(*name)] = 0;\n \n \t/* check if this could be a MSYS2 pty pipe ('msys-XXXX-ptyN-XX') */\n \tif (!wcsstr(name, L\"msys-\") || !wcsstr(name, L\"-pty\"))\n\nbase-commit: f7f90e0f4f58d493242078d17c0eba41dd3f1f79\n-- \n2.11.0-416-g1351c11cce\n\n"}]}