Bug 950 - terminal settings not restored after Ctrl+C during password prompt in scp/sftp
Summary: terminal settings not restored after Ctrl+C during password prompt in scp/sftp
Status: CLOSED FIXED
Alias: None
Product: Portable OpenSSH
Classification: Unclassified
Component: scp (show other bugs)
Version: 3.8.1p1
Hardware: Other Linux
: P2 minor
Assignee: OpenSSH Bugzilla mailing list
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2004-11-05 11:05 AEDT by Pavel Kankovsky
Modified: 2006-10-07 11:37 AEST (History)
1 user (show)

See Also:


Attachments
proposed patch (586 bytes, patch)
2004-11-05 11:08 AEDT, Pavel Kankovsky
no flags Details | Diff
possible fix (647 bytes, patch)
2005-05-06 01:08 AEST, Pavel Kankovsky
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Pavel Kankovsky 2004-11-05 11:05:50 AEDT
It appears there is a race condition between the handler for SIGINT et al. in
scp/sftp and signal handling in readpassphrase(). When I press Ctrl+C during
password prompt, SIGINT kills the parent process (scp/sftp) before the child
process (ssh) exits. This makes the child process orphaned: it loses access to
its controlling terminal, tcsetattr() fails with EIO, and readpassphrase() exits
without restoring the original terminal settings (i.e. ECHO). The problem was
observed on 3.8.1p1 on Linux 2.4.
Comment 1 Pavel Kankovsky 2004-11-05 11:08:20 AEDT
Created attachment 738 [details]
proposed patch

This small patch (for 3.8.1p1 but should apply to the current version as well)
fixes the problem.
Comment 2 Darren Tucker 2005-01-20 22:36:11 AEDT
Comment on attachment 738 [details]
proposed patch

Does this need to go to OpenBSD?  It doesn't seem to occur on OpenBSD, but I
don't know if it's just because the dice haven't landed right or something with
the tty semantics (or process scheduling?) causes it to not happen.
Comment 3 Pavel Kankovsky 2005-01-20 23:35:17 AEDT
It appears BSD unlike POSIX/SUS does not revoke access to the controlling
terminal from orphaned processes. This makes the race condition almost
irrelevant on BSD but it does not make it completely irrelevant IMHO.

Consider this scenario (unlikely but possible):
1. scp (the parent) exits
2. ssh (the child) gets SIGINT but blocks for whatever reason (e.g. swapping)
3. the grandparent sees scp has exited and starts a new process
4. the new process turns tty echo off
5. ssh wakes up, turns tty echo on in order to clean up after itself, and exits
6. the new process resumes, unaware echo has been turned on behind its back
(I suspect POSIX demands the revocation of tty access from orphans to make that
impossible.)
Comment 4 Darren Tucker 2005-01-24 22:02:14 AEDT
The patch has been applied to both OpenBSD and -Portable.  Thanks.
Comment 5 Pavel Kankovsky 2005-05-06 01:06:29 AEST
Damn it! There is another nasty subtle race condition there: kill() from the
parent (e.g. scp) can interrupt tcsetattr() in the child (ssh), make it fail
with EINTR (both POSIX and OpenBSD documentation says tcsetattr() can be
interrupted by a signal) and leave the tty in a broken state. Observed on
3.8.1p1 on Linux 2.4 on a SMP machine (well, it was not a true SMP box, it was a
single Xeon in HT mode). As far as I can tell, every version using OpenBSD
readpassphrase() up to the current snapshot is affected.
Comment 6 Pavel Kankovsky 2005-05-06 01:08:54 AEST
Created attachment 898 [details]
possible fix

This patch makes readpassphrase() retry tcsetattr() when it fails with EINTR.
It appears to solve the problem.
Comment 7 Damien Miller 2005-05-24 16:01:07 AEST
Applied to -portable and submitted to OpenBSD libc.
Comment 8 Darren Tucker 2006-10-07 11:37:51 AEST
Change all RESOLVED bug to CLOSED with the exception of the ones fixed post-4.4.