Hello,
Please consider including upstream commit fb5ef9e7da39968fec6d6f37f20a23d23740c75e
in the next v3.14.y, 3.16.y-ckt, 3.18.y, and 3.19.y stable releases. It was
included upstream as of v4.0-rc1. It has been tested and confirmed to resolve
http://bugs.launchpad.net/bugs/1381005 . This bug was introduced in v3.12-rc1 when
commit 24a89d1 was applied. This commit is not needed in stable kernels prior to
v3.12.y.
commit fb5ef9e7da39968fec6d6f37f20a23d23740c75e
Author: Peter Hurley <[email protected]>
Date: Fri Jan 16 15:05:39 2015 -0500
n_tty: Fix read buffer overwrite when no newline
This commit does not apply cleanly to the kernel versions mentined, so I performed
a backport, which is in email 1/1.
The 3.12.y and 3.13.y-ckt upstrem kernels can't use the cherry pick of fb5ef9e7
becuase smp_load_acquire was not introduced until 3.14-rc1. I'll look into
alternative way that this bug could be fixed in 3.12.y and 3.13.y-ckt.
Sincerely,
Joseph Salisbury
Peter Hurley (1):
n_tty: Fix read buffer overwrite when no newline
drivers/tty/n_tty.c | 108 +++++++++++++++++++++++++++++++++++++++-------------
1 file changed, 81 insertions(+), 27 deletions(-)
--
2.1.0
From: Peter Hurley <[email protected]>
BugLink: http://bugs.launchpad.net/bugs/1381005
In canon mode, the read buffer head will advance over the buffer tail
if the input > 4095 bytes without receiving a line termination char.
Discard additional input until a line termination is received.
Before evaluating for overflow, the 'room' value is normalized for
I_PARMRK and 1 byte is reserved for line termination (even in !icanon
mode, in case the mode is switched). The following table shows the
transform:
actual buffer | 'room' value before overflow calc
space avail | !I_PARMRK | I_PARMRK
--------------------------------------------------
0 | -1 | -1
1 | 0 | 0
2 | 1 | 0
3 | 2 | 0
4+ | 3 | 1
When !icanon or when icanon and the read buffer contains newlines,
normalized 'room' values of -1 and 0 are clamped to 0, and
'overflow' is 0, so read_head is not adjusted and the input i/o loop
exits (setting no_room if called from flush_to_ldisc()). No input
is discarded since the reader does have input available to read
which ensures forward progress.
When icanon and the read buffer does not contain newlines and the
normalized 'room' value is 0, then overflow and room are reset to 1,
so that the i/o loop will process the next input char normally
(except for parity errors which are ignored). Thus, erasures, signalling
chars, 7-bit mode, etc. will continue to be handled properly.
If the input char processed was not a line termination char, then
the canon_head index will not have advanced, so the normalized 'room'
value will now be -1 and 'overflow' will be set, which indicates the
read_head can safely be reset, effectively erasing the last char
processed.
If the input char processed was a line termination, then the
canon_head index will have advanced, so 'overflow' is cleared to 0,
the read_head is not reset, and 'room' is cleared to 0, which exits
the i/o loop (because the reader now have input available to read
which ensures forward progress).
Note that it is possible for a line termination to be received, and
for the reader to copy the line to the user buffer before the
input i/o loop is ready to process the next input char. This is
why the i/o loop recomputes the room/overflow state with every
input char while handling overflow.
Finally, if the input data was processed without receiving
a line termination (so that overflow is still set), the pty
driver must receive a write wakeup. A pty writer may be waiting
to write more data in n_tty_write() but without unthrottling
here that wakeup will not arrive, and forward progress will halt.
(Normally, the pty writer is woken when the reader reads data out
of the buffer and more space become available).
Signed-off-by: Peter Hurley <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
(backported from commit fb5ef9e7da39968fec6d6f37f20a23d23740c75e)
Signed-off-by: Joseph Salisbury <[email protected]>
---
drivers/tty/n_tty.c | 108 +++++++++++++++++++++++++++++++++++++++-------------
1 file changed, 81 insertions(+), 27 deletions(-)
diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index 4ddfa60..f190136 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -247,8 +247,8 @@ static void n_tty_write_wakeup(struct tty_struct *tty)
static void n_tty_check_throttle(struct tty_struct *tty)
{
- if (tty->driver->type == TTY_DRIVER_TYPE_PTY)
- return;
+ struct n_tty_data *ldata = tty->disc_data;
+
/*
* Check the remaining room for the input canonicalization
* mode. We don't want to throttle the driver if we're in
@@ -1512,23 +1512,6 @@ n_tty_receive_char_lnext(struct tty_struct *tty, unsigned char c, char flag)
n_tty_receive_char_flagged(tty, c, flag);
}
-/**
- * n_tty_receive_buf - data receive
- * @tty: terminal device
- * @cp: buffer
- * @fp: flag buffer
- * @count: characters
- *
- * Called by the terminal driver when a block of characters has
- * been received. This function must be called from soft contexts
- * not from interrupt context. The driver is responsible for making
- * calls one at a time and in order (or using flush_to_ldisc)
- *
- * n_tty_receive_buf()/producer path:
- * claims non-exclusive termios_rwsem
- * publishes read_head and canon_head
- */
-
static void
n_tty_receive_buf_real_raw(struct tty_struct *tty, const unsigned char *cp,
char *fp, int count)
@@ -1684,24 +1667,85 @@ static void __receive_buf(struct tty_struct *tty, const unsigned char *cp,
}
}
+/**
+ * n_tty_receive_buf_common - process input
+ * @tty: device to receive input
+ * @cp: input chars
+ * @fp: flags for each char (if NULL, all chars are TTY_NORMAL)
+ * @count: number of input chars in @cp
+ *
+ * Called by the terminal driver when a block of characters has
+ * been received. This function must be called from soft contexts
+ * not from interrupt context. The driver is responsible for making
+ * calls one at a time and in order (or using flush_to_ldisc)
+ *
+ * Returns the # of input chars from @cp which were processed.
+ *
+ * In canonical mode, the maximum line length is 4096 chars (including
+ * the line termination char); lines longer than 4096 chars are
+ * truncated. After 4095 chars, input data is still processed but
+ * not stored. Overflow processing ensures the tty can always
+ * receive more input until at least one line can be read.
+ *
+ * In non-canonical mode, the read buffer will only accept 4095 chars;
+ * this provides the necessary space for a newline char if the input
+ * mode is switched to canonical.
+ *
+ * Note it is possible for the read buffer to _contain_ 4096 chars
+ * in non-canonical mode: the read buffer could already contain the
+ * maximum canon line of 4096 chars when the mode is switched to
+ * non-canonical.
+ *
+ * n_tty_receive_buf()/producer path:
+ * claims non-exclusive termios_rwsem
+ * publishes commit_head or canon_head
+ */
static int
n_tty_receive_buf_common(struct tty_struct *tty, const unsigned char *cp,
char *fp, int count, int flow)
{
struct n_tty_data *ldata = tty->disc_data;
- int room, n, rcvd = 0;
+ int room, n, rcvd = 0, overflow;
down_read(&tty->termios_rwsem);
while (1) {
- room = receive_room(tty);
+ /*
+ * When PARMRK is set, each input char may take up to 3 chars
+ * in the read buf; reduce the buffer space avail by 3x
+ *
+ * If we are doing input canonicalization, and there are no
+ * pending newlines, let characters through without limit, so
+ * that erase characters will be handled. Other excess
+ * characters will be beeped.
+ *
+ * paired with store in *_copy_from_read_buf() -- guarantees
+ * the consumer has loaded the data in read_buf up to the new
+ * read_tail (so this producer will not overwrite unread data)
+ */
+ size_t tail = smp_load_acquire(&ldata->read_tail);
+
+ room = N_TTY_BUF_SIZE - (ldata->read_head - tail);
+ if (I_PARMRK(tty))
+ room = (room + 2) / 3;
+ room--;
+ if (room <= 0) {
+ overflow = ldata->icanon && ldata->canon_head == tail;
+ if (overflow && room < 0)
+ ldata->read_head--;
+ room = overflow;
+ ldata->no_room = flow && !room;
+ } else
+ overflow = 0;
+
n = min(count, room);
- if (!n) {
- if (flow && !room)
- ldata->no_room = 1;
+ if (!n)
break;
- }
- __receive_buf(tty, cp, fp, n);
+
+ /* ignore parity errors if handling overflow */
+ if (!overflow || !fp || *fp != TTY_PARITY)
+ __receive_buf(tty, cp, fp, n);
+
cp += n;
if (fp)
fp += n;
@@ -1710,7 +1754,17 @@ n_tty_receive_buf_common(struct tty_struct *tty, const unsigned char *cp,
}
tty->receive_room = room;
- n_tty_check_throttle(tty);
+
+ /* Unthrottle if handling overflow on pty */
+ if (tty->driver->type == TTY_DRIVER_TYPE_PTY) {
+ if (overflow) {
+ tty_set_flow_change(tty, TTY_UNTHROTTLE_SAFE);
+ tty_unthrottle_safe(tty);
+ __tty_set_flow_change(tty, 0);
+ }
+ } else
+ n_tty_check_throttle(tty);
+
up_read(&tty->termios_rwsem);
return rcvd;
--
2.1.0
Hi Joseph,
On 04/15/2015 01:39 PM, Joseph Salisbury wrote:
> From: Peter Hurley <[email protected]>
>
> BugLink: http://bugs.launchpad.net/bugs/1381005
>
> In canon mode, the read buffer head will advance over the buffer tail
> if the input > 4095 bytes without receiving a line termination char.
>
> Discard additional input until a line termination is received.
> Before evaluating for overflow, the 'room' value is normalized for
> I_PARMRK and 1 byte is reserved for line termination (even in !icanon
> mode, in case the mode is switched). The following table shows the
> transform:
>
> actual buffer | 'room' value before overflow calc
> space avail | !I_PARMRK | I_PARMRK
> --------------------------------------------------
> 0 | -1 | -1
> 1 | 0 | 0
> 2 | 1 | 0
> 3 | 2 | 0
> 4+ | 3 | 1
>
> When !icanon or when icanon and the read buffer contains newlines,
> normalized 'room' values of -1 and 0 are clamped to 0, and
> 'overflow' is 0, so read_head is not adjusted and the input i/o loop
> exits (setting no_room if called from flush_to_ldisc()). No input
> is discarded since the reader does have input available to read
> which ensures forward progress.
>
> When icanon and the read buffer does not contain newlines and the
> normalized 'room' value is 0, then overflow and room are reset to 1,
> so that the i/o loop will process the next input char normally
> (except for parity errors which are ignored). Thus, erasures, signalling
> chars, 7-bit mode, etc. will continue to be handled properly.
>
> If the input char processed was not a line termination char, then
> the canon_head index will not have advanced, so the normalized 'room'
> value will now be -1 and 'overflow' will be set, which indicates the
> read_head can safely be reset, effectively erasing the last char
> processed.
>
> If the input char processed was a line termination, then the
> canon_head index will have advanced, so 'overflow' is cleared to 0,
> the read_head is not reset, and 'room' is cleared to 0, which exits
> the i/o loop (because the reader now have input available to read
> which ensures forward progress).
>
> Note that it is possible for a line termination to be received, and
> for the reader to copy the line to the user buffer before the
> input i/o loop is ready to process the next input char. This is
> why the i/o loop recomputes the room/overflow state with every
> input char while handling overflow.
>
> Finally, if the input data was processed without receiving
> a line termination (so that overflow is still set), the pty
> driver must receive a write wakeup. A pty writer may be waiting
> to write more data in n_tty_write() but without unthrottling
> here that wakeup will not arrive, and forward progress will halt.
> (Normally, the pty writer is woken when the reader reads data out
> of the buffer and more space become available).
Thanks for doing this!
(I can now cross off the 1st item on my TODO list)
comments below.
> Signed-off-by: Peter Hurley <[email protected]>
> Signed-off-by: Greg Kroah-Hartman <[email protected]>
> (backported from commit fb5ef9e7da39968fec6d6f37f20a23d23740c75e)
Please note this is essentially also a backport of commit
06c49f9fa31f ("n_tty: Fix PARMRK over-throttling") as well, since
it incorporates the results.
> Signed-off-by: Joseph Salisbury <[email protected]>
> ---
> drivers/tty/n_tty.c | 108 +++++++++++++++++++++++++++++++++++++++-------------
> 1 file changed, 81 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
> index 4ddfa60..f190136 100644
> --- a/drivers/tty/n_tty.c
> +++ b/drivers/tty/n_tty.c
> @@ -247,8 +247,8 @@ static void n_tty_write_wakeup(struct tty_struct *tty)
>
> static void n_tty_check_throttle(struct tty_struct *tty)
> {
> - if (tty->driver->type == TTY_DRIVER_TYPE_PTY)
> - return;
Ok.
> + struct n_tty_data *ldata = tty->disc_data;
> +
Drop this. Nothing in n_tty_check_throttle() uses 'ldata' as a result
of this commit.
> /*
> * Check the remaining room for the input canonicalization
> * mode. We don't want to throttle the driver if we're in
> @@ -1512,23 +1512,6 @@ n_tty_receive_char_lnext(struct tty_struct *tty, unsigned char c, char flag)
> n_tty_receive_char_flagged(tty, c, flag);
> }
>
> -/**
> - * n_tty_receive_buf - data receive
> - * @tty: terminal device
> - * @cp: buffer
> - * @fp: flag buffer
> - * @count: characters
> - *
> - * Called by the terminal driver when a block of characters has
> - * been received. This function must be called from soft contexts
> - * not from interrupt context. The driver is responsible for making
> - * calls one at a time and in order (or using flush_to_ldisc)
> - *
> - * n_tty_receive_buf()/producer path:
> - * claims non-exclusive termios_rwsem
> - * publishes read_head and canon_head
> - */
> -
> static void
> n_tty_receive_buf_real_raw(struct tty_struct *tty, const unsigned char *cp,
> char *fp, int count)
> @@ -1684,24 +1667,85 @@ static void __receive_buf(struct tty_struct *tty, const unsigned char *cp,
> }
> }
>
> +/**
> + * n_tty_receive_buf_common - process input
> + * @tty: device to receive input
> + * @cp: input chars
> + * @fp: flags for each char (if NULL, all chars are TTY_NORMAL)
> + * @count: number of input chars in @cp
> + *
> + * Called by the terminal driver when a block of characters has
> + * been received. This function must be called from soft contexts
> + * not from interrupt context. The driver is responsible for making
> + * calls one at a time and in order (or using flush_to_ldisc)
> + *
> + * Returns the # of input chars from @cp which were processed.
> + *
> + * In canonical mode, the maximum line length is 4096 chars (including
> + * the line termination char); lines longer than 4096 chars are
> + * truncated. After 4095 chars, input data is still processed but
> + * not stored. Overflow processing ensures the tty can always
> + * receive more input until at least one line can be read.
> + *
> + * In non-canonical mode, the read buffer will only accept 4095 chars;
> + * this provides the necessary space for a newline char if the input
> + * mode is switched to canonical.
> + *
> + * Note it is possible for the read buffer to _contain_ 4096 chars
> + * in non-canonical mode: the read buffer could already contain the
> + * maximum canon line of 4096 chars when the mode is switched to
> + * non-canonical.
> + *
> + * n_tty_receive_buf()/producer path:
> + * claims non-exclusive termios_rwsem
> + * publishes commit_head or canon_head
> + */
> static int
> n_tty_receive_buf_common(struct tty_struct *tty, const unsigned char *cp,
> char *fp, int count, int flow)
> {
> struct n_tty_data *ldata = tty->disc_data;
> - int room, n, rcvd = 0;
> + int room, n, rcvd = 0, overflow;
>
> down_read(&tty->termios_rwsem);
>
> while (1) {
> - room = receive_room(tty);
> + /*
> + * When PARMRK is set, each input char may take up to 3 chars
> + * in the read buf; reduce the buffer space avail by 3x
> + *
> + * If we are doing input canonicalization, and there are no
> + * pending newlines, let characters through without limit, so
> + * that erase characters will be handled. Other excess
> + * characters will be beeped.
> + *
> + * paired with store in *_copy_from_read_buf() -- guarantees
> + * the consumer has loaded the data in read_buf up to the new
> + * read_tail (so this producer will not overwrite unread data)
> + */
> + size_t tail = smp_load_acquire(&ldata->read_tail);
smp_load_acquire() is part of another fix not associated with this problem.
This line should simply be
size_t tail = ldata->read_tail;
Then this fix can be applied across 3.12~3.19, inclusive.
Thanks again.
Regards,
Peter Hurley
> +
> + room = N_TTY_BUF_SIZE - (ldata->read_head - tail);
> + if (I_PARMRK(tty))
> + room = (room + 2) / 3;
> + room--;
> + if (room <= 0) {
> + overflow = ldata->icanon && ldata->canon_head == tail;
> + if (overflow && room < 0)
> + ldata->read_head--;
> + room = overflow;
> + ldata->no_room = flow && !room;
> + } else
> + overflow = 0;
> +
> n = min(count, room);
> - if (!n) {
> - if (flow && !room)
> - ldata->no_room = 1;
> + if (!n)
> break;
> - }
> - __receive_buf(tty, cp, fp, n);
> +
> + /* ignore parity errors if handling overflow */
> + if (!overflow || !fp || *fp != TTY_PARITY)
> + __receive_buf(tty, cp, fp, n);
> +
> cp += n;
> if (fp)
> fp += n;
> @@ -1710,7 +1754,17 @@ n_tty_receive_buf_common(struct tty_struct *tty, const unsigned char *cp,
> }
>
> tty->receive_room = room;
> - n_tty_check_throttle(tty);
> +
> + /* Unthrottle if handling overflow on pty */
> + if (tty->driver->type == TTY_DRIVER_TYPE_PTY) {
> + if (overflow) {
> + tty_set_flow_change(tty, TTY_UNTHROTTLE_SAFE);
> + tty_unthrottle_safe(tty);
> + __tty_set_flow_change(tty, 0);
> + }
> + } else
> + n_tty_check_throttle(tty);
> +
> up_read(&tty->termios_rwsem);
>
> return rcvd;
>
On 04/15/2015 02:25 PM, Peter Hurley wrote:
> Hi Joseph,
>
> On 04/15/2015 01:39 PM, Joseph Salisbury wrote:
>> From: Peter Hurley <[email protected]>
>>
>> BugLink: http://bugs.launchpad.net/bugs/1381005
>>
>> In canon mode, the read buffer head will advance over the buffer tail
>> if the input > 4095 bytes without receiving a line termination char.
>>
>> Discard additional input until a line termination is received.
>> Before evaluating for overflow, the 'room' value is normalized for
>> I_PARMRK and 1 byte is reserved for line termination (even in !icanon
>> mode, in case the mode is switched). The following table shows the
>> transform:
>>
>> actual buffer | 'room' value before overflow calc
>> space avail | !I_PARMRK | I_PARMRK
>> --------------------------------------------------
>> 0 | -1 | -1
>> 1 | 0 | 0
>> 2 | 1 | 0
>> 3 | 2 | 0
>> 4+ | 3 | 1
>>
>> When !icanon or when icanon and the read buffer contains newlines,
>> normalized 'room' values of -1 and 0 are clamped to 0, and
>> 'overflow' is 0, so read_head is not adjusted and the input i/o loop
>> exits (setting no_room if called from flush_to_ldisc()). No input
>> is discarded since the reader does have input available to read
>> which ensures forward progress.
>>
>> When icanon and the read buffer does not contain newlines and the
>> normalized 'room' value is 0, then overflow and room are reset to 1,
>> so that the i/o loop will process the next input char normally
>> (except for parity errors which are ignored). Thus, erasures, signalling
>> chars, 7-bit mode, etc. will continue to be handled properly.
>>
>> If the input char processed was not a line termination char, then
>> the canon_head index will not have advanced, so the normalized 'room'
>> value will now be -1 and 'overflow' will be set, which indicates the
>> read_head can safely be reset, effectively erasing the last char
>> processed.
>>
>> If the input char processed was a line termination, then the
>> canon_head index will have advanced, so 'overflow' is cleared to 0,
>> the read_head is not reset, and 'room' is cleared to 0, which exits
>> the i/o loop (because the reader now have input available to read
>> which ensures forward progress).
>>
>> Note that it is possible for a line termination to be received, and
>> for the reader to copy the line to the user buffer before the
>> input i/o loop is ready to process the next input char. This is
>> why the i/o loop recomputes the room/overflow state with every
>> input char while handling overflow.
>>
>> Finally, if the input data was processed without receiving
>> a line termination (so that overflow is still set), the pty
>> driver must receive a write wakeup. A pty writer may be waiting
>> to write more data in n_tty_write() but without unthrottling
>> here that wakeup will not arrive, and forward progress will halt.
>> (Normally, the pty writer is woken when the reader reads data out
>> of the buffer and more space become available).
> Thanks for doing this!
> (I can now cross off the 1st item on my TODO list)
>
> comments below.
>
>> Signed-off-by: Peter Hurley <[email protected]>
>> Signed-off-by: Greg Kroah-Hartman <[email protected]>
>> (backported from commit fb5ef9e7da39968fec6d6f37f20a23d23740c75e)
> Please note this is essentially also a backport of commit
> 06c49f9fa31f ("n_tty: Fix PARMRK over-throttling") as well, since
> it incorporates the results.
>
>> Signed-off-by: Joseph Salisbury <[email protected]>
>> ---
>> drivers/tty/n_tty.c | 108 +++++++++++++++++++++++++++++++++++++++-------------
>> 1 file changed, 81 insertions(+), 27 deletions(-)
>>
>> diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
>> index 4ddfa60..f190136 100644
>> --- a/drivers/tty/n_tty.c
>> +++ b/drivers/tty/n_tty.c
>> @@ -247,8 +247,8 @@ static void n_tty_write_wakeup(struct tty_struct *tty)
>>
>> static void n_tty_check_throttle(struct tty_struct *tty)
>> {
>> - if (tty->driver->type == TTY_DRIVER_TYPE_PTY)
>> - return;
> Ok.
>
>> + struct n_tty_data *ldata = tty->disc_data;
>> +
> Drop this. Nothing in n_tty_check_throttle() uses 'ldata' as a result
> of this commit.
>
>> /*
>> * Check the remaining room for the input canonicalization
>> * mode. We don't want to throttle the driver if we're in
>> @@ -1512,23 +1512,6 @@ n_tty_receive_char_lnext(struct tty_struct *tty, unsigned char c, char flag)
>> n_tty_receive_char_flagged(tty, c, flag);
>> }
>>
>> -/**
>> - * n_tty_receive_buf - data receive
>> - * @tty: terminal device
>> - * @cp: buffer
>> - * @fp: flag buffer
>> - * @count: characters
>> - *
>> - * Called by the terminal driver when a block of characters has
>> - * been received. This function must be called from soft contexts
>> - * not from interrupt context. The driver is responsible for making
>> - * calls one at a time and in order (or using flush_to_ldisc)
>> - *
>> - * n_tty_receive_buf()/producer path:
>> - * claims non-exclusive termios_rwsem
>> - * publishes read_head and canon_head
>> - */
>> -
>> static void
>> n_tty_receive_buf_real_raw(struct tty_struct *tty, const unsigned char *cp,
>> char *fp, int count)
>> @@ -1684,24 +1667,85 @@ static void __receive_buf(struct tty_struct *tty, const unsigned char *cp,
>> }
>> }
>>
>> +/**
>> + * n_tty_receive_buf_common - process input
>> + * @tty: device to receive input
>> + * @cp: input chars
>> + * @fp: flags for each char (if NULL, all chars are TTY_NORMAL)
>> + * @count: number of input chars in @cp
>> + *
>> + * Called by the terminal driver when a block of characters has
>> + * been received. This function must be called from soft contexts
>> + * not from interrupt context. The driver is responsible for making
>> + * calls one at a time and in order (or using flush_to_ldisc)
>> + *
>> + * Returns the # of input chars from @cp which were processed.
>> + *
>> + * In canonical mode, the maximum line length is 4096 chars (including
>> + * the line termination char); lines longer than 4096 chars are
>> + * truncated. After 4095 chars, input data is still processed but
>> + * not stored. Overflow processing ensures the tty can always
>> + * receive more input until at least one line can be read.
>> + *
>> + * In non-canonical mode, the read buffer will only accept 4095 chars;
>> + * this provides the necessary space for a newline char if the input
>> + * mode is switched to canonical.
>> + *
>> + * Note it is possible for the read buffer to _contain_ 4096 chars
>> + * in non-canonical mode: the read buffer could already contain the
>> + * maximum canon line of 4096 chars when the mode is switched to
>> + * non-canonical.
>> + *
>> + * n_tty_receive_buf()/producer path:
>> + * claims non-exclusive termios_rwsem
>> + * publishes commit_head or canon_head
>> + */
>> static int
>> n_tty_receive_buf_common(struct tty_struct *tty, const unsigned char *cp,
>> char *fp, int count, int flow)
>> {
>> struct n_tty_data *ldata = tty->disc_data;
>> - int room, n, rcvd = 0;
>> + int room, n, rcvd = 0, overflow;
>>
>> down_read(&tty->termios_rwsem);
>>
>> while (1) {
>> - room = receive_room(tty);
>> + /*
>> + * When PARMRK is set, each input char may take up to 3 chars
>> + * in the read buf; reduce the buffer space avail by 3x
>> + *
>> + * If we are doing input canonicalization, and there are no
>> + * pending newlines, let characters through without limit, so
>> + * that erase characters will be handled. Other excess
>> + * characters will be beeped.
>> + *
>> + * paired with store in *_copy_from_read_buf() -- guarantees
>> + * the consumer has loaded the data in read_buf up to the new
>> + * read_tail (so this producer will not overwrite unread data)
>> + */
>> + size_t tail = smp_load_acquire(&ldata->read_tail);
> smp_load_acquire() is part of another fix not associated with this problem.
> This line should simply be
>
> size_t tail = ldata->read_tail;
>
> Then this fix can be applied across 3.12~3.19, inclusive.
>
> Thanks again.
>
> Regards,
> Peter Hurley
>
>> +
>> + room = N_TTY_BUF_SIZE - (ldata->read_head - tail);
>> + if (I_PARMRK(tty))
>> + room = (room + 2) / 3;
>> + room--;
>> + if (room <= 0) {
>> + overflow = ldata->icanon && ldata->canon_head == tail;
>> + if (overflow && room < 0)
>> + ldata->read_head--;
>> + room = overflow;
>> + ldata->no_room = flow && !room;
>> + } else
>> + overflow = 0;
>> +
>> n = min(count, room);
>> - if (!n) {
>> - if (flow && !room)
>> - ldata->no_room = 1;
>> + if (!n)
>> break;
>> - }
>> - __receive_buf(tty, cp, fp, n);
>> +
>> + /* ignore parity errors if handling overflow */
>> + if (!overflow || !fp || *fp != TTY_PARITY)
>> + __receive_buf(tty, cp, fp, n);
>> +
>> cp += n;
>> if (fp)
>> fp += n;
>> @@ -1710,7 +1754,17 @@ n_tty_receive_buf_common(struct tty_struct *tty, const unsigned char *cp,
>> }
>>
>> tty->receive_room = room;
>> - n_tty_check_throttle(tty);
>> +
>> + /* Unthrottle if handling overflow on pty */
>> + if (tty->driver->type == TTY_DRIVER_TYPE_PTY) {
>> + if (overflow) {
>> + tty_set_flow_change(tty, TTY_UNTHROTTLE_SAFE);
>> + tty_unthrottle_safe(tty);
>> + __tty_set_flow_change(tty, 0);
>> + }
>> + } else
>> + n_tty_check_throttle(tty);
>> +
>> up_read(&tty->termios_rwsem);
>>
>> return rcvd;
>>
Thanks for the feedback, Peter. I'll put together a V2 of this with all
your suggestions and resend.
Thanks,
Joe