This is the mail archive of the cygwin mailing list for the Cygwin project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Patch for run-1.3.0-1 core dump


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: pgpiPEf7abtZQ.pgp
Description: PGP signature


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]