Debian Bug report logs - #12261
xterm: closes its pty too early

version graph

Package: xterm; Maintainer for xterm is Debian X Strike Force <[email protected]>; Source for xterm is src:xterm (PTS, buildd, popcon).

Reported by: [email protected] (Richard Braakman)

Date: Sun, 24 Aug 1997 22:33:00 UTC

Severity: normal

Tags: upstream

Fixed in version xterm/226-1

Done: Julien Cristau <[email protected]>

Bug is archived. No further changes may be made.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to [email protected], [email protected] (Mark W. Eichin):
Bug#12261; Package xbase. (full text, mbox, link).


Acknowledgement sent to [email protected] (Richard Braakman):
New bug report received and forwarded. Copy sent to [email protected] (Mark W. Eichin). (full text, mbox, link).


Message #5 received at [email protected] (full text, mbox, reply):

From: [email protected] (Richard Braakman)
To: [email protected]
Subject: xterm closes its pty too early
Date: Mon, 25 Aug 1997 00:33:58 +0200 (CEST)
Package: xbase

In main.c:Exit(), xterm has this bit of code:

| #ifndef AMOEBA
|         close(term->screen.respond); /* close explicitly to avoid race with slave side */
| #endif
| #ifdef ALLOWLOGGING
|         if(term->screen.logging)
|                 CloseLog(&term->screen);
| #endif
| 
| #ifndef AMOEBA
|         if (!am_slave) {
|                 /* restore ownership of tty and pty */
|                 chown (ttydev, 0, 0);
| #if (!defined(__sgi) && !defined(__osf__))
|                 chown (ptydev, 0, 0);
| #endif
| 
|                 /* restore modes of tty and pty */
|                 chmod (ttydev, 0666);
| #if (!defined(__sgi) && !defined(__osf__))
|                 chmod (ptydev, 0666);
| #endif
|         }
| #endif /* AMOEBA */

The variable term->screen.respond contains the file descriptor for the
pty device that corresponds to the filename in ptydev.  The variable
ttydev contains the filename of the tty device that corresponds to the
pty device.

Between the close(term->screen.respond) and the chown and chmod calls,
there is a window in which another process can open the now-freed pty,
set the appropriate ownership and permissions on the corresponding tty
device, and start using it.  Then xterm comes along and sets the tty
to world readable and writeable :)

Exchanging the close and the chmod calls will close that window, but
open another.  It will become possible for another process to jump in
and open the tty, then write to xterm's child process via that
descriptor.

I don't see a solution here.  Please refer to bugs #7112 and #988 for
similarly thorny pseudotty problems.


Information forwarded to [email protected], [email protected] (Mark W. Eichin):
Bug#12261; Package xbase. (full text, mbox, link).


Acknowledgement sent to [email protected] (Richard Braakman):
Extra info received and forwarded to list. Copy sent to [email protected] (Mark W. Eichin). (full text, mbox, link).


Message #10 received at [email protected] (full text, mbox, reply):

From: [email protected] (Richard Braakman)
To: [email protected]
Subject: xterm closes its pty too early (more info)
Date: Thu, 28 Aug 1997 14:56:56 +0200 (CEST)
>Exchanging the close and the chmod calls will close that window, but
>open another.  It will become possible for another process to jump in
>and open the tty, then write to xterm's child process via that
>descriptor.

I was mistaken here.  A process can jump in and open the tty, but the
worst it can do is write to the tty (which will show up on the input
to xterm's pty, which it will ignore because it is exiting), or read
from the tty (which will grab characters that were on their way from
xterm to the child process).

The latter can be a problem in some cases, but it can be taken care 
of by flushing the pty output buffer, which can be done with the
ioctl TCFLSH, argument TCOFLUSH.  (See linux kernel sources,
drivers/char/tty_ioctl.c and drivers/char/pty.c).

Thus, the close(term->screen.respond) call can safely be replaced with
a call to that ioctl, and the pty can be explicitly closed after the
chown/chmod.

(The libpty I am developing, which I hope xterm will use, will encapsulate
 this in its close_pty() function.)

Richard Braakman


Bug reassigned from package `xbase' to `xterm'. Request was from Branden Robinson <[email protected]> to [email protected]. (full text, mbox, link).


Changed Bug title. Request was from [email protected] (Branden Robinson) to [email protected]. (full text, mbox, link).


Changed Bug title. Request was from Branden Robinson <[email protected]> to [email protected]. (full text, mbox, link).


Tags added: upstream Request was from Branden Robinson <[email protected]> to [email protected]. (full text, mbox, link).


Information forwarded to [email protected], Debian X Strike Force <[email protected]>:
Bug#12261; Package xterm. (full text, mbox, link).


Acknowledgement sent to Nathanael Nerode <[email protected]>:
Extra info received and forwarded to list. Copy sent to Debian X Strike Force <[email protected]>. (full text, mbox, link).


Message #23 received at [email protected] (full text, mbox, reply):

From: Nathanael Nerode <[email protected]>
To: [email protected]
Subject: Followup, possible patch for second oldest bug in xterm
Date: Tue, 10 Apr 2007 05:01:11 -0400
This bug still exists in current xterm, as far as I can tell.

The following untested patch implements Richard Braakman's suggestion from 1997,
slightly more portably.

Open questions:
(1) Where should the logging be closed?
(2) Is any additional portability goo needed to send this upstream (e.g. for 
systems without termios.h)?
(3) Is anyone interested in testing it?

I assign these changes to the public domain, since they're trivial.

--- main.c	2007-04-10 04:39:13.000000000 -0400
+++ main.c.new	2007-04-10 04:56:45.000000000 -0400
@@ -4613,11 +4613,9 @@
 #endif /* USE_SYSV_UTMP */
 #endif /* HAVE_UTMP */
 
-    close(screen->respond);	/* close explicitly to avoid race with slave side */
-#ifdef ALLOWLOGGING
-    if (screen->logging)
-	CloseLog(screen);
-#endif
+    /* Flush pending data before releasing ownership, so nobody else can
+       grab it */
+    tcflush(screen->respond, TCOFLUSH);
 
     if (am_slave < 0) {
 	/* restore ownership of tty and pty */
@@ -4625,6 +4623,16 @@
 #if (defined(USE_PTY_DEVICE) && !defined(__sgi) && !defined(__hpux))
 	set_owner(ptydev, 0, 0, 0666U);
 #endif
+
+   /* Close after releasing ownership to avoid race condition: other programs 
+      grabbing it, and *then* having us release ownership... */
+
+    close(screen->respond);	/* close explicitly to avoid race with slave side */
+#ifdef ALLOWLOGGING
+    if (screen->logging)
+	CloseLog(screen);
+#endif
+
     }
 #ifdef NO_LEAKS
     if (n == 0) {

-- 
Nathanael Nerode  <[email protected]>

This space intentionally left blank.




Information forwarded to [email protected], Debian X Strike Force <[email protected]>:
Bug#12261; Package xterm. (full text, mbox, link).


Acknowledgement sent to Thomas Dickey <[email protected]>:
Extra info received and forwarded to list. Copy sent to Debian X Strike Force <[email protected]>. (full text, mbox, link).


Message #28 received at [email protected] (full text, mbox, reply):

From: Thomas Dickey <[email protected]>
To: Nathanael Nerode <[email protected]>, [email protected]
Subject: Re: Bug#12261: Followup, possible patch for second oldest bug in xterm
Date: Tue, 10 Apr 2007 06:40:00 -0400
[Message part 1 (text/plain, inline)]
On Tue, Apr 10, 2007 at 11:10:47AM +0200, Nathanael Nerode wrote:
> This bug still exists in current xterm, as far as I can tell.

I might have seen this before, but see that it's against "xbase" rather
than "xterm" - so perhaps I didn't see it...
 
> The following untested patch implements Richard Braakman's suggestion from 1997,
> slightly more portably.
> 
> Open questions:
> (1) Where should the logging be closed?
> (2) Is any additional portability goo needed to send this upstream (e.g. for 
> systems without termios.h)?

I'll take a closer look (thanks)

-- 
Thomas E. Dickey
http://invisible-island.net
ftp://invisible-island.net
[Message part 2 (application/pgp-signature, inline)]

Reply sent to Julien Cristau <[email protected]>:
You have taken responsibility. (full text, mbox, link).


Notification sent to [email protected] (Richard Braakman):
Bug acknowledged by developer. (full text, mbox, link).


Message #33 received at [email protected] (full text, mbox, reply):

From: Julien Cristau <[email protected]>
To: [email protected]
Subject: Bug#12261: fixed in xterm 226-1
Date: Mon, 18 Jun 2007 13:47:09 +0000
Source: xterm
Source-Version: 226-1

We believe that the bug you reported is fixed in the latest version of
xterm, which is due to be installed in the Debian FTP archive:

xterm_226-1.diff.gz
  to pool/main/x/xterm/xterm_226-1.diff.gz
xterm_226-1.dsc
  to pool/main/x/xterm/xterm_226-1.dsc
xterm_226-1_i386.deb
  to pool/main/x/xterm/xterm_226-1_i386.deb
xterm_226.orig.tar.gz
  to pool/main/x/xterm/xterm_226.orig.tar.gz



A summary of the changes between this version and the previous one is
attached.

Thank you for reporting the bug, which will now be closed.  If you
have further comments please address them to [email protected],
and the maintainer will reopen the bug report if appropriate.

Debian distribution maintenance software
pp.
Julien Cristau <[email protected]> (supplier of updated xterm package)

(This message was generated automatically at their request; if you
believe that there is a problem with it please contact the archive
administrators by mailing [email protected])


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Format: 1.7
Date: Mon, 18 Jun 2007 14:02:22 +0100
Source: xterm
Binary: xterm
Architecture: source i386
Version: 226-1
Distribution: unstable
Urgency: low
Maintainer: Debian X Strike Force <[email protected]>
Changed-By: Julien Cristau <[email protected]>
Description: 
 xterm      - X terminal emulator
Closes: 12261 349142 418324 420974 421523 422521 426364 426863
Changes: 
 xterm (226-1) unstable; urgency=low
 .
   [ Branden Robinson ]
   * Remove debian/NEWS; the events it attested to (like the "upcoming 7.0
     modularization" are no longer news, and are in the past.
 .
   [ Julien Cristau ]
   * Configure with --with-tty-group=tty, to prevent security problems in case
     of buggy build environment (closes: #349142).
   * New upstream release.
     + fix  an  infinite  loop  when  showing  a  2-column character in a
       1-column screen (closes: #426863).
     + add  XF86Paste  and  SunPaste  to the default translations
       (closes: #422521, patch by Bernhard R Link).
     + improve  permissions  logic  when  closing pseudo-terminal
       (closes: #12261, patch by Nathanael Nerode, analysis by Richard
       Braakman).
     + add  a check in case someone tries to call the popup-menu() action
       on a menu which is not initialized (closes: #426364).
     + fix error-checking on internal font switching for "Selection" menu
       entry (closes: #421523).
     + amend select/paste change from patch #225 by limiting it to
       non-UTF-8/non-KOI8-R encoding (closes: #420974).
     + add  workaround  for  groff  ".URL" codes which are not present in
       some commonly-used bitmap fonts (closes: #418324).
   * Update reference to xlibs-data in xterm's description, refer to xbitmaps
     instead.
   * Build-depend on desktop-file-utils to install the new desktop files for
     xterm and uxterm, and change debian/rules and debian/xterm.install to
     install these files and the icons.
Files: 
 104129b8aabcf3498c64be24716de973 825 x11 optional xterm_226-1.dsc
 93d1f43ac3c13af86c598493f14a36f6 835862 x11 optional xterm_226.orig.tar.gz
 57ae2778e0a806deb01a41cc6fee0292 61616 x11 optional xterm_226-1.diff.gz
 94f81ecbb70c7cf120552b2a5a4bdeb2 452080 x11 optional xterm_226-1_i386.deb

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)

iD8DBQFGdovGmEvTgKxfcAwRAhbbAKCEL0Aq2kLgqYfTaeThWox3p9KYNwCePjvT
hGOcCnN7IKcs8dgUyCxOLVc=
=U44l
-----END PGP SIGNATURE-----




Bug archived. Request was from Debbugs Internal Request <[email protected]> to [email protected]. (Sat, 28 Jul 2007 07:29:56 GMT) (full text, mbox, link).


Send a report that this bug log contains spam.


Debian bug tracking system administrator <[email protected]>. Last modified: Fri Jun 6 02:02:02 2025; Machine Name: bembo

Debian Bug tracking system

Debbugs is free software and licensed under the terms of the GNU General Public License version 2. The current version can be obtained from https://bugs.debian.org/debbugs-source/.

Copyright © 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson, 2005-2017 Don Armstrong, and many other contributors.