* [PATCH] mingw: consider that UNICODE_STRING::Length counts bytes
@ 2016-12-17 13:20 Max Kirillov
0 siblings, 0 replies; 6+ messages in thread
From: Max Kirillov @ 2016-12-17 13:20 UTC (permalink / raw)
To: Junio C Hamano, Johannes Schindelin, Karsten Blees; +Cc: Max Kirillov, git
UNICODE_STRING::Length field means size of buffer in bytes[1], despite of buffer
itself being array of wchar_t. Because of that terminating zero is placed twice
as far. Fix it.
[1] https://msdn.microsoft.com/en-us/library/windows/desktop/aa380518.aspx
Signed-off-by: Max Kirillov <max@max630.net>
---
Access outside of buffer was very unlikely (for that user needed to redirect
standard fd to a file with path longer than ~250 symbols), it still did not
seem to do any harm, and otherwise it did not break because only substring is
checked, but it was still incorrect.
compat/winansi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/compat/winansi.c b/compat/winansi.c
index 3be60ce..6b4f736 100644
--- a/compat/winansi.c
+++ b/compat/winansi.c
@@ -553,7 +553,7 @@ static void detect_msys_tty(int fd)
buffer, sizeof(buffer) - 2, &result)))
return;
name = nameinfo->Name.Buffer;
- name[nameinfo->Name.Length] = 0;
+ name[nameinfo->Name.Length / sizeof(*name)] = 0;
/* check if this could be a MSYS2 pty pipe ('msys-XXXX-ptyN-XX') */
if (!wcsstr(name, L"msys-") || !wcsstr(name, L"-pty"))
--
2.3.4.2801.g3d0809b
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH] mingw: consider that UNICODE_STRING::Length counts bytes
@ 2016-12-19 21:32 Max Kirillov
2016-12-19 21:57 ` Junio C Hamano
2016-12-20 15:16 ` Johannes Schindelin
0 siblings, 2 replies; 6+ messages in thread
From: Max Kirillov @ 2016-12-19 21:32 UTC (permalink / raw)
To: Junio C Hamano, Johannes Schindelin, Karsten Blees; +Cc: Max Kirillov, git
UNICODE_STRING::Length field means size of buffer in bytes[1], despite of buffer
itself being array of wchar_t. Because of that terminating zero is placed twice
as far. Fix it.
[1] https://msdn.microsoft.com/en-us/library/windows/desktop/aa380518.aspx
Signed-off-by: Max Kirillov <max@max630.net>
---
Access outside of buffer was very unlikely (for that user needed to redirect
standard fd to a file with path longer than ~250 symbols), it still did not
seem to do any harm, and otherwise it did not break because only substring is
checked, but it was still incorrect.
compat/winansi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/compat/winansi.c b/compat/winansi.c
index 3be60ce..6b4f736 100644
--- a/compat/winansi.c
+++ b/compat/winansi.c
@@ -553,7 +553,7 @@ static void detect_msys_tty(int fd)
buffer, sizeof(buffer) - 2, &result)))
return;
name = nameinfo->Name.Buffer;
- name[nameinfo->Name.Length] = 0;
+ name[nameinfo->Name.Length / sizeof(*name)] = 0;
/* check if this could be a MSYS2 pty pipe ('msys-XXXX-ptyN-XX') */
if (!wcsstr(name, L"msys-") || !wcsstr(name, L"-pty"))
--
2.3.4.2801.g3d0809b
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] mingw: consider that UNICODE_STRING::Length counts bytes
2016-12-19 21:32 Max Kirillov
@ 2016-12-19 21:57 ` Junio C Hamano
2016-12-20 5:21 ` Max Kirillov
2016-12-20 15:16 ` Johannes Schindelin
1 sibling, 1 reply; 6+ messages in thread
From: Junio C Hamano @ 2016-12-19 21:57 UTC (permalink / raw)
To: Max Kirillov; +Cc: Johannes Schindelin, Karsten Blees, git, Johannes Sixt
Max Kirillov <max@max630.net> writes:
> UNICODE_STRING::Length field means size of buffer in bytes[1], despite of buffer
> itself being array of wchar_t. Because of that terminating zero is placed twice
> as far. Fix it.
>
> [1] https://msdn.microsoft.com/en-us/library/windows/desktop/aa380518.aspx
>
> Signed-off-by: Max Kirillov <max@max630.net>
> ---
Max, I see this is a resend from a few days ago. I suspect that we
are in a slow season, so there may be longer delay until we see
responses to a patch.
I will wait until taking any patch to files specific to MS Windows
platform (compat/win*, compat/mingw*, and compat/vcbuild*) without
first seeing it reviewed and acked by either of the two Johannes's
(well, there might be other people in addition to those two, whose
Acked-by/Reviewed-by I should be trusting; if that is the case, it
only shows how unqualified I would be as the first contact to be on
the To: line of such a patch).
I do not mind "see the patch, ping Johanneses as needed, wait and
then apply it to my tree" flow, but I also wonder if the process can
be made more efficient. Dscho (one of the Johanneses) gets many
patches specific to Windows port via the Git-for-Windows project and
then "upstreams" by forwarding with his sign-off in my direction,
and I do not mind that flow, either. Whichever one is the most
efficient for all three parties involved is fine by me, but if one
is preferred over the other, perhaps I should write it down in the
"notes from the maintainer" or somewhere?
Dscho, J6t, what do you think?
Thanks.
> Access outside of buffer was very unlikely (for that user needed to redirect
> standard fd to a file with path longer than ~250 symbols), it still did not
> seem to do any harm, and otherwise it did not break because only substring is
> checked, but it was still incorrect.
> compat/winansi.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/compat/winansi.c b/compat/winansi.c
> index 3be60ce..6b4f736 100644
> --- a/compat/winansi.c
> +++ b/compat/winansi.c
> @@ -553,7 +553,7 @@ static void detect_msys_tty(int fd)
> buffer, sizeof(buffer) - 2, &result)))
> return;
> name = nameinfo->Name.Buffer;
> - name[nameinfo->Name.Length] = 0;
> + name[nameinfo->Name.Length / sizeof(*name)] = 0;
>
> /* check if this could be a MSYS2 pty pipe ('msys-XXXX-ptyN-XX') */
> if (!wcsstr(name, L"msys-") || !wcsstr(name, L"-pty"))
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mingw: consider that UNICODE_STRING::Length counts bytes
2016-12-19 21:57 ` Junio C Hamano
@ 2016-12-20 5:21 ` Max Kirillov
0 siblings, 0 replies; 6+ messages in thread
From: Max Kirillov @ 2016-12-20 5:21 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, Karsten Blees, git, Johannes Sixt
On Mon, Dec 19, 2016 at 01:57:29PM -0800, Junio C Hamano wrote:
> Max, I see this is a resend from a few days ago
Sorry about resend. For some reason I don't get the list
copy (might be some clever duplicate elimination in my
forwardings), and marc.info seems to be slow to update, so I
had no indication the first message got into list. Now I see
they are there in the other archive.
PS: probably http://vger.kernel.org/vger-lists.html#git
should be updated because there is no archive at gmane
anymore.
--
Max
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mingw: consider that UNICODE_STRING::Length counts bytes
2016-12-19 21:32 Max Kirillov
2016-12-19 21:57 ` Junio C Hamano
@ 2016-12-20 15:16 ` Johannes Schindelin
2016-12-20 17:07 ` Junio C Hamano
1 sibling, 1 reply; 6+ messages in thread
From: Johannes Schindelin @ 2016-12-20 15:16 UTC (permalink / raw)
To: Max Kirillov; +Cc: Junio C Hamano, Karsten Blees, git
Hi Max,
On Mon, 19 Dec 2016, Max Kirillov wrote:
> UNICODE_STRING::Length field means size of buffer in bytes[1], despite
> of buffer itself being array of wchar_t. Because of that terminating
> zero is placed twice as far. Fix it.
This commit message needs to be wrapped at <= 76 columns per row.
> [1] https://msdn.microsoft.com/en-us/library/windows/desktop/aa380518.aspx
>
> Signed-off-by: Max Kirillov <max@max630.net>
> ---
> Access outside of buffer was very unlikely (for that user needed to redirect
> standard fd to a file with path longer than ~250 symbols), it still did not
> seem to do any harm, and otherwise it did not break because only substring is
> checked, but it was still incorrect.
Very good, thank you for fixing my mistake! I verified locally that it
does exactly the right thing with your patch.
ACK,
Dscho
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mingw: consider that UNICODE_STRING::Length counts bytes
2016-12-20 15:16 ` Johannes Schindelin
@ 2016-12-20 17:07 ` Junio C Hamano
0 siblings, 0 replies; 6+ messages in thread
From: Junio C Hamano @ 2016-12-20 17:07 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Max Kirillov, Karsten Blees, git
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> Hi Max,
>
> On Mon, 19 Dec 2016, Max Kirillov wrote:
>
>> UNICODE_STRING::Length field means size of buffer in bytes[1], despite
>> of buffer itself being array of wchar_t. Because of that terminating
>> zero is placed twice as far. Fix it.
>
> This commit message needs to be wrapped at <= 76 columns per row.
> ...
> Very good, thank you for fixing my mistake! I verified locally that it
> does exactly the right thing with your patch.
Thanks, both. I'll queue this like so.
-- >8 --
From: Max Kirillov <max@max630.net>
Date: Mon, 19 Dec 2016 23:32:00 +0200
Subject: [PATCH] mingw: consider that UNICODE_STRING::Length counts bytes
UNICODE_STRING::Length field means size of buffer in bytes[1],
despite of buffer itself being array of wchar_t. Because of that
terminating zero is placed twice as far. Fix it.
[1] https://msdn.microsoft.com/en-us/library/windows/desktop/aa380518.aspx
Signed-off-by: Max Kirillov <max@max630.net>
Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
compat/winansi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/compat/winansi.c b/compat/winansi.c
index 3be60ce1c6..6b4f736fdc 100644
--- a/compat/winansi.c
+++ b/compat/winansi.c
@@ -553,7 +553,7 @@ static void detect_msys_tty(int fd)
buffer, sizeof(buffer) - 2, &result)))
return;
name = nameinfo->Name.Buffer;
- name[nameinfo->Name.Length] = 0;
+ name[nameinfo->Name.Length / sizeof(*name)] = 0;
/* check if this could be a MSYS2 pty pipe ('msys-XXXX-ptyN-XX') */
if (!wcsstr(name, L"msys-") || !wcsstr(name, L"-pty"))
base-commit: f7f90e0f4f58d493242078d17c0eba41dd3f1f79
--
2.11.0-416-g1351c11cce
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-12-20 17:08 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-17 13:20 [PATCH] mingw: consider that UNICODE_STRING::Length counts bytes Max Kirillov
-- strict thread matches above, loose matches on Subject: below --
2016-12-19 21:32 Max Kirillov
2016-12-19 21:57 ` Junio C Hamano
2016-12-20 5:21 ` Max Kirillov
2016-12-20 15:16 ` Johannes Schindelin
2016-12-20 17:07 ` Junio C Hamano
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).