ACK: [SRU][Trusty][PATCH 1/1] n_tty: fix EXTPROC vs ICANON interaction with TIOCINQ (aka FIONREAD)

Colin Ian King colin.king at canonical.com
Fri Nov 9 17:07:21 UTC 2018


On 09/11/2018 17:04, Kleber Sacilotto de Souza wrote:
> From: Linus Torvalds <torvalds at linux-foundation.org>
> 
> We added support for EXTPROC back in 2010 in commit 26df6d13406d ("tty:
> Add EXTPROC support for LINEMODE") and the intent was to allow it to
> override some (all?) ICANON behavior.  Quoting from that original commit
> message:
> 
>          There is a new bit in the termios local flag word, EXTPROC.
>          When this bit is set, several aspects of the terminal driver
>          are disabled.  Input line editing, character echo, and mapping
>          of signals are all disabled.  This allows the telnetd to turn
>          off these functions when in linemode, but still keep track of
>          what state the user wants the terminal to be in.
> 
> but the problem turns out that "several aspects of the terminal driver
> are disabled" is a bit ambiguous, and you can really confuse the n_tty
> layer by setting EXTPROC and then causing some of the ICANON invariants
> to no longer be maintained.
> 
> This fixes at least one such case (TIOCINQ) becoming unhappy because of
> the confusion over whether ICANON really means ICANON when EXTPROC is set.
> 
> This basically makes TIOCINQ match the case of read: if EXTPROC is set,
> we ignore ICANON.  Also, make sure to reset the ICANON state ie EXTPROC
> changes, not just if ICANON changes.
> 
> Fixes: 26df6d13406d ("tty: Add EXTPROC support for LINEMODE")
> Reported-by: Tetsuo Handa <penguin-kernel at i-love.sakura.ne.jp>
> Reported-by: syzkaller <syzkaller at googlegroups.com>
> Cc: Jiri Slaby <jslaby at suse.com>
> Signed-off-by: Linus Torvalds <torvalds at linux-foundation.org>
> Signed-off-by: Greg Kroah-Hartman <gregkh at linuxfoundation.org>
> 
> CVE-2018-18386
> (cherry picked from commit 966031f340185eddd05affcf72b740549f056348)
> Signed-off-by: Kleber Sacilotto de Souza <kleber.souza at canonical.com>
> ---
>  drivers/tty/n_tty.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
> index 8a13b3372804..38bf1d5230d0 100644
> --- a/drivers/tty/n_tty.c
> +++ b/drivers/tty/n_tty.c
> @@ -1811,7 +1811,7 @@ static void n_tty_set_termios(struct tty_struct *tty, struct ktermios *old)
>  {
>  	struct n_tty_data *ldata = tty->disc_data;
>  
> -	if (!old || (old->c_lflag ^ tty->termios.c_lflag) & ICANON) {
> +	if (!old || (old->c_lflag ^ tty->termios.c_lflag) & (ICANON | EXTPROC)) {
>  		bitmap_zero(ldata->read_flags, N_TTY_BUF_SIZE);
>  		ldata->line_start = ldata->read_tail;
>  		if (!L_ICANON(tty) || !read_cnt(ldata)) {
> @@ -2526,7 +2526,7 @@ static int n_tty_ioctl(struct tty_struct *tty, struct file *file,
>  		return put_user(tty_chars_in_buffer(tty), (int __user *) arg);
>  	case TIOCINQ:
>  		down_write(&tty->termios_rwsem);
> -		if (L_ICANON(tty))
> +		if (L_ICANON(tty) && !L_EXTPROC(tty))
>  			retval = inq_canon(ldata);
>  		else
>  			retval = read_cnt(ldata);
> 

Clean cherry pick. Been upstream for a while, so..

Acked-by: Colin Ian King <colin.king at canonical.com>



More information about the kernel-team mailing list