* [PATCH 0/2] Fix issues of stat()/fstat() for /dev/tty.
@ 2023-07-07 3:34 Takashi Yano
2023-07-07 3:34 ` [PATCH 1/2] Cygwin: stat(): Fix "Bad address" error on stat() " Takashi Yano
2023-07-07 3:34 ` [PATCH 2/2] Cygwin: fstat(): Fix st_rdev returned by fstat() " Takashi Yano
0 siblings, 2 replies; 8+ messages in thread
From: Takashi Yano @ 2023-07-07 3:34 UTC (permalink / raw)
To: cygwin-patches; +Cc: Takashi Yano
Takashi Yano (2):
Cygwin: stat(): Fix "Bad address" error on stat() for /dev/tty.
Cygwin: fstat(): Fix st_rdev returned by fstat() for /dev/tty.
winsup/cygwin/dtable.cc | 13 ++++++++++---
winsup/cygwin/fhandler/console.cc | 12 +++++++++++-
winsup/cygwin/fhandler/pty.cc | 8 +++++---
winsup/cygwin/fhandler/termios.cc | 9 +++++++++
winsup/cygwin/local_includes/fhandler.h | 12 ++++++++++--
5 files changed, 45 insertions(+), 9 deletions(-)
--
2.39.0
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/2] Cygwin: stat(): Fix "Bad address" error on stat() for /dev/tty.
2023-07-07 3:34 [PATCH 0/2] Fix issues of stat()/fstat() for /dev/tty Takashi Yano
@ 2023-07-07 3:34 ` Takashi Yano
2023-07-07 9:46 ` Corinna Vinschen
2023-07-07 3:34 ` [PATCH 2/2] Cygwin: fstat(): Fix st_rdev returned by fstat() " Takashi Yano
1 sibling, 1 reply; 8+ messages in thread
From: Takashi Yano @ 2023-07-07 3:34 UTC (permalink / raw)
To: cygwin-patches; +Cc: Takashi Yano, Bruce Jerrick
As reported in
https://cygwin.com/pipermail/cygwin/2023-June/253888.html,
"Bad address" error occurs when stat() is called after the commit
3721a756b0d8 ("Cygwin: console: Make the console accessible from
other terminals.").
There are two problems in the current code. One is fhandler_console::
fstat() calls get_ttyp()->getsid(). However, fh_alloc() in dtable.cc
omits to initialize the fhandler_console instance when stat() is
called. Due to this, get_ttyp() returns NULL and access violation
occurs. The other problem is fh_alloc() assigns fhandler_console
even if the CTTY is not a console. So the first problem above occurs
even if the CTTY is a pty.
This patch fixes the issue by:
1) Call set_unit() to initialize _tc if the get_ttyp() returns NULL.
2) Assign fhandler_pty_slave for /dev/tty if CTTY is a pty in fh_alloc().
Fixes: 3721a756b0d8 ("Cygwin: console: Make the console accessible
from other terminals.").
Fixes: 23771fa1f7028 ("dtable.cc (fh_alloc): Make different decisions
when generating fhandler for not-opened devices. Add kludge to deal
with opening /dev/tty.")
Reported-by: Bruce Jerrick <bmj001@gmail.com>
Signed-off-by: Takashi Yano <takashi.yano@nifty.ne.jp>
---
winsup/cygwin/dtable.cc | 8 +++++++-
winsup/cygwin/fhandler/console.cc | 6 ++++++
2 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/winsup/cygwin/dtable.cc b/winsup/cygwin/dtable.cc
index 18e0f3097..2aae2fd65 100644
--- a/winsup/cygwin/dtable.cc
+++ b/winsup/cygwin/dtable.cc
@@ -600,7 +600,13 @@ fh_alloc (path_conv& pc)
case FH_TTY:
if (!pc.isopen ())
{
- fhraw = cnew_no_ctor (fhandler_console, -1);
+ if (CTTY_IS_VALID (myself->ctty))
+ {
+ if (iscons_dev (myself->ctty))
+ fhraw = cnew_no_ctor (fhandler_console, -1);
+ else
+ fhraw = cnew_no_ctor (fhandler_pty_slave, -1);
+ }
debug_printf ("not called from open for /dev/tty");
}
else if (!CTTY_IS_VALID (myself->ctty) && last_tty_dev
diff --git a/winsup/cygwin/fhandler/console.cc b/winsup/cygwin/fhandler/console.cc
index 7768a9941..6aa3b50bf 100644
--- a/winsup/cygwin/fhandler/console.cc
+++ b/winsup/cygwin/fhandler/console.cc
@@ -4554,6 +4554,12 @@ fhandler_console::set_disable_master_thread (bool x, fhandler_console *cons)
int
fhandler_console::fstat (struct stat *st)
{
+ /* When stat() is called, fh_alloc() in dtable.cc omits to initialize
+ the console instance. Due to this, get_ttyp() returns NULL here.
+ So, calling set_unit() is necessary to access getsid(). */
+ if (!get_ttyp ())
+ set_unit ();
+
fhandler_base::fstat (st);
st->st_mode = S_IFCHR | S_IRUSR | S_IWUSR;
pinfo p (get_ttyp ()->getsid ());
--
2.39.0
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/2] Cygwin: fstat(): Fix st_rdev returned by fstat() for /dev/tty.
2023-07-07 3:34 [PATCH 0/2] Fix issues of stat()/fstat() for /dev/tty Takashi Yano
2023-07-07 3:34 ` [PATCH 1/2] Cygwin: stat(): Fix "Bad address" error on stat() " Takashi Yano
@ 2023-07-07 3:34 ` Takashi Yano
2023-07-07 10:10 ` Corinna Vinschen
1 sibling, 1 reply; 8+ messages in thread
From: Takashi Yano @ 2023-07-07 3:34 UTC (permalink / raw)
To: cygwin-patches; +Cc: Takashi Yano
While st_rdev returned by fstat() for /dev/tty should be FH_TTY,
the current cygwin1.dll returns FH_PTYS+minor or FH_CONS+minor.
Similarly, fstat() does not return correct value for /dev/console,
/dev/conout, /dev/conin or /dev/ptmx.
This patch fixes the issue by:
1) Introduce dev_refered_via in fhandler_termios.
2) Add new argument, which has dev_t value refered by open(),
for constructors of fhandler_pty_slave and fhandler_pty_master to
set the value of dev_refered_via.
3) Set st_rdev using dev_refered_via in fhandler_termios::fstat()
if it is available.
Signed-off-by: Takashi Yano <takashi.yano@nifty.ne.jp>
---
winsup/cygwin/dtable.cc | 5 +++--
winsup/cygwin/fhandler/console.cc | 8 ++++++--
winsup/cygwin/fhandler/pty.cc | 8 +++++---
winsup/cygwin/fhandler/termios.cc | 9 +++++++++
winsup/cygwin/local_includes/fhandler.h | 12 ++++++++++--
5 files changed, 33 insertions(+), 9 deletions(-)
diff --git a/winsup/cygwin/dtable.cc b/winsup/cygwin/dtable.cc
index 2aae2fd65..156445119 100644
--- a/winsup/cygwin/dtable.cc
+++ b/winsup/cygwin/dtable.cc
@@ -509,7 +509,7 @@ fh_alloc (path_conv& pc)
break;
case FH_PTMX:
if (pc.isopen ())
- fh = cnew (fhandler_pty_master, -1);
+ fh = cnew (fhandler_pty_master, -1, (dev_t) pc.dev);
else
fhraw = cnew_no_ctor (fhandler_pty_master, -1);
break;
@@ -618,7 +618,8 @@ fh_alloc (path_conv& pc)
if (iscons_dev (myself->ctty))
fh = cnew (fhandler_console, pc.dev);
else
- fh = cnew (fhandler_pty_slave, myself->ctty);
+ fh = cnew (fhandler_pty_slave,
+ minor (myself->ctty), (dev_t) pc.dev);
if (fh->dev () != FH_NADA)
fh->set_name ("/dev/tty");
}
diff --git a/winsup/cygwin/fhandler/console.cc b/winsup/cygwin/fhandler/console.cc
index 6aa3b50bf..f4e3fad92 100644
--- a/winsup/cygwin/fhandler/console.cc
+++ b/winsup/cygwin/fhandler/console.cc
@@ -2110,6 +2110,7 @@ fhandler_console::fhandler_console (fh_devices devunit) :
fhandler_termios (), input_ready (false), thread_sync_event (NULL),
input_mutex (NULL), output_mutex (NULL), unit (MAX_CONS_DEV)
{
+ dev_refered_via = (dev_t) devunit;
if (devunit > 0)
dev ().parse (devunit);
setup ();
@@ -4558,9 +4559,12 @@ fhandler_console::fstat (struct stat *st)
the console instance. Due to this, get_ttyp() returns NULL here.
So, calling set_unit() is necessary to access getsid(). */
if (!get_ttyp ())
- set_unit ();
+ {
+ dev_refered_via = get_device ();
+ set_unit ();
+ }
- fhandler_base::fstat (st);
+ fhandler_termios::fstat (st);
st->st_mode = S_IFCHR | S_IRUSR | S_IWUSR;
pinfo p (get_ttyp ()->getsid ());
if (p)
diff --git a/winsup/cygwin/fhandler/pty.cc b/winsup/cygwin/fhandler/pty.cc
index 1f2b634a0..67289369d 100644
--- a/winsup/cygwin/fhandler/pty.cc
+++ b/winsup/cygwin/fhandler/pty.cc
@@ -765,10 +765,11 @@ out:
/* pty slave stuff */
-fhandler_pty_slave::fhandler_pty_slave (int unit)
+fhandler_pty_slave::fhandler_pty_slave (int unit, dev_t via)
: fhandler_pty_common (), inuse (NULL), output_handle_nat (NULL),
io_handle_nat (NULL), slave_reading (NULL), num_reader (0)
{
+ dev_refered_via = via;
if (unit >= 0)
dev ().parse (DEV_PTYS_MAJOR, unit);
}
@@ -1769,7 +1770,7 @@ out:
int
fhandler_pty_slave::fstat (struct stat *st)
{
- fhandler_base::fstat (st);
+ fhandler_termios::fstat (st);
bool to_close = false;
if (!input_available_event)
@@ -1974,13 +1975,14 @@ errout:
/*******************************************************
fhandler_pty_master
*/
-fhandler_pty_master::fhandler_pty_master (int unit)
+fhandler_pty_master::fhandler_pty_master (int unit, dev_t via)
: fhandler_pty_common (), pktmode (0), master_ctl (NULL),
master_thread (NULL), from_master_nat (NULL), to_master_nat (NULL),
from_slave_nat (NULL), to_slave_nat (NULL), echo_r (NULL), echo_w (NULL),
dwProcessId (0), to_master (NULL), from_master (NULL),
master_fwd_thread (NULL)
{
+ dev_refered_via = via;
if (unit >= 0)
dev ().parse (DEV_PTYM_MAJOR, unit);
set_name ("/dev/ptmx");
diff --git a/winsup/cygwin/fhandler/termios.cc b/winsup/cygwin/fhandler/termios.cc
index 1a5dfdd1b..72a2e0e1b 100644
--- a/winsup/cygwin/fhandler/termios.cc
+++ b/winsup/cygwin/fhandler/termios.cc
@@ -691,6 +691,15 @@ fhandler_termios::tcgetsid ()
return -1;
}
+int
+fhandler_termios::fstat (struct stat *buf)
+{
+ fhandler_base::fstat (buf);
+ if (dev_refered_via > 0)
+ buf->st_rdev = dev_refered_via;
+ return 0;
+}
+
static bool
is_console_app (const WCHAR *filename)
{
diff --git a/winsup/cygwin/local_includes/fhandler.h b/winsup/cygwin/local_includes/fhandler.h
index f82f565cf..da7d40864 100644
--- a/winsup/cygwin/local_includes/fhandler.h
+++ b/winsup/cygwin/local_includes/fhandler.h
@@ -1936,6 +1936,7 @@ class fhandler_termios: public fhandler_base
virtual void acquire_input_mutex_if_necessary (DWORD ms) {};
virtual void release_input_mutex_if_necessary (void) {};
virtual void discard_input () {};
+ dev_t dev_refered_via;
/* Result status of processing keys in process_sigs(). */
enum process_sig_state {
@@ -1975,6 +1976,7 @@ class fhandler_termios: public fhandler_base
void echo_erase (int force = 0);
virtual off_t lseek (off_t, int);
pid_t tcgetsid ();
+ virtual int fstat (struct stat *buf);
fhandler_termios (void *) {}
@@ -2297,7 +2299,9 @@ private:
void copy_from (fhandler_base *x)
{
pc.free_strings ();
+ dev_t via = this->dev_refered_via; /* Do not copy dev_refered_via */
*this = *reinterpret_cast<fhandler_console *> (x);
+ this->dev_refered_via = via;
_copy_from_reset_helper ();
}
@@ -2424,7 +2428,7 @@ class fhandler_pty_slave: public fhandler_pty_common
typedef ptys_handle_set_t handle_set_t;
/* Constructor */
- fhandler_pty_slave (int);
+ fhandler_pty_slave (int, dev_t via = 0);
void set_output_handle_nat (HANDLE h) { output_handle_nat = h; }
HANDLE& get_output_handle_nat () { return output_handle_nat; }
@@ -2462,7 +2466,9 @@ class fhandler_pty_slave: public fhandler_pty_common
void copy_from (fhandler_base *x)
{
pc.free_strings ();
+ dev_t via = this->dev_refered_via; /* Do not copy dev_refered_via */
*this = *reinterpret_cast<fhandler_pty_slave *> (x);
+ this->dev_refered_via = via;
_copy_from_reset_helper ();
}
@@ -2537,7 +2543,7 @@ private:
public:
HANDLE get_echo_handle () const { return echo_r; }
/* Constructor */
- fhandler_pty_master (int);
+ fhandler_pty_master (int, dev_t via = 0);
static DWORD pty_master_thread (const master_thread_param_t *p);
static DWORD pty_master_fwd_thread (const master_fwd_thread_param_t *p);
@@ -2572,7 +2578,9 @@ public:
void copy_from (fhandler_base *x)
{
pc.free_strings ();
+ dev_t via = this->dev_refered_via; /* Do not copy dev_refered_via */
*this = *reinterpret_cast<fhandler_pty_master *> (x);
+ this->dev_refered_via = via;
_copy_from_reset_helper ();
}
--
2.39.0
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] Cygwin: stat(): Fix "Bad address" error on stat() for /dev/tty.
2023-07-07 3:34 ` [PATCH 1/2] Cygwin: stat(): Fix "Bad address" error on stat() " Takashi Yano
@ 2023-07-07 9:46 ` Corinna Vinschen
2023-07-07 22:59 ` Takashi Yano
0 siblings, 1 reply; 8+ messages in thread
From: Corinna Vinschen @ 2023-07-07 9:46 UTC (permalink / raw)
To: cygwin-patches
Hi Takashi,
On Jul 7 12:34, Takashi Yano wrote:
> diff --git a/winsup/cygwin/dtable.cc b/winsup/cygwin/dtable.cc
> index 18e0f3097..2aae2fd65 100644
> --- a/winsup/cygwin/dtable.cc
> +++ b/winsup/cygwin/dtable.cc
> @@ -600,7 +600,13 @@ fh_alloc (path_conv& pc)
> case FH_TTY:
> if (!pc.isopen ())
> {
> - fhraw = cnew_no_ctor (fhandler_console, -1);
> + if (CTTY_IS_VALID (myself->ctty))
> + {
> + if (iscons_dev (myself->ctty))
> + fhraw = cnew_no_ctor (fhandler_console, -1);
> + else
> + fhraw = cnew_no_ctor (fhandler_pty_slave, -1);
> + }
What happens if CTTY_IS_VALID fails at this point? There's no
`else' catching that situation?
> debug_printf ("not called from open for /dev/tty");
> }
> else if (!CTTY_IS_VALID (myself->ctty) && last_tty_dev
Thanks,
Corinna
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] Cygwin: fstat(): Fix st_rdev returned by fstat() for /dev/tty.
2023-07-07 3:34 ` [PATCH 2/2] Cygwin: fstat(): Fix st_rdev returned by fstat() " Takashi Yano
@ 2023-07-07 10:10 ` Corinna Vinschen
2023-07-07 23:01 ` Takashi Yano
0 siblings, 1 reply; 8+ messages in thread
From: Corinna Vinschen @ 2023-07-07 10:10 UTC (permalink / raw)
To: cygwin-patches
On Jul 7 12:34, Takashi Yano wrote:
> While st_rdev returned by fstat() for /dev/tty should be FH_TTY,
> the current cygwin1.dll returns FH_PTYS+minor or FH_CONS+minor.
> Similarly, fstat() does not return correct value for /dev/console,
> /dev/conout, /dev/conin or /dev/ptmx.
>
> This patch fixes the issue by:
> 1) Introduce dev_refered_via in fhandler_termios.
^
dev_referred_via, please
Thanks,
Corinna
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] Cygwin: stat(): Fix "Bad address" error on stat() for /dev/tty.
2023-07-07 9:46 ` Corinna Vinschen
@ 2023-07-07 22:59 ` Takashi Yano
2023-07-10 8:31 ` Corinna Vinschen
0 siblings, 1 reply; 8+ messages in thread
From: Takashi Yano @ 2023-07-07 22:59 UTC (permalink / raw)
To: cygwin-patches
Hi Corinna,
On Fri, 7 Jul 2023 11:46:15 +0200
Corinna Vinschen wrote:
> On Jul 7 12:34, Takashi Yano wrote:
> > diff --git a/winsup/cygwin/dtable.cc b/winsup/cygwin/dtable.cc
> > index 18e0f3097..2aae2fd65 100644
> > --- a/winsup/cygwin/dtable.cc
> > +++ b/winsup/cygwin/dtable.cc
> > @@ -600,7 +600,13 @@ fh_alloc (path_conv& pc)
> > case FH_TTY:
> > if (!pc.isopen ())
> > {
> > - fhraw = cnew_no_ctor (fhandler_console, -1);
> > + if (CTTY_IS_VALID (myself->ctty))
> > + {
> > + if (iscons_dev (myself->ctty))
> > + fhraw = cnew_no_ctor (fhandler_console, -1);
> > + else
> > + fhraw = cnew_no_ctor (fhandler_pty_slave, -1);
> > + }
>
> What happens if CTTY_IS_VALID fails at this point? There's no
> `else' catching that situation?
>
> > debug_printf ("not called from open for /dev/tty");
> > }
> > else if (!CTTY_IS_VALID (myself->ctty) && last_tty_dev
That happens when CTTY is not assigned. In that case fhandler_nodevice
is assigned at:
https://cygwin.com/git/?p=newlib-cygwin.git;a=blob;f=winsup/cygwin/dtable.cc;h=18e0f3097823f00ff9651685be06583818eb2140;hb=e38f91d5a96c4554c69c833243e5afec8e3e90eb#l634
Then fhandler_base::fstat() is called when stat() is called.
--
Takashi Yano <takashi.yano@nifty.ne.jp>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] Cygwin: fstat(): Fix st_rdev returned by fstat() for /dev/tty.
2023-07-07 10:10 ` Corinna Vinschen
@ 2023-07-07 23:01 ` Takashi Yano
0 siblings, 0 replies; 8+ messages in thread
From: Takashi Yano @ 2023-07-07 23:01 UTC (permalink / raw)
To: cygwin-patches
On Fri, 7 Jul 2023 12:10:54 +0200
Corinna Vinschen wrote:
> On Jul 7 12:34, Takashi Yano wrote:
> > While st_rdev returned by fstat() for /dev/tty should be FH_TTY,
> > the current cygwin1.dll returns FH_PTYS+minor or FH_CONS+minor.
> > Similarly, fstat() does not return correct value for /dev/console,
> > /dev/conout, /dev/conin or /dev/ptmx.
> >
> > This patch fixes the issue by:
> > 1) Introduce dev_refered_via in fhandler_termios.
> ^
> dev_referred_via, please
Thansk!
--
Takashi Yano <takashi.yano@nifty.ne.jp>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] Cygwin: stat(): Fix "Bad address" error on stat() for /dev/tty.
2023-07-07 22:59 ` Takashi Yano
@ 2023-07-10 8:31 ` Corinna Vinschen
0 siblings, 0 replies; 8+ messages in thread
From: Corinna Vinschen @ 2023-07-10 8:31 UTC (permalink / raw)
To: cygwin-patches
On Jul 8 07:59, Takashi Yano wrote:
> Hi Corinna,
>
> On Fri, 7 Jul 2023 11:46:15 +0200
> Corinna Vinschen wrote:
> > On Jul 7 12:34, Takashi Yano wrote:
> > > diff --git a/winsup/cygwin/dtable.cc b/winsup/cygwin/dtable.cc
> > > index 18e0f3097..2aae2fd65 100644
> > > --- a/winsup/cygwin/dtable.cc
> > > +++ b/winsup/cygwin/dtable.cc
> > > @@ -600,7 +600,13 @@ fh_alloc (path_conv& pc)
> > > case FH_TTY:
> > > if (!pc.isopen ())
> > > {
> > > - fhraw = cnew_no_ctor (fhandler_console, -1);
> > > + if (CTTY_IS_VALID (myself->ctty))
> > > + {
> > > + if (iscons_dev (myself->ctty))
> > > + fhraw = cnew_no_ctor (fhandler_console, -1);
> > > + else
> > > + fhraw = cnew_no_ctor (fhandler_pty_slave, -1);
> > > + }
> >
> > What happens if CTTY_IS_VALID fails at this point? There's no
> > `else' catching that situation?
> >
> > > debug_printf ("not called from open for /dev/tty");
> > > }
> > > else if (!CTTY_IS_VALID (myself->ctty) && last_tty_dev
>
> That happens when CTTY is not assigned. In that case fhandler_nodevice
> is assigned at:
> https://cygwin.com/git/?p=newlib-cygwin.git;a=blob;f=winsup/cygwin/dtable.cc;h=18e0f3097823f00ff9651685be06583818eb2140;hb=e38f91d5a96c4554c69c833243e5afec8e3e90eb#l634
>
> Then fhandler_base::fstat() is called when stat() is called.
Oh, ok. Sorry, I was a bit puzzled by the missing else.
Please push.
Thanks,
Corinna
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2023-07-10 8:31 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-07-07 3:34 [PATCH 0/2] Fix issues of stat()/fstat() for /dev/tty Takashi Yano
2023-07-07 3:34 ` [PATCH 1/2] Cygwin: stat(): Fix "Bad address" error on stat() " Takashi Yano
2023-07-07 9:46 ` Corinna Vinschen
2023-07-07 22:59 ` Takashi Yano
2023-07-10 8:31 ` Corinna Vinschen
2023-07-07 3:34 ` [PATCH 2/2] Cygwin: fstat(): Fix st_rdev returned by fstat() " Takashi Yano
2023-07-07 10:10 ` Corinna Vinschen
2023-07-07 23:01 ` Takashi Yano
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).