Hi,
as mentioned earlier, deraadt@ reported that sftp(1) ignores Ctrl-C.
Fixing that without longjmp(3) requires making editline(3) better
behaved.
Currently, when read(2) from the terminal gets interrupted by a
signal, editline(3) ignores the (first) signal and unconditionally
calls read(2) a second time. That seems wrong: if the user hits
Ctrl-C, it is sane to assume that they meant it, not that they
want it ignored.
The following patch causes el_gets(3) and el_wgets(3) to return
failure when read(2)ing from the terminal is interrupted by a
signal other than SIGCONT or SIGWINCH. That allows the calling
program to decide what to do, usually either exit the program or
provide a fresh prompt to the user.
If i know how to grep(1), the following programs in the base system
use -ledit:
* bc(1)
It behaves well with the patch: Ctrl-C discards the current
input line and starts a new input line.
The reason why this already works even without the patch
is that bc(1) does very scary stuff inside the signal handler:
pass a file-global EditLine pointer on the heap to el_line(3)
and access fields inside the returned struct. Needless to
say that no signal handler should do such things...
* cdio(1)
Behaviour is acceptable and unchanged with the patch:
Ctrl-C exits cdio(1).
* ftp(1)
It behaves well with the patch: Ctrl-C discards the current
input line and provides a fresh prompt.
The reason why it already works without the patch is that ftp(1)
uses setjmp(3)/longjmp(3) to forcefully grab back control
from el_gets(3) without waiting for it to return.
* lldb(1)
It misbehaves with or without the patch and ignores Ctrl-C.
I freely admit that i don't feel too enthusiastic about
debugging that beast.
* sftp(1)
Behaviour is improved with the patch: Ctrl-C now exits sftp(1).
If desired, i can supply a very simple follow-up patch to sftp.c
to instead behave like ftp(1) and bc(1), i.e. discard the
current input line and provide a fresh prompt.
Neither doing undue work in the signal handler nor longjmp(3)
will be required for that (if this patch gets committed).
* bgplgsh(8), fsdb(8)
I have no idea how to test those. Does anyone think that testing
either of them would be required?
Regarding the patch below, note that differentiating EINTR behaviour
by signal number is not needed because the calling code in read_char()
already handles SIGCONT and SIGWINCH, so those two never arrive in
read__fixio() in the first place.
Also note that deraadt@ pointed out in private mail to me that the
fact that read__fixio() clears FIONBIO is probably a symptom of
botched editline(3) API design. That might be worth fixing, too,
as far as that is feasible, but it is unrelated to the sftp(1)
Ctrl-C issue; let's address one topic at a time.
Any feedback is welcome.
Yours,
Ingo
P.S.
I decided to Cc: tech@ again because this patch might affect
even people who are not specifically interested in editline(3),
and i have no intention to cause unpleasant surprises.
Index: read.c
===================================================================
RCS file: /cvs/src/lib/libedit/read.c,v
retrieving revision 1.45
diff -u -U15 -r1.45 read.c
--- read.c 9 Aug 2021 09:11:26 -0000 1.45
+++ read.c 9 Aug 2021 10:00:18 -0000
@@ -134,22 +134,19 @@
/* read__fixio():
* Try to recover from a read error
*/
static int
read__fixio(int fd, int e)
{
int zero = 0;
switch (e) {
case EAGAIN:
if (ioctl(fd, FIONBIO, &zero) == -1)
return -1;
return 0;
- case EINTR:
- return 0;
-
default:
return -1;
}
}
