Re: one XXX in ksh(1)
On Mon, Dec 18, 2017 at 08:37:17PM +0100, Anton Lindqvist wrote: > On Mon, Dec 18, 2017 at 03:18:37PM +0800, Michael W. Bombardieri wrote: > > Hello, > > > > In my understanding the reason why texec had to be static was > > because the struct member ioact was never initialised. > > The call tree is execute() -> comexec() -> exchild() -> execute() > > and a fork happens in exchild(). > > The second execute() dereferences t->ioact on line 115 which causes > > SEGV. Setting ioact to NULL explicitly seems better than > > doing this via "static". > > Does this work for you? > > > > - Michael > > > > > > Index: exec.c > > === > > RCS file: /cvs/src/bin/ksh/exec.c,v > > retrieving revision 1.68 > > diff -u -p -u -r1.68 exec.c > > --- exec.c 11 Dec 2016 17:49:19 - 1.68 > > +++ exec.c 18 Dec 2017 07:05:47 - > > @@ -409,7 +409,7 @@ comexec(struct op *t, struct tbl *volati > > volatile int rv = 0; > > char *cp; > > char **lastp; > > - static struct op texec; /* Must be static (XXX but why?) */ > > + struct op texec; > > int type_flags; > > int keepasn_ok; > > int fcflags = FC_BI|FC_FUNC|FC_PATH; > > @@ -688,6 +688,7 @@ comexec(struct op *t, struct tbl *volati > > texec.left = t; /* for tprint */ > > texec.str = tp->val.s; > > texec.args = ap; > > + texec.ioact = NULL; > > rv = exchild(&texec, flags, xerrok, -1); > > break; > > } > > > > Good analysis. I would go even further and zero out texec completely. > > Comments? OK? > > Index: exec.c > === > RCS file: /cvs/src/bin/ksh/exec.c,v > retrieving revision 1.68 > diff -u -p -r1.68 exec.c > --- exec.c11 Dec 2016 17:49:19 - 1.68 > +++ exec.c18 Dec 2017 19:35:16 - > @@ -409,7 +409,7 @@ comexec(struct op *t, struct tbl *volati > volatile int rv = 0; > char *cp; > char **lastp; > - static struct op texec; /* Must be static (XXX but why?) */ > + struct op texec; > int type_flags; > int keepasn_ok; > int fcflags = FC_BI|FC_FUNC|FC_PATH; > @@ -684,6 +684,7 @@ comexec(struct op *t, struct tbl *volati > } > > /* to fork we set up a TEXEC node and call execute */ > + memset(&texec, 0, sizeof(texec)); > texec.type = TEXEC; > texec.left = t; /* for tprint */ > texec.str = tp->val.s; Just committed the diff above, thanks!
Re: one XXX in ksh(1)
On Mon, Dec 18, 2017 at 03:18:37PM +0800, Michael W. Bombardieri wrote: > Hello, > > In my understanding the reason why texec had to be static was > because the struct member ioact was never initialised. > The call tree is execute() -> comexec() -> exchild() -> execute() > and a fork happens in exchild(). > The second execute() dereferences t->ioact on line 115 which causes > SEGV. Setting ioact to NULL explicitly seems better than > doing this via "static". > Does this work for you? > > - Michael > > > Index: exec.c > === > RCS file: /cvs/src/bin/ksh/exec.c,v > retrieving revision 1.68 > diff -u -p -u -r1.68 exec.c > --- exec.c11 Dec 2016 17:49:19 - 1.68 > +++ exec.c18 Dec 2017 07:05:47 - > @@ -409,7 +409,7 @@ comexec(struct op *t, struct tbl *volati > volatile int rv = 0; > char *cp; > char **lastp; > - static struct op texec; /* Must be static (XXX but why?) */ > + struct op texec; > int type_flags; > int keepasn_ok; > int fcflags = FC_BI|FC_FUNC|FC_PATH; > @@ -688,6 +688,7 @@ comexec(struct op *t, struct tbl *volati > texec.left = t; /* for tprint */ > texec.str = tp->val.s; > texec.args = ap; > + texec.ioact = NULL; > rv = exchild(&texec, flags, xerrok, -1); > break; > } > Good analysis. I would go even further and zero out texec completely. Comments? OK? Index: exec.c === RCS file: /cvs/src/bin/ksh/exec.c,v retrieving revision 1.68 diff -u -p -r1.68 exec.c --- exec.c 11 Dec 2016 17:49:19 - 1.68 +++ exec.c 18 Dec 2017 19:35:16 - @@ -409,7 +409,7 @@ comexec(struct op *t, struct tbl *volati volatile int rv = 0; char *cp; char **lastp; - static struct op texec; /* Must be static (XXX but why?) */ + struct op texec; int type_flags; int keepasn_ok; int fcflags = FC_BI|FC_FUNC|FC_PATH; @@ -684,6 +684,7 @@ comexec(struct op *t, struct tbl *volati } /* to fork we set up a TEXEC node and call execute */ + memset(&texec, 0, sizeof(texec)); texec.type = TEXEC; texec.left = t; /* for tprint */ texec.str = tp->val.s;
one XXX in ksh(1)
Hello, In my understanding the reason why texec had to be static was because the struct member ioact was never initialised. The call tree is execute() -> comexec() -> exchild() -> execute() and a fork happens in exchild(). The second execute() dereferences t->ioact on line 115 which causes SEGV. Setting ioact to NULL explicitly seems better than doing this via "static". Does this work for you? - Michael Index: exec.c === RCS file: /cvs/src/bin/ksh/exec.c,v retrieving revision 1.68 diff -u -p -u -r1.68 exec.c --- exec.c 11 Dec 2016 17:49:19 - 1.68 +++ exec.c 18 Dec 2017 07:05:47 - @@ -409,7 +409,7 @@ comexec(struct op *t, struct tbl *volati volatile int rv = 0; char *cp; char **lastp; - static struct op texec; /* Must be static (XXX but why?) */ + struct op texec; int type_flags; int keepasn_ok; int fcflags = FC_BI|FC_FUNC|FC_PATH; @@ -688,6 +688,7 @@ comexec(struct op *t, struct tbl *volati texec.left = t; /* for tprint */ texec.str = tp->val.s; texec.args = ap; + texec.ioact = NULL; rv = exchild(&texec, flags, xerrok, -1); break; }
