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

2013-10-05 Thread Wataru Noguchi 

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/.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/.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:

On Thu, Oct 3, 2013 at 7:25 PM, Antoine Pelisse apeli...@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.0...@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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html



--

  Wataru Noguchi
  wnoguchi.0...@gmail.com
  http://wnoguchi.github.io/

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2013-10-03 Thread Antoine Pelisse 
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.

On Thu, Oct 3, 2013 at 12:26 AM, Wataru Noguchi wnoguchi.0...@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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2013-10-03 Thread Erik Faye-Lund 
On Thu, Oct 3, 2013 at 7:25 PM, Antoine Pelisse apeli...@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.0...@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 majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2013-10-02 Thread Wataru Noguchi 

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


// entry.c:237

int checkout_entry(struct cache_entry *ce,
   const struct checkout *state, char *topath)
{
//--
// crash!
static char path[PATH_MAX + 1];
//--
// works fine. but bad practice.
static char path[4096 + 1];
//--
struct stat st;
int len = state-base_dir_len;

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


Thanks.


(2013/10/01 22:35), Wataru Noguchi wrote:

Hi,

Thanks for your patch.

Unfortunately, in my case still crash...

But PATH_MAX length kinds issues interesting.

I'll try investigate a little more.

- PATH_MAX and O2

Thanks.

(2013/10/01 2:00), René Scharfe wrote:

Am 29.09.2013 04:56, schrieb Wataru Noguchi:

Hi,

Thanks for comments.

My currently working repository is

https://github.com/wnoguchi/git/tree/hotfix/mingw-multibyte-path-checkout-failure

I have revert commits to 1f10da3.
I'll try failure step.

- gcc optimization level is O2.(fail)
- gcc O0, O1 works fine.


$ gdb git-clone
GNU gdb 6.8
Copyright (C) 2008 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later http://gnu.org/licenses/gpl.html
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type show copying
and show warranty for details.
This GDB was configured as i686-pc-mingw32...
(gdb) r https://github.com/wnoguchi/mingw-checkout-crash.git
Starting program: C:\msysgit\git/git-clone.exe https://github.com/wnoguchi/mingw
-checkout-crash.git
[New thread 800.0xa10]
Error: dll starting at 0x779f not found.
Error: dll starting at 0x7590 not found.
Error: dll starting at 0x779f not found.
Error: dll starting at 0x778f not found.
[New thread 800.0x92c]
Cloning into 'mingw-checkout-crash'...
Error: dll starting at 0x29f not found.
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
[New thread 800.0xea0]

Program received signal SIGSEGV, Segmentation fault.
0x004d5200 in git_check_attr (
   path=0xacc6a0 ..., num=5, check=0x572440) at attr.c:754
754 const char *value = check_all_attr[check[i].attr-attr_n
r].value;
(gdb) list
749 int i;
750
751 collect_all_attrs(path);
752
753 for (i = 0; i  num; i++) {
754 const char *value = check_all_attr[check[i].attr-attr_n
r].value;
755 if (value == ATTR__UNKNOWN)
756 value = ATTR__UNSET;
757 check[i].value = value;
758 }


I get a different crash on Linux if I set PATH_MAX to 260.  The following
hackish patch prevents it.  Does it help in your case as well?  If it does
then I'll send a nicer (but longer) one.

Thanks,
René


diff --git a/unpack-trees.c b/unpack-trees.c
index 1a61e6f..9bd7dcb 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -961,7 +961,7 @@ static int clear_ce_flags(struct cache_entry **cache, int 
nr,
  int select_mask, int clear_mask,
  struct exclude_list *el)
  {
-char prefix[PATH_MAX];
+char prefix[4096];
  return clear_ce_flags_1(cache, nr,
  prefix, 0,
  select_mask, clear_mask,







--

  Wataru Noguchi
  wnoguchi.0...@gmail.com
  http://wnoguchi.github.io/

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2013-10-01 Thread Wataru Noguchi 

Hi,

Thanks for your patch.

Unfortunately, in my case still crash...

But PATH_MAX length kinds issues interesting.

I'll try investigate a little more.

- PATH_MAX and O2

Thanks.

(2013/10/01 2:00), René Scharfe wrote:

Am 29.09.2013 04:56, schrieb Wataru Noguchi:

Hi,

Thanks for comments.

My currently working repository is

https://github.com/wnoguchi/git/tree/hotfix/mingw-multibyte-path-checkout-failure

I have revert commits to 1f10da3.
I'll try failure step.

- gcc optimization level is O2.(fail)
- gcc O0, O1 works fine.


$ gdb git-clone
GNU gdb 6.8
Copyright (C) 2008 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later http://gnu.org/licenses/gpl.html
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type show copying
and show warranty for details.
This GDB was configured as i686-pc-mingw32...
(gdb) r https://github.com/wnoguchi/mingw-checkout-crash.git
Starting program: C:\msysgit\git/git-clone.exe https://github.com/wnoguchi/mingw
-checkout-crash.git
[New thread 800.0xa10]
Error: dll starting at 0x779f not found.
Error: dll starting at 0x7590 not found.
Error: dll starting at 0x779f not found.
Error: dll starting at 0x778f not found.
[New thread 800.0x92c]
Cloning into 'mingw-checkout-crash'...
Error: dll starting at 0x29f not found.
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
[New thread 800.0xea0]

Program received signal SIGSEGV, Segmentation fault.
0x004d5200 in git_check_attr (
   path=0xacc6a0 ..., num=5, check=0x572440) at attr.c:754
754 const char *value = check_all_attr[check[i].attr-attr_n
r].value;
(gdb) list
749 int i;
750
751 collect_all_attrs(path);
752
753 for (i = 0; i  num; i++) {
754 const char *value = check_all_attr[check[i].attr-attr_n
r].value;
755 if (value == ATTR__UNKNOWN)
756 value = ATTR__UNSET;
757 check[i].value = value;
758 }


I get a different crash on Linux if I set PATH_MAX to 260.  The following
hackish patch prevents it.  Does it help in your case as well?  If it does
then I'll send a nicer (but longer) one.

Thanks,
René


diff --git a/unpack-trees.c b/unpack-trees.c
index 1a61e6f..9bd7dcb 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -961,7 +961,7 @@ static int clear_ce_flags(struct cache_entry **cache, int 
nr,
int select_mask, int clear_mask,
struct exclude_list *el)
  {
-   char prefix[PATH_MAX];
+   char prefix[4096];
return clear_ce_flags_1(cache, nr,
prefix, 0,
select_mask, clear_mask,




--
=
  Wataru Noguchi
  wnoguchi.0...@gmail.com
  http://wnoguchi.github.io/
=
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2013-09-30 Thread René Scharfe 
Am 29.09.2013 04:56, schrieb Wataru Noguchi:
 Hi,
 
 Thanks for comments.
 
 My currently working repository is
 
 https://github.com/wnoguchi/git/tree/hotfix/mingw-multibyte-path-checkout-failure
 
 I have revert commits to 1f10da3.
 I'll try failure step.
 
 - gcc optimization level is O2.(fail)
 - gcc O0, O1 works fine.
 
 
 $ gdb git-clone
 GNU gdb 6.8
 Copyright (C) 2008 Free Software Foundation, Inc.
 License GPLv3+: GNU GPL version 3 or later http://gnu.org/licenses/gpl.html
 This is free software: you are free to change and redistribute it.
 There is NO WARRANTY, to the extent permitted by law.  Type show copying
 and show warranty for details.
 This GDB was configured as i686-pc-mingw32...
 (gdb) r https://github.com/wnoguchi/mingw-checkout-crash.git
 Starting program: C:\msysgit\git/git-clone.exe 
 https://github.com/wnoguchi/mingw
 -checkout-crash.git
 [New thread 800.0xa10]
 Error: dll starting at 0x779f not found.
 Error: dll starting at 0x7590 not found.
 Error: dll starting at 0x779f not found.
 Error: dll starting at 0x778f not found.
 [New thread 800.0x92c]
 Cloning into 'mingw-checkout-crash'...
 Error: dll starting at 0x29f not found.
 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
 [New thread 800.0xea0]
 
 Program received signal SIGSEGV, Segmentation fault.
 0x004d5200 in git_check_attr (
   path=0xacc6a0 ..., num=5, check=0x572440) at attr.c:754
 754 const char *value = 
 check_all_attr[check[i].attr-attr_n
 r].value;
 (gdb) list
 749 int i;
 750
 751 collect_all_attrs(path);
 752
 753 for (i = 0; i  num; i++) {
 754 const char *value = 
 check_all_attr[check[i].attr-attr_n
 r].value;
 755 if (value == ATTR__UNKNOWN)
 756 value = ATTR__UNSET;
 757 check[i].value = value;
 758 }

I get a different crash on Linux if I set PATH_MAX to 260.  The following
hackish patch prevents it.  Does it help in your case as well?  If it does
then I'll send a nicer (but longer) one.

Thanks,
René


diff --git a/unpack-trees.c b/unpack-trees.c
index 1a61e6f..9bd7dcb 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -961,7 +961,7 @@ static int clear_ce_flags(struct cache_entry **cache, int 
nr,
int select_mask, int clear_mask,
struct exclude_list *el)
 {
-   char prefix[PATH_MAX];
+   char prefix[4096];
return clear_ce_flags_1(cache, nr,
prefix, 0,
select_mask, clear_mask,


--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2013-09-30 Thread Erik Faye-Lund 
On Mon, Sep 30, 2013 at 7:00 PM, René Scharfe l@web.de wrote:
 Am 29.09.2013 04:56, schrieb Wataru Noguchi:
 Hi,

 Thanks for comments.

 My currently working repository is

 https://github.com/wnoguchi/git/tree/hotfix/mingw-multibyte-path-checkout-failure

 I have revert commits to 1f10da3.
 I'll try failure step.

 - gcc optimization level is O2.(fail)
 - gcc O0, O1 works fine.


 $ gdb git-clone
 GNU gdb 6.8
 Copyright (C) 2008 Free Software Foundation, Inc.
 License GPLv3+: GNU GPL version 3 or later http://gnu.org/licenses/gpl.html
 This is free software: you are free to change and redistribute it.
 There is NO WARRANTY, to the extent permitted by law.  Type show copying
 and show warranty for details.
 This GDB was configured as i686-pc-mingw32...
 (gdb) r https://github.com/wnoguchi/mingw-checkout-crash.git
 Starting program: C:\msysgit\git/git-clone.exe 
 https://github.com/wnoguchi/mingw
 -checkout-crash.git
 [New thread 800.0xa10]
 Error: dll starting at 0x779f not found.
 Error: dll starting at 0x7590 not found.
 Error: dll starting at 0x779f not found.
 Error: dll starting at 0x778f not found.
 [New thread 800.0x92c]
 Cloning into 'mingw-checkout-crash'...
 Error: dll starting at 0x29f not found.
 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
 [New thread 800.0xea0]

 Program received signal SIGSEGV, Segmentation fault.
 0x004d5200 in git_check_attr (
   path=0xacc6a0 ..., num=5, check=0x572440) at attr.c:754
 754 const char *value = 
 check_all_attr[check[i].attr-attr_n
 r].value;
 (gdb) list
 749 int i;
 750
 751 collect_all_attrs(path);
 752
 753 for (i = 0; i  num; i++) {
 754 const char *value = 
 check_all_attr[check[i].attr-attr_n
 r].value;
 755 if (value == ATTR__UNKNOWN)
 756 value = ATTR__UNSET;
 757 check[i].value = value;
 758 }

 I get a different crash on Linux if I set PATH_MAX to 260.  The following
 hackish patch prevents it.

Interesting. It does not seem to help here. Multiple issues in the
same code-path?

  Does it help in your case as well?  If it does
 then I'll send a nicer (but longer) one.

Sounds like something that's still worth investigating, even if it
does not help.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html