portable sftp oddity: sftp, redirection of stderr and ControPersist

[Nick Holland]

On 07/11/16 04:06, Jakub Jelen wrote:
> On 07/07/2016 09:07 PM, Nick Holland wrote:
...
> This looks like related to the bug #1988 [1]. The mux process is holding 
> the stderr. Could you test it with the patch from the referenced bug 
> applied?
> Also as I see you are using RHEL, you can contact your support to 
> provide a test package (we plan to fix this in RHEL7.3).
> 
> [1] https://bugzilla.mindrot.org/show_bug.cgi?id=1988
> 

well, RHEL is what I reproduced the error on, the actual problem cropped up
in an AIX project.  Just not too easy to test AIX if you don't have power-
hungry, outrageously expensive hw on hand.  :-/

So.  I loaded up a CentOS 7.2 VM, downloaded the most recent OpenSSH-portable
tar file (openssh-7.2p2.tar.gz), added zlib-devel and openssl-devel,
./configure, make, make install'd the program.  pkill'ed sshd, 
/usr/local/sbin/sshd to start sshd....and same results.

I then applied this patch:

diff --git a/log.c b/log.c
index a5ba22f..9011f9d 100644
--- a/log.c
+++ b/log.c
@@ -308,7 +308,7 @@ log_change_level(LogLevel new_log_level)
 int
 log_is_on_stderr(void)
 {
-	return log_on_stderr;
+	return log_on_stderr && log_stderr_fd == STDERR_FILENO;
 }
 
 /* redirect what would usually get written to stderr to specified file */
diff --git a/ssh.c b/ssh.c
index ec8a515..65e0217 100644
--- a/ssh.c
+++ b/ssh.c
@@ -1346,7 +1346,7 @@ static void
 control_persist_detach(void)
 {
 	pid_t pid;
-	int devnull;
+	int devnull, keep_stderr;
 
 	debug("%s: backgrounding master process", __func__);
 
@@ -1377,8 +1377,10 @@ control_persist_detach(void)
 		error("%s: open(\"/dev/null\"): %s", __func__,
 		    strerror(errno));
 	} else {
+		keep_stderr = log_is_on_stderr() && debug_flag;
 		if (dup2(devnull, STDIN_FILENO) == -1 ||
-		    dup2(devnull, STDOUT_FILENO) == -1)
+		    dup2(devnull, STDOUT_FILENO) == -1 ||
+		    (!keep_stderr && dup2(devnull, STDERR_FILENO) == -1))
 			error("%s: dup2: %s", __func__, strerror(errno));
 		if (devnull > STDERR_FILENO)
 			close(devnull);

and seems to work, at least once I modified my script to use the sftp I had compiled in /usr/local/bin rather than the "factory" /usr/bin (oops).

my test script:
====
#!/bin/bash

mkdir -p ~/dest

for X in 1 2 3 4 5 6 7 8 9 10; do
     echo $X
     echo "cd dest
     put $0" | /usr/local/bin/sftp -b - localhost 2>&1 | tee -a outfile
done
====
So yes, looks like there's a fix on the way.  Thanks!

Nick.