linux-kernel - Re: Ctrl+C doesn't interrupt process waiting for I/O

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <486BF272.2010703@skyrush.com>
Date:	Wed, 02 Jul 2008 16:26:10 -0500
From:	Joe Peterson <joe@...rush.com>
To:	Elias Oltmanns <eo@...ensachen.de>
CC:	Török Edwin <edwintorok@...il.com>,
	Alan Cox <alan@...rguk.ukuu.org.uk>,
	Linux Kernel <linux-kernel@...r.kernel.org>
Subject: Re: Ctrl+C doesn't interrupt process waiting for I/O

Elias Oltmanns wrote:
> -			/*
> -			 * Echo character, and then send the signal.
> -			 * Note that we do not use isig() here because we want
> -			 * the order to be:
> -			 * 1) flush, 2) echo, 3) signal
> -			 */
> -			if (!L_NOFLSH(tty)) {
> -				n_tty_flush_buffer(tty);
> -				tty_driver_flush_buffer(tty);
> -			}
>  			if (L_ECHO(tty))
>  				echo_char(c, tty);
> -			if (tty->pgrp)
> -				kill_pgrp(tty->pgrp, signal, 1);
> +			isig(signal, tty, 0);
>  			return;

I've been doing some experimenting with the order of the three
operations (flush, echo, and signal), and the behavior is slightly
different with each.

The way I have it in the code now matches the order used by FreeBSD, so
there may actually be a good reason to flush the tty buffers *before*
issuing the signal.  Here is their snippet of code:

if (ISSET(lflag, ISIG)) {
    if (CCEQ(cc[VINTR], c) || CCEQ(cc[VQUIT], c)) {
        if (!ISSET(lflag, NOFLSH))
            ttyflush(tp, FREAD | FWRITE);
        ttyecho(c, tp);
        if (tp->t_pgrp != NULL) {
            PGRP_LOCK(tp->t_pgrp);
            pgsignal(tp->t_pgrp,
                     CCEQ(cc[VINTR], c) ? SIGINT : SIGQUIT, 1);
            PGRP_UNLOCK(tp->t_pgrp);
        }
        goto endcase;
    }
    if (CCEQ(cc[VSUSP], c)) {
        if (!ISSET(lflag, NOFLSH))
            ttyflush(tp, FREAD);
        ttyecho(c, tp);
        if (tp->t_pgrp != NULL) {
            PGRP_LOCK(tp->t_pgrp);
            pgsignal(tp->t_pgrp, SIGTSTP, 1);
            PGRP_UNLOCK(tp->t_pgrp);
        }
        goto endcase;
    }
}

The first section handles ^C and ^\ (and flushes read and write), and
the second handles ^Z (only flushes read).

In any case, we should consider if the flush in Linux should precede the
signal.  Perhaps interrupting before the flush can happen is bad?
Perhaps this has something to do with anomalies observed (below) with
other ordering, or maybe I'm seeing other latent bugs not involved with
this at all.


Now to the results of the ordering...

flush, echo, signal (the way it is now)
-------------------
* Follows FreeBSD's ordering
* works on both console and xterm
* seems to delay interrupt when process is IO bound

echo, signal, flush (proposed in Elias' patch)
-------------------
* seems to fix IO bound issue
* echo works in console but not xterm

signal, flush, echo
-------------------
* works in both console and xterm
* may cause late echo (and does not match BSD order)
* I tested inserting an artificial delay between flush and echo:
  strange result: in console, echo does not appear; in xterm,
                  ^C appears right before next prompt, but sometimes
                  echo does not appear, along with final program output
                  (something eats the output)

signal, echo, flush
-------------------
* same as above


So changing the order seems to always introduce some bugs or issues.
I'm still experimenting; feedback welcome!

						-Joe
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/