[PATCH] Cygwin: console: Fix segfault on shared_console_info access.

Takashi Yano takashi.yano@nifty.ne.jp
Sat Feb 22 13:36:00 GMT 2020


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 <takashi.yano@nifty.ne.jp>



More information about the Cygwin-patches mailing list