This is the mail archive of the cygwin-patches 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] Cygwin: console: Fix segfault on shared_console_info access.


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

-- 
Takashi Yano <takashi.yano@nifty.ne.jp>


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