Hello, is this the right place to send a patch for st (simple
terminal)?  This is the first time for me to submit a patch to
suckless.org.  I'm not familiar with the development flow of suckless
software.  If I should have completed something before submitting the
patch (for example, opening a discussion in another mailing list
[email protected] in prior to sending the patch), please feel free to
tell me that.  I'd be happy to follow the standard procedure.

Problem:

  When st is started with fd 0, 1, and 2 being closed, some of the
  standard streams (file descriptors 0, 1, and 2) are closed in the
  child process (i.e., the shell process).

Description:

  In the current implementation, the slave PTY (assigned to the
  variable `s') is always closed after duplicating it to file
  descriptors of standard streams (0, 1, and 2).  However, when the
  allocated slave PTY `s' is already one of 0, 1, or 2, this causes
  the unexpected closing of a standard stream.  The same problem
  occurs when the file descriptor of the master PTY (the variable `m')
  is one of 0, 1, or 2.

Repeat-By:

  The problem can be reproduced by e.g. starting `st' with file
  descriptors 0, 1, and 2 being closed:

  $ st 0<&- 1>&- 2>&-

  Then in the opened `st' window,

  $ echo hello[RET]

  will produce the following error messages from Bash (when the shell
  is Bash):

  bash: echo: write error: Bad file descriptor
  bash: echo: write error: Bad file descriptor

  This is because the standard error output (fd 2) is unexpectedly
  closed.

Fix:

  I attach a patch file:

  - st-DontCloseStandardStreamsUnexpectedly-20210819-2ec571a.diff

  In this patch, the original master PTY (m) is closed before it would
  be overwritten by duplicated slave PTYs.  The original slave PTY (s)
  is closed only when it is not one of the standard streams.  Here's
  also the inline copy of the patch (though my email client breaks
  the whitespaces):

------------------------------------------------------------------------
>From 9bcc379b7fa8ada0bdd2b2f7ae8c7ce5bb712ce7 Mon Sep 17 00:00:00 2001
From: Koichi Murase <[email protected]>
Date: Thu, 19 Aug 2021 16:01:48 +0900
Subject: [st][PATCH] fix a problem that the standard streams are
 unexpectedly closed

In the current implementation, the slave PTY (assigned to the variable
`s') is always closed after duplicating it to file descriptors of
standard streams (0, 1, and 2).  However, when the allocated slave PTY
`s' is already one of 0, 1, or 2, this causes the unexpected closing
of a standard stream.  The same problem occurs when the file
descriptor of the master PTY (the variable `m') is one of 0, 1, or 2.

The problem can be reproduced by e.g. starting `st' with file
descriptors 0, 1, and 2 being closed:

  $ st 0<&- 1>&- 2>&-

Then in the opened `st' window,

  $ echo hello[RET]

will produce the following error messages from Bash (when the shell is
Bash):

  bash: echo: write error: Bad file descriptor
  bash: echo: write error: Bad file descriptor

In this patch, the original master PTY (m) is closed before it would
be overwritten by duplicated slave PTYs.  The original slave PTY (s)
is closed only when it is not one of the standard streams.
---
 st.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/st.c b/st.c
index ebdf360..a9338e1 100644
--- a/st.c
+++ b/st.c
@@ -793,14 +793,15 @@ ttynew(const char *line, char *cmd, const char
*out, char **args)
  break;
  case 0:
  close(iofd);
+ close(m);
  setsid(); /* create a new process group */
  dup2(s, 0);
  dup2(s, 1);
  dup2(s, 2);
  if (ioctl(s, TIOCSCTTY, NULL) < 0)
  die("ioctl TIOCSCTTY failed: %s\n", strerror(errno));
- close(s);
- close(m);
+ if (s > 2)
+ close(s);
 #ifdef __OpenBSD__
  if (pledge("stdio getpw proc exec", NULL) == -1)
  die("pledge\n");
-- 
2.21.3

------------------------------------------------------------------------

Thank you.

--
Koichi

Reply via email to