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


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


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