On Sat, 22 Feb 2020 17:01:23 +0900
Takashi Yano wrote:
> Hi Corinna,
>
> On Fri, 21 Feb 2020 20:43:33 +0100
> Corinna Vinschen wrote:
> > On Feb 22 04:10, Takashi Yano wrote:
> > > - Accessing shared_console_info accidentaly causes segmentation
> > > fault when it is a NULL pointer. The cause of the problem reported
> > > in https://cygwin.com/ml/cygwin/2020-02/msg00197.html is this NULL
> > > pointer access in request_xterm_mode_output(). This patch fixes
> > > the issue.
> >
> > When does this occur? I guess this is during initialization. Is it
> > really necessary to switch to xterm mode at all at that time? If not,
> > it might be simpler to just
> >
> > - if (con_is_legacy)
> > + if (con_is_legacy || !shared_console_info)
> >
> > at the start of the functions and only switch to xterm mode when
> > fully up and running.
>
> This happens when request_xterm_mode_output() is called from
> close(). Usually, shared_console_info has been set when it is
> called from close(), buf this happnes in mintty case. Since I
> was not sure why shared_console_info is NULL in mintty case,
> I have investigated deeper.
>
> And I found the following code causes the same situation.
>
> /* fork-setsid.c: */
> /* gcc -mwindows fork-setsid.c -o fork-setsid */
> #include <unistd.h>
>
> int main()
> {
> if (!fork()) setsid();
> return 0;
> }
>
> In this case, close() is called via cygheap->close_ctty() from
> setsid() even though console is not opened.
>
> Therefore, the following patch also fixes the mintty issue.
> However, I am not sure this is the right thing.
>
> diff --git a/winsup/cygwin/dtable.cc b/winsup/cygwin/dtable.cc
> index 4652de929..47a78bae4 100644
> --- a/winsup/cygwin/dtable.cc
> +++ b/winsup/cygwin/dtable.cc
> @@ -937,6 +937,7 @@ dtable::fixup_after_exec ()
> void
> dtable::fixup_after_fork (HANDLE parent)
> {
> + bool ctty_opened = false;
> fhandler_base *fh;
> for (size_t i = 0; i < size; i++)
> if ((fh = fds[i]) != NULL)
> @@ -957,7 +958,11 @@ dtable::fixup_after_fork (HANDLE parent)
> SetStdHandle (std_consts[i], fh->get_handle ());
> else if (i <= 2)
> SetStdHandle (std_consts[i], fh->get_output_handle ());
> + if (cygheap->ctty == fh)
> + ctty_opened = true;
> }
> + if (!ctty_opened)
> + cygheap->ctty = NULL;
> }
>
> static void
This does not work as expected. How about this one?
diff --git a/winsup/cygwin/dtable.cc b/winsup/cygwin/dtable.cc
index 4652de929..138b7a1eb 100644
--- a/winsup/cygwin/dtable.cc
+++ b/winsup/cygwin/dtable.cc
@@ -937,6 +937,7 @@ dtable::fixup_after_exec ()
void
dtable::fixup_after_fork (HANDLE parent)
{
+ bool ctty_opened = false;
fhandler_base *fh;
for (size_t i = 0; i < size; i++)
if ((fh = fds[i]) != NULL)
@@ -957,7 +958,11 @@ dtable::fixup_after_fork (HANDLE parent)
SetStdHandle (std_consts[i], fh->get_handle ());
else if (i <= 2)
SetStdHandle (std_consts[i], fh->get_output_handle ());
+ if (cygheap->ctty == fh->archetype)
+ ctty_opened = true;
}
+ if (!ctty_opened)
+ cygheap->ctty = NULL;
}
static void
--
Takashi Yano <[email protected]>