Patch for run-1.3.0-1 core dump

Patch for run-1.3.0-1 core dump

[foo]

Hi,

Whenever I execute run.exe, it generates run.exe.stackdump.

At line 370 in run.c, run2_freeargv() tries to free newargv, and
run2_freeqrgv() expects that newargv is terminated by NULL. However,
in shifting newargv at line 253-256, it fails to shift NULL
terminator. Therefore, run2_freeargv() frees memory illegally.
The following patch is a workaround.

--- run.c.old
+++ run.c.new
@@ -252,7 +252,7 @@
       newargv = run2_dupargv (argv);
       /* discard newargv[0] and shift up */
       free (newargv[0]);
-      for (newargc = 1; newargc < argc; newargc++)
+      for (newargc = 1; newargv[newargc-1] != NULL; newargc++)
          newargv[newargc-1] = newargv[newargc];
       newargc = argc - 1;


Regards,

---
   ggl329

--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

Re: Patch for run-1.3.0-1 core dump

[Charles Wilson]

On 8/10/2013 1:34 PM, foo wrote:
> Whenever I execute run.exe, it generates run.exe.stackdump.
>
> At line 370 in run.c, run2_freeargv() tries to free newargv, and
> run2_freeqrgv() expects that newargv is terminated by NULL. However,
> in shifting newargv at line 253-256, it fails to shift NULL
> terminator. Therefore, run2_freeargv() frees memory illegally.
> The following patch is a workaround.
>
> --- run.c.old
> +++ run.c.new
> @@ -252,7 +252,7 @@
>         newargv = run2_dupargv (argv);
>         /* discard newargv[0] and shift up */
>         free (newargv[0]);
> -      for (newargc = 1; newargc < argc; newargc++)
> +      for (newargc = 1; newargv[newargc-1] != NULL; newargc++)
>            newargv[newargc-1] = newargv[newargc];
>         newargc = argc - 1;

Thanks for the bug report and the patch. I'll investigate and update the 
package soon.

--
Chuck



--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

Re: Patch for run-1.3.0-1 core dump

[Jon TURNEY]

On 12/08/2013 15:22, Charles Wilson wrote:
> On 8/10/2013 1:34 PM, foo wrote:
>> Whenever I execute run.exe, it generates run.exe.stackdump.
>>
>> At line 370 in run.c, run2_freeargv() tries to free newargv, and
>> run2_freeqrgv() expects that newargv is terminated by NULL. However,
>> in shifting newargv at line 253-256, it fails to shift NULL
>> terminator. Therefore, run2_freeargv() frees memory illegally.
>> The following patch is a workaround.
>>
>> --- run.c.old
>> +++ run.c.new
>> @@ -252,7 +252,7 @@
>>         newargv = run2_dupargv (argv);
>>         /* discard newargv[0] and shift up */
>>         free (newargv[0]);
>> -      for (newargc = 1; newargc < argc; newargc++)
>> +      for (newargc = 1; newargv[newargc-1] != NULL; newargc++)
>>            newargv[newargc-1] = newargv[newargc];
>>         newargc = argc - 1;
> 
> Thanks for the bug report and the patch. I'll investigate and update the
> package soon.

Since I've been running with CYGWIN error_start always set at the moment, I've
noticed that run is always crashing after launching the process.

I went to all the trouble of investigating this, discovering that
run2_freeargv() is double-freeing the last element in newargv because the NULL
terminator isn't moved when the arguments are shifted down over newargv[0],
and writing a patch, before I noticed that we already had one :-(

--- origsrc/run-1.3.0/src/run.c 2013-07-24 16:26:39.000000000 +0100
+++ src/run-1.3.0/src/run.c     2014-02-17 17:08:49.125000000 +0000
@@ -254,6 +254,7 @@ realMain(int argc, char* argv[])
       free (newargv[0]);
       for (newargc = 1; newargc < argc; newargc++)
          newargv[newargc-1] = newargv[newargc];
+      newargv[argc-1] = 0;
       newargc = argc - 1;

       /* update execname */



--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

Re: Patch for run-1.3.0-1 core dump

[Charles Wilson]

On 2/17/2014 12:25 PM, Jon TURNEY wrote:
> Since I've been running with CYGWIN error_start always set at the moment, I've
> noticed that run is always crashing after launching the process.
>
> I went to all the trouble of investigating this, discovering that
> run2_freeargv() is double-freeing the last element in newargv because the NULL
> terminator isn't moved when the arguments are shifted down over newargv[0],
> and writing a patch, before I noticed that we already had one :-(

Yeah, I suck. I'm in the middle of a move, but I'll try to get this 
updated by this weekend.

--
Chuck



--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

Re: Patch for run-1.3.0-1 core dump

[Corinna Vinschen]

[-- Attachment #1: Type: text/plain, Size: 2438 bytes --]

Hi Jon,
Hi Chuck,

On Feb 17 17:25, Jon TURNEY wrote:
> On 12/08/2013 15:22, Charles Wilson wrote:
> > On 8/10/2013 1:34 PM, foo wrote:
> >> Whenever I execute run.exe, it generates run.exe.stackdump.
> >>
> >> At line 370 in run.c, run2_freeargv() tries to free newargv, and
> >> run2_freeqrgv() expects that newargv is terminated by NULL. However,
> >> in shifting newargv at line 253-256, it fails to shift NULL
> >> terminator. Therefore, run2_freeargv() frees memory illegally.
> >> The following patch is a workaround.
> >>
> >> --- run.c.old
> >> +++ run.c.new
> >> @@ -252,7 +252,7 @@
> >>         newargv = run2_dupargv (argv);
> >>         /* discard newargv[0] and shift up */
> >>         free (newargv[0]);
> >> -      for (newargc = 1; newargc < argc; newargc++)
> >> +      for (newargc = 1; newargv[newargc-1] != NULL; newargc++)
> >>            newargv[newargc-1] = newargv[newargc];
> >>         newargc = argc - 1;
> > 
> > Thanks for the bug report and the patch. I'll investigate and update the
> > package soon.
> 
> Since I've been running with CYGWIN error_start always set at the moment, I've
> noticed that run is always crashing after launching the process.
> 
> I went to all the trouble of investigating this, discovering that
> run2_freeargv() is double-freeing the last element in newargv because the NULL
> terminator isn't moved when the arguments are shifted down over newargv[0],
> and writing a patch, before I noticed that we already had one :-(
> 
> --- origsrc/run-1.3.0/src/run.c 2013-07-24 16:26:39.000000000 +0100
> +++ src/run-1.3.0/src/run.c     2014-02-17 17:08:49.125000000 +0000
> @@ -254,6 +254,7 @@ realMain(int argc, char* argv[])
>        free (newargv[0]);
>        for (newargc = 1; newargc < argc; newargc++)
>           newargv[newargc-1] = newargv[newargc];
> +      newargv[argc-1] = 0;
>        newargc = argc - 1;
> 
>        /* update execname */

There's still something wrong.  I build run with this patch locally,
and it seems to fix the issue at first sight.  However, after the
child process of run exits, run throws an exception in free(), and
the stack looks broken (on 64 bit).  It seems there is a double free
or a free of an entirely unrelated address.


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

Re: Patch for run-1.3.0-1 core dump

[Corinna Vinschen]

[-- Attachment #1: Type: text/plain, Size: 2652 bytes --]

On Feb 18 10:05, Corinna Vinschen wrote:
> Hi Jon,
> Hi Chuck,
> 
> On Feb 17 17:25, Jon TURNEY wrote:
> > On 12/08/2013 15:22, Charles Wilson wrote:
> > > On 8/10/2013 1:34 PM, foo wrote:
> > >> Whenever I execute run.exe, it generates run.exe.stackdump.
> > >>
> > >> At line 370 in run.c, run2_freeargv() tries to free newargv, and
> > >> run2_freeqrgv() expects that newargv is terminated by NULL. However,
> > >> in shifting newargv at line 253-256, it fails to shift NULL
> > >> terminator. Therefore, run2_freeargv() frees memory illegally.
> > >> The following patch is a workaround.
> > >>
> > >> --- run.c.old
> > >> +++ run.c.new
> > >> @@ -252,7 +252,7 @@
> > >>         newargv = run2_dupargv (argv);
> > >>         /* discard newargv[0] and shift up */
> > >>         free (newargv[0]);
> > >> -      for (newargc = 1; newargc < argc; newargc++)
> > >> +      for (newargc = 1; newargv[newargc-1] != NULL; newargc++)
> > >>            newargv[newargc-1] = newargv[newargc];
> > >>         newargc = argc - 1;
> > > 
> > > Thanks for the bug report and the patch. I'll investigate and update the
> > > package soon.
> > 
> > Since I've been running with CYGWIN error_start always set at the moment, I've
> > noticed that run is always crashing after launching the process.
> > 
> > I went to all the trouble of investigating this, discovering that
> > run2_freeargv() is double-freeing the last element in newargv because the NULL
> > terminator isn't moved when the arguments are shifted down over newargv[0],
> > and writing a patch, before I noticed that we already had one :-(
> > 
> > --- origsrc/run-1.3.0/src/run.c 2013-07-24 16:26:39.000000000 +0100
> > +++ src/run-1.3.0/src/run.c     2014-02-17 17:08:49.125000000 +0000
> > @@ -254,6 +254,7 @@ realMain(int argc, char* argv[])
> >        free (newargv[0]);
> >        for (newargc = 1; newargc < argc; newargc++)
> >           newargv[newargc-1] = newargv[newargc];
> > +      newargv[argc-1] = 0;
> >        newargc = argc - 1;
> > 
> >        /* update execname */
> 
> There's still something wrong.  I build run with this patch locally,
> and it seems to fix the issue at first sight.  However, after the
> child process of run exits, run throws an exception in free(), and
> the stack looks broken (on 64 bit).  It seems there is a double free
> or a free of an entirely unrelated address.

Scratch that.  I managed to fat-finger a one-line patch.  Sorry.


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

Re: Patch for run-1.3.0-1 core dump

[Max Polk]

On 2/18/2014 4:16 AM, Corinna Vinschen wrote:
> On Feb 17 17:25, Jon TURNEY wrote:
>>> ...
>>> I went to all the trouble of investigating this, discovering that
>>> run2_freeargv() is double-freeing the last element in newargv because the NULL
>>> terminator isn't moved when the arguments are shifted down over newargv[0],
>>> and writing a patch, before I noticed that we already had one :-(
>>>
>>> --- origsrc/run-1.3.0/src/run.c 2013-07-24 16:26:39.000000000 +0100
>>> +++ src/run-1.3.0/src/run.c     2014-02-17 17:08:49.125000000 +0000
>>> @@ -254,6 +254,7 @@ realMain(int argc, char* argv[])
>>>         free (newargv[0]);
>>>         for (newargc = 1; newargc < argc; newargc++)
>>>            newargv[newargc-1] = newargv[newargc];
>>> +      newargv[argc-1] = 0;
>>>         newargc = argc - 1;
>>>
>>>         /* update execname */
>> There's still something wrong.  I build run with this patch locally,
>> and it seems to fix the issue at first sight.  However, after the
>> child process of run exits, run throws an exception in free(), and
>> the stack looks broken (on 64 bit).  It seems there is a double free
>> or a free of an entirely unrelated address.
> Scratch that.  I managed to fat-finger a one-line patch.  Sorry.
>
> Corinna

Did my earlier patch get included?  I haven't seen a "run" new version yet.

http://www.cygwin.com/ml/cygwin/2013-12/msg00006.html

My patch was the one that properly quote arguments.  Maybe let's start 
with that before putting new stuff underneath it.  From Chuck: "I'll 
roll a new update fairly soon."

http://www.cygwin.com/ml/cygwin/2013-12/msg00045.html


--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

Re: Patch for run-1.3.0-1 core dump

[Patrick Herbst]

> On 2/18/2014 4:16 AM, Corinna Vinschen wrote:
> Did my earlier patch get included?  I haven't seen a "run" new version
> yet.
>
> http://www.cygwin.com/ml/cygwin/2013-12/msg00006.html
>
> My patch was the one that properly quote arguments.  Maybe let's start
> with that before putting new stuff underneath it.  From Chuck: "I'll
> roll a new update fairly soon."
>
> http://www.cygwin.com/ml/cygwin/2013-12/msg00045.html

I'm still looking for a released fix for this too.  Thus far I've been
using run 1.2.

Any word on this?


--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

Re: Patch for run-1.3.0-1 core dump

[Max Polk]

On 3/20/2014 8:57 AM, Patrick Herbst wrote:
>> On 2/18/2014 4:16 AM, Corinna Vinschen wrote:
>> Did my earlier patch get included?  I haven't seen a "run" new version
>> yet.
>>
>> http://www.cygwin.com/ml/cygwin/2013-12/msg00006.html
>>
>> My patch was the one that properly quote arguments.  Maybe let's start
>> with that before putting new stuff underneath it.  From Chuck: "I'll
>> roll a new update fairly soon."
>>
>> http://www.cygwin.com/ml/cygwin/2013-12/msg00045.html
> I'm still looking for a released fix for this too.  Thus far I've been
> using run 1.2.
>
> Any word on this?

In the mean time, you can run it from /usr/local/bin if you like:

wget -O RUN-patch.txt 'http://pastebin.com/download.php?i=4SSPDGQh'
export CVSROOT=:pserver:anoncvs@cygwin.com:/cvs/cygwin-apps
cvs checkout run
cd run
patch -p0 < ../RUN-patch.txt
autoreconf -vif
./configure --prefix=/usr/local
make
make install

You end up with:
/usr/local/bin/run.exe
/usr/local/share/man/man1/run.1

If you start natively from Windows, note that the run utility, by virtue 
of sitting in the same directory as the cygwin dlls, takes advantage of 
Window's behavior to find dlls in the same directory as the program 
being run.  So by installing to /usr/local/bin, and running it from 
Windows (not from Cygwin shell), assuming Cygwin bin is not in your 
path, you lose that advantage.  Then you have to modify the path where 
and when you execute run, or using run to launch /bin/bash -c command to 
run with a hidden shell as the parent, otherwise your app may not find 
what it needs.

If you start from Cygwin shell, you probably already have /usr/local/bin 
before /usr/bin in your path, but if not, make it so.


--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

Re: Patch for run-1.3.0-1 core dump

[Patrick Herbst]

>>
>> On 3/20/2014 8:57 AM, Patrick Herbst wrote:
>>
>>> On 2/18/2014 4:16 AM, Corinna Vinschen wrote:
>>> Did my earlier patch get included?  I haven't seen a "run" new version
>>> yet.
>>>
>>> http://www.cygwin.com/ml/cygwin/2013-12/msg00006.html
>>>
>>> My patch was the one that properly quote arguments.  Maybe let's start
>>> with that before putting new stuff underneath it.  From Chuck: "I'll
>>> roll a new update fairly soon."
>>
>>
>> http://www.cygwin.com/ml/cygwin/2013-12/msg00045.html
>>
>> I'm still looking for a released fix for this too.  Thus far I've been
>> using run 1.2.
>>
>> Any word on this?
>
>
>
> In the mean time, you can run it from /usr/local/bin if you like:
>
> wget -O RUN-patch.txt 'http://pastebin.com/download.php?i=4SSPDGQh'
> export CVSROOT=:pserver:anoncvs@cygwin.com:/cvs/cygwin-apps
> cvs checkout run
> cd run
> patch -p0 < ../RUN-patch.txt
> autoreconf -vif
> ./configure --prefix=/usr/local
> make
> make install
>
> You end up with:
> /usr/local/bin/run.exe
> /usr/local/share/man/man1/run.1

Ok So run.exe has not been updated even though patches have been submitted.

Can this be released yet??

Current version 1.3 produces a stackdump every . single . time . it runs.

what is the test procedure for this application??

--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple