Re: [PATCH] Cygwin: console: Make the console accessible from other terminals.

[Corinna Vinschen <corinna-cygwin@cygwin.com>]

On Dec 21 19:23, Takashi Yano wrote:
> On Tue, 20 Dec 2022 22:48:06 +0100
> Corinna Vinschen wrote:
> > On Dec 20 21:45, Takashi Yano wrote:
> > > Previously, the console device could not be accessed from other terminals.
> > > Due to this limitation, GNU screen and tmux cannot be opened in console.
> > > With this patch, console device can be accessed from other TTYs, such as
> > > other consoles or ptys. Thanks to this patch, screen and tmux get working
> > > in console.
> > > 
> > > Signed-off-by: Takashi Yano <takashi.yano@nifty.ne.jp>
> > > ---
> > >  winsup/cygwin/devices.cc                |  24 +-
> > >  winsup/cygwin/devices.in                |  24 +-
> > >  winsup/cygwin/fhandler/console.cc       | 438 +++++++++++++++++-------
> > >  winsup/cygwin/fhandler/pty.cc           |   4 +-
> > >  winsup/cygwin/local_includes/fhandler.h |  26 +-
> > >  winsup/cygwin/local_includes/winsup.h   |   1 -
> > >  winsup/cygwin/select.cc                 |   2 +
> > >  7 files changed, 382 insertions(+), 137 deletions(-)
> > 
> > I just toyed around with screen and this looks really great.
> > 
> > Just one question: What about security?  If we now can share
> > consoles, don't we need fchmod/fchown calls, too?
> 
> Thanks for reviewing.
> 
> As for security, AttachConsole() for another user's process
> will failed with ERROR_ACCESS_DENIED, so the console of
> another user is inaccessible.

That's what I'm wondering about...

Since Windows 7, Console handles are real OS handles, not just pseudo
handles like in olden times.

Given they are real handles, they should have an ACL attached and,
theoretically, it should be possible to change this ACL.

> However, fstat() does not return appropriate information,
> so, I implemented fhandler_console::fstat(). I also set proper
> errno for that case. Please see v2 patch.

That would also affect fstat, kind of like in fhandler_pty_slave.

However, there's something broken with these patches in terms of
debugging:

With current origin/master:

  $ ls -l  /dev/cons0
  crw-rw-rw- 4 corinna vinschen 3, 0 Dec 21 13:46 /dev/cons0
  $ strace -o xxx /bin/ls /dev/cons0
  /dev/cons0

After applying "pinfo: Align CTTY behavior to the statement of POSIX."

  $ ls -l /dev/cons0
  crw-rw-rw- 4 corinna vinschen 3, 0 Dec 21 13:51 /dev/cons0
  $ strace -o xxx /bin/ls /dev/cons0
  /usr/bin/ls: cannot access '/dev/cons0': No such device or address

"devices: Make generic console devices invisible from pty." doesn't
change this, but after applying "console: Make the console accessible
from other terminals.":

  $ ls -l /dev/cons0
  crw------- 4 corinna vinschen 3, 0 Dec 21 13:55 /dev/cons0
  $ strace -o xxx /bin/ls /dev/cons0
   670400 [main] ls 1630 C:\cygwin64\bin\ls.exe: *** fatal error - MapViewOfFileEx '(null)'(0x54), Win32 error 487.  Terminating.
   674526 [main] ls 1630 cygwin_exception::open_stackdumpfile: Dumping stack trace to ls.exe.stackdump

FWIW:

  $ strace -o xxx /bin/ls
   673796 [main] ls 1633 C:\cygwin64\bin\ls.exe: *** fatal error - MapViewOfFileEx '(null)'(0x54), Win32 error 487.  Terminating.
   676814 [main] ls 1633 cygwin_exception::open_stackdumpfile: Dumping stack trace to ls.exe.stackdump


Corinna