linux-kernel - Re: [PATCH 5/6] n_tty: Fix stuck write wakeup

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: <20151213151823.GB10204@sig21.net>
Date:	Sun, 13 Dec 2015 16:18:23 +0100
From:	Johannes Stezenbach <js@...21.net>
To:	Peter Hurley <peter@...leysoftware.com>
Cc:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Jiri Slaby <jslaby@...e.cz>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 5/6] n_tty: Fix stuck write wakeup

Hi Peter,

On Sat, Dec 12, 2015 at 02:16:38PM -0800, Peter Hurley wrote:
> If signal-driven i/o is disabled while write wakeup is pending (ie.,
> n_tty_write() has set TTY_DO_WRITE_WAKEUP but then signal-driven i/o
> is disabled), the TTY_DO_WRITE_WAKEUP bit will never be cleared and
> will cause tty_wakeup() to always call n_tty_write_wakeup.
> 
> Unconditionally clear the write wakeup, and since kill_fasync()
> already checks if the fasync ptr is null, call kill_fasync()
> unconditionally as well.
...
> @@ -230,8 +230,8 @@ static ssize_t chars_in_buffer(struct tty_struct *tty)
>  
>  static void n_tty_write_wakeup(struct tty_struct *tty)
>  {
> -	if (tty->fasync && test_and_clear_bit(TTY_DO_WRITE_WAKEUP, &tty->flags))
> -		kill_fasync(&tty->fasync, SIGIO, POLL_OUT);
> +	clear_bit(TTY_DO_WRITE_WAKEUP, &tty->flags);
> +	kill_fasync(&tty->fasync, SIGIO, POLL_OUT);
>  }

There is a related bug that I meant to send a patch, but I
never got around because the issue was found with proprietary
userspace and ancient kernel.  Maybe you could take care of it?
The patch might not apply cleanly after your recent changes
or might even be invalid now, please check.

Thanks,
Johannes


---
tty: n_tty: fix SIGIO for output

According to fcntl(2), "a SIGIO signal is sent whenever input
or output becomes possible on that file descriptor", i.e.
after the output buffer was full and now has space for new data.
But in fact SIGIO is sent after every write.

n_tty_write() should set TTY_DO_WRITE_WAKEUP only when
not all data could be written to the buffer.

Signed-off-by: Johannes Stezenbach <js@...21.net>

--- drivers/char/n_tty.c.orig   2015-11-02 22:26:04.124227148 +0100
+++ drivers/char/n_tty.c        2015-11-02 22:26:10.644212115 +0100
@@ -1925,6 +1925,7 @@ static ssize_t n_tty_write(struct tty_st
        DECLARE_WAITQUEUE(wait, current);
        int c;
        ssize_t retval = 0;
+       size_t count = nr;

        /* Job control check -- must be done at start (POSIX.1 7.1.1.4). */
        if (L_TOSTOP(tty) && file->f_op->write != redirected_tty_write) {
@@ -1991,7 +1992,7 @@ static ssize_t n_tty_write(struct tty_st
 break_out:
        __set_current_state(TASK_RUNNING);
        remove_wait_queue(&tty->write_wait, &wait);
-       if (b - buf != nr && tty->fasync)
+       if (b - buf != count && tty->fasync)
                set_bit(TTY_DO_WRITE_WAKEUP, &tty->flags);
        return (b - buf) ? b - buf : retval;
 }


--
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/