Re: [PATCH] mingw-multibyte: fix memory acces violation and path length limits.

From: Wataru Noguchi <wnoguchi.0727@gmail.com>
Date: Oct 5, 2013, 11:39 UTC
Message-ID: <524FFA8E.70009@gmail.com>
In-Reply-To: <CABPQNSaqjKPGAQ4EKBSk+bQP2WMksc6M0YQxSkB91UrnFF28xQ@mail.gmail.com>

Hi,

I put following printf logs.

int checkout_entry(struct cache_entry *ce,
		   const struct checkout *state, char *topath)
{
	static char path[PATH_MAX + 1];
	struct stat st;
	int len = state->base_dir_len;

	if (topath)
		return write_entry(ce, topath, state, 1);

	memcpy(path, state->base_dir, len);
	fprintf(stderr, "path: %s\n", path);
	fprintf(stderr, "len: %d\n", len);
	strcpy(path + len, ce->name);
	len += ce_namelen(ce);
	fprintf(stderr, "path: %s\n", path);
	fprintf(stderr, "len: %d\n", len);
	fprintf(stderr, "path_max: %d\n", PATH_MAX);

--------------------------------------------------------------------------------------

crash result

wnoguchi@WIN-72R9044R72V /usr/tmp (master) $ git clone https://github.com/wnoguchi/mingw-checkout-crash.git a2 Cloning into 'a2'... remote: Counting objects: 8, done. remote: Compressing objects: 100% (7/7), done. remote: Total 8 (delta 0), reused 8 (delta 0) Unpacking objects: 100% (8/8), done. Checking connectivity... done path: len: 0 path: dummy 1-long-long-long-dirname/dummy 2-long-long -long-dirname/dummy 3-long-long-long-dirname/dummy 4-l ong-long-long-dirname/dummy 5-long-long-long-dirname/aaaaaaaaaaaa.txt len: 302 path_max: 259

crash!!

--------------------------------------------------------------------------------------

build with

CFLAGS = -g -O2 -fno-inline-small-functions -Wall

wnoguchi@WIN-72R9044R72V /usr/tmp (master) $ git clone https://github.com/wnoguchi/mingw-checkout-crash.git a3 Cloning into 'a3'... remote: Counting objects: 8, done. remote: Compressing objects: 100% (7/7), done. remote: Total 8 (delta 0), reused 8 (delta 0) Unpacking objects: 100% (8/8), done. Checking connectivity... done path: len: 0 path: dummy 1-long-long-long-dirname/dummy 2-long-long -long-dirname/dummy 3-long-long-long-dirname/dummy 4-l ong-long-long-dirname/dummy 5-long-long-long-dirname/aaaaaaaaaaaa.txt len: 302 path_max: 259

Warning: Your console font probably doesn't support Unicode. If you experience s
trange characters in the output, consider switching to a TrueType font such as L
ucida Console!

works fine.

------------------------------------------------------------------------------------

this result means actual path byte length over run path buffer?

	static char path[PATH_MAX + 1];

hmmm...

I'm not sure why -fno-inline-small-functions works.

(2013/10/04 2:36), Erik Faye-Lund wrote:

Show 53 quoted lines

> On Thu, Oct 3, 2013 at 7:25 PM, Antoine Pelisse <apelisse@gmail.com> wrote:
>> I've not followed the thread so much but, in that
>> entry.c::checkout_entry,() we do:
>>
>> memcpy(path, state->base_dir, len);
>> strcpy(path + len, ce->name);
>>
>> which can of course result in memory violation if PATH is not long enough.
>>
>
> ...aaand you're spot on. The following patch illustrates it:
>
> $ /git/git-clone.exe mingw-checkout-crash.git
> Cloning into 'mingw-checkout-crash'...
> done.
> fatal: argh, this won't work!
> warning: Clone succeeded, but checkout failed.
> You can inspect what was checked out with 'git status'
> and retry the checkout with 'git checkout -f HEAD'
>
> ---
>
> diff --git a/entry.c b/entry.c
> index acc892f..505638e 100644
> --- a/entry.c
> +++ b/entry.c
> @@ -244,6 +244,9 @@ int checkout_entry(struct cache_entry *ce,
>    if (topath)
>    return write_entry(ce, topath, state, 1);
>
> + if (len > PATH_MAX || len + strlen(ce->name) > PATH_MAX)
> + die("argh, this won't work!");
> +
>    memcpy(path, state->base_dir, len);
>    strcpy(path + len, ce->name);
>    len += ce_namelen(ce);
>
>
>> On Thu, Oct 3, 2013 at 12:26 AM, Wataru Noguchi <wnoguchi.0727@gmail.com> wrote:
>>> Hi,
>>>
>>> At last, I foundfollowing Makefile optimization suppression works fine in my
>>> case.
>>>
>>> CFLAGS = -g -O2 -fno-inline-small-functions -Wall
>>>
>>> Following optimization option cause crash,
>>>
>>> -finline-small-functions
>> --
>> To unsubscribe from this list: send the line "unsubscribe git" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
================================
   Wataru Noguchi
   wnoguchi.0727@gmail.com
   http://wnoguchi.github.io/
================================

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.