<feed xmlns='http://www.w3.org/2005/Atom'>
<title>linux-stable.git/drivers/tty/n_tty.c, branch linux-5.11.y</title>
<subtitle>Linux kernel stable tree</subtitle>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux-stable.git/'/>
<entry>
<title>tty: teach the n_tty ICANON case about the new "cookie continuations" too</title>
<updated>2021-03-07T11:35:57+00:00</updated>
<author>
<name>Linus Torvalds</name>
<email>torvalds@linux-foundation.org</email>
</author>
<published>2021-01-20T23:43:38+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux-stable.git/commit/?id=79bc678f4ad9ea36a9d095ecab5e3cd3badcf89c'/>
<id>79bc678f4ad9ea36a9d095ecab5e3cd3badcf89c</id>
<content type='text'>
commit d7fe75cbc23c7d225eee2ef04def239b6603dce7 upstream.

The ICANON case is a bit messy, since it has to look for the line
ending, and has special code to then suppress line ending characters if
they match the __DISABLED_CHAR.  So it actually looks up the line ending
even past the point where it knows it won't copy it to the result
buffer.

That said, apart from all those odd legacy N_TTY ICANON cases, the
actual "should we continue copying" logic isn't really all that
complicated or different from the non-canon case.  In fact, the lack of
"wait for at least N characters" arguably makes the repeat case slightly
simpler.  It really just boils down to "there's more of the line to be
copied".

So add the necessarily trivial logic, and now the N_TTY case will give
long result lines even when in canon mode.

Signed-off-by: Linus Torvalds &lt;torvalds@linux-foundation.org&gt;
Signed-off-by: Greg Kroah-Hartman &lt;gregkh@linuxfoundation.org&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
commit d7fe75cbc23c7d225eee2ef04def239b6603dce7 upstream.

The ICANON case is a bit messy, since it has to look for the line
ending, and has special code to then suppress line ending characters if
they match the __DISABLED_CHAR.  So it actually looks up the line ending
even past the point where it knows it won't copy it to the result
buffer.

That said, apart from all those odd legacy N_TTY ICANON cases, the
actual "should we continue copying" logic isn't really all that
complicated or different from the non-canon case.  In fact, the lack of
"wait for at least N characters" arguably makes the repeat case slightly
simpler.  It really just boils down to "there's more of the line to be
copied".

So add the necessarily trivial logic, and now the N_TTY case will give
long result lines even when in canon mode.

Signed-off-by: Linus Torvalds &lt;torvalds@linux-foundation.org&gt;
Signed-off-by: Greg Kroah-Hartman &lt;gregkh@linuxfoundation.org&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>tty: teach n_tty line discipline about the new "cookie continuations"</title>
<updated>2021-03-07T11:35:57+00:00</updated>
<author>
<name>Linus Torvalds</name>
<email>torvalds@linux-foundation.org</email>
</author>
<published>2021-01-20T02:14:20+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux-stable.git/commit/?id=9d6b2b866044abf681c1864a08027d16e25bfab9'/>
<id>9d6b2b866044abf681c1864a08027d16e25bfab9</id>
<content type='text'>
commit 15ea8ae8e03fdb845ed3ff5d9f11dd5f4f60252c upstream.

With the conversion to do the tty ldisc read operations in small chunks,
the n_tty line discipline became noticeably slower for throughput
oriented loads, because rather than read things in up to 2kB chunks, it
would return at most 64 bytes per read() system call.

The cost is mainly all in the "do system calls over and over", not
really in the new "copy to an extra kernel buffer".

This can be fixed by teaching the n_tty line discipline about the
"cookie continuation" model, which the chunking code supports because
things like hdlc need to be able to handle packets up to 64kB in size.

Doing that doesn't just get us back to the old performace, but to much
better performance: my stupid "copy 10MB of data over a pty" test
program is now almost twice as fast as it used to be (going down from
0.1s to 0.054s).

This is entirely because it now creates maximal chunks (which happens to
be "one byte less than one page" due to how we do the circular tty
buffers).

NOTE! This case only handles the simpler non-icanon case, which is the
one where people may care about throughput.  I'm going to do the icanon
case later too, because while performance isn't a major issue for that,
there may be programs that think they'll always get a full line and
don't like the 64-byte chunking for that reason.

Such programs are arguably buggy (signals etc can cause random partial
results from tty reads anyway), and good programs will handle such
partial reads, but expecting everybody to write "good programs" has
never been a winning policy for the kernel..

Signed-off-by: Linus Torvalds &lt;torvalds@linux-foundation.org&gt;
Signed-off-by: Greg Kroah-Hartman &lt;gregkh@linuxfoundation.org&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
commit 15ea8ae8e03fdb845ed3ff5d9f11dd5f4f60252c upstream.

With the conversion to do the tty ldisc read operations in small chunks,
the n_tty line discipline became noticeably slower for throughput
oriented loads, because rather than read things in up to 2kB chunks, it
would return at most 64 bytes per read() system call.

The cost is mainly all in the "do system calls over and over", not
really in the new "copy to an extra kernel buffer".

This can be fixed by teaching the n_tty line discipline about the
"cookie continuation" model, which the chunking code supports because
things like hdlc need to be able to handle packets up to 64kB in size.

Doing that doesn't just get us back to the old performace, but to much
better performance: my stupid "copy 10MB of data over a pty" test
program is now almost twice as fast as it used to be (going down from
0.1s to 0.054s).

This is entirely because it now creates maximal chunks (which happens to
be "one byte less than one page" due to how we do the circular tty
buffers).

NOTE! This case only handles the simpler non-icanon case, which is the
one where people may care about throughput.  I'm going to do the icanon
case later too, because while performance isn't a major issue for that,
there may be programs that think they'll always get a full line and
don't like the 64-byte chunking for that reason.

Such programs are arguably buggy (signals etc can cause random partial
results from tty reads anyway), and good programs will handle such
partial reads, but expecting everybody to write "good programs" has
never been a winning policy for the kernel..

Signed-off-by: Linus Torvalds &lt;torvalds@linux-foundation.org&gt;
Signed-off-by: Greg Kroah-Hartman &lt;gregkh@linuxfoundation.org&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>tty: clean up legacy leftovers from n_tty line discipline</title>
<updated>2021-03-07T11:35:57+00:00</updated>
<author>
<name>Linus Torvalds</name>
<email>torvalds@linux-foundation.org</email>
</author>
<published>2021-01-19T21:46:28+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux-stable.git/commit/?id=7f68c3ba95ed62fedf8000be20c6cdd36cd5cc1c'/>
<id>7f68c3ba95ed62fedf8000be20c6cdd36cd5cc1c</id>
<content type='text'>
commit 64a69892afadd6fffaeadc65427bb7601161139d upstream.

Back when the line disciplines did their own direct user accesses, they
had to deal with the data copy possibly failing in the middle.

Now that the user copy is done by the tty_io.c code, that failure case
no longer exists.

Remove the left-over error handling code that cannot trigger.

Signed-off-by: Linus Torvalds &lt;torvalds@linux-foundation.org&gt;
Signed-off-by: Greg Kroah-Hartman &lt;gregkh@linuxfoundation.org&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
commit 64a69892afadd6fffaeadc65427bb7601161139d upstream.

Back when the line disciplines did their own direct user accesses, they
had to deal with the data copy possibly failing in the middle.

Now that the user copy is done by the tty_io.c code, that failure case
no longer exists.

Remove the left-over error handling code that cannot trigger.

Signed-off-by: Linus Torvalds &lt;torvalds@linux-foundation.org&gt;
Signed-off-by: Greg Kroah-Hartman &lt;gregkh@linuxfoundation.org&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>tty: convert tty_ldisc_ops 'read()' function to take a kernel pointer</title>
<updated>2021-03-04T11:14:00+00:00</updated>
<author>
<name>Linus Torvalds</name>
<email>torvalds@linux-foundation.org</email>
</author>
<published>2021-01-18T21:31:30+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux-stable.git/commit/?id=c075867d710865fe579d0f5b03392149a832596a'/>
<id>c075867d710865fe579d0f5b03392149a832596a</id>
<content type='text'>
[ Upstream commit 3b830a9c34d5897be07176ce4e6f2d75e2c8cfd7 ]

The tty line discipline .read() function was passed the final user
pointer destination as an argument, which doesn't match the 'write()'
function, and makes it very inconvenient to do a splice method for
ttys.

This is a conversion to use a kernel buffer instead.

NOTE! It does this by passing the tty line discipline -&gt;read() function
an additional "cookie" to fill in, and an offset into the cookie data.

The line discipline can fill in the cookie data with its own private
information, and then the reader will repeat the read until either the
cookie is cleared or it runs out of data.

The only real user of this is N_HDLC, which can use this to handle big
packets, even if the kernel buffer is smaller than the whole packet.

Cc: Christoph Hellwig &lt;hch@lst.de&gt;
Cc: Greg Kroah-Hartman &lt;gregkh@linuxfoundation.org&gt;
Cc: Al Viro &lt;viro@zeniv.linux.org.uk&gt;
Signed-off-by: Linus Torvalds &lt;torvalds@linux-foundation.org&gt;
Signed-off-by: Sasha Levin &lt;sashal@kernel.org&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
[ Upstream commit 3b830a9c34d5897be07176ce4e6f2d75e2c8cfd7 ]

The tty line discipline .read() function was passed the final user
pointer destination as an argument, which doesn't match the 'write()'
function, and makes it very inconvenient to do a splice method for
ttys.

This is a conversion to use a kernel buffer instead.

NOTE! It does this by passing the tty line discipline -&gt;read() function
an additional "cookie" to fill in, and an offset into the cookie data.

The line discipline can fill in the cookie data with its own private
information, and then the reader will repeat the read until either the
cookie is cleared or it runs out of data.

The only real user of this is N_HDLC, which can use this to handle big
packets, even if the kernel buffer is smaller than the whole packet.

Cc: Christoph Hellwig &lt;hch@lst.de&gt;
Cc: Greg Kroah-Hartman &lt;gregkh@linuxfoundation.org&gt;
Cc: Al Viro &lt;viro@zeniv.linux.org.uk&gt;
Signed-off-by: Linus Torvalds &lt;torvalds@linux-foundation.org&gt;
Signed-off-by: Sasha Levin &lt;sashal@kernel.org&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>Commit 9bb48c82aced ("tty: implement write_iter") converted the tty</title>
<updated>2021-01-25T20:08:07+00:00</updated>
<author>
<name>Sami Tolvanen</name>
<email>samitolvanen@google.com</email>
</author>
<published>2021-01-25T19:09:25+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux-stable.git/commit/?id=9f12e37cae44a96132fc3031535a0b165486941a'/>
<id>9f12e37cae44a96132fc3031535a0b165486941a</id>
<content type='text'>
layer to use write_iter. Fix the redirected_tty_write declaration
also in n_tty and change the comparisons to use write_iter instead of
write.

[ Also moved the declaration of redirected_tty_write() to the proper
  location in a header file. The reason for the bug was the bogus extern
  declaration in n_tty.c silently not matching the changed definition in
  tty_io.c, and because it wasn't in a shared header file, there was no
  cross-checking of the declaration.

  Sami noticed because Clang's Control Flow Integrity checking ended up
  incidentally noticing the inconsistent declaration.    - Linus ]

Fixes: 9bb48c82aced ("tty: implement write_iter")
Signed-off-by: Sami Tolvanen &lt;samitolvanen@google.com&gt;
Signed-off-by: Linus Torvalds &lt;torvalds@linux-foundation.org&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
layer to use write_iter. Fix the redirected_tty_write declaration
also in n_tty and change the comparisons to use write_iter instead of
write.

[ Also moved the declaration of redirected_tty_write() to the proper
  location in a header file. The reason for the bug was the bogus extern
  declaration in n_tty.c silently not matching the changed definition in
  tty_io.c, and because it wasn't in a shared header file, there was no
  cross-checking of the declaration.

  Sami noticed because Clang's Control Flow Integrity checking ended up
  incidentally noticing the inconsistent declaration.    - Linus ]

Fixes: 9bb48c82aced ("tty: implement write_iter")
Signed-off-by: Sami Tolvanen &lt;samitolvanen@google.com&gt;
Signed-off-by: Linus Torvalds &lt;torvalds@linux-foundation.org&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>tty: n_tty: Add 2 missing parameter descriptions</title>
<updated>2020-11-06T09:49:27+00:00</updated>
<author>
<name>Lee Jones</name>
<email>lee.jones@linaro.org</email>
</author>
<published>2020-11-04T19:35:23+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux-stable.git/commit/?id=171044a70b5750ea621659b143c075a47c1bfe40'/>
<id>171044a70b5750ea621659b143c075a47c1bfe40</id>
<content type='text'>
Fixes the following W=1 kernel build warning(s):

 drivers/tty/n_tty.c:405: warning: Function parameter or member 'tty' not described in 'is_continuation'
 drivers/tty/n_tty.c:1701: warning: Function parameter or member 'flow' not described in 'n_tty_receive_buf_common'

Cc: Greg Kroah-Hartman &lt;gregkh@linuxfoundation.org&gt;
Cc: Jiri Slaby &lt;jirislaby@kernel.org&gt;
Cc: "Andrew J. Kroll" &lt;ag784@freenet.buffalo.edu&gt;
Cc: processes-Sapan Bhatia &lt;sapan@corewars.org&gt;
Signed-off-by: Lee Jones &lt;lee.jones@linaro.org&gt;
Link: https://lore.kernel.org/r/20201104193549.4026187-11-lee.jones@linaro.org
Signed-off-by: Greg Kroah-Hartman &lt;gregkh@linuxfoundation.org&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Fixes the following W=1 kernel build warning(s):

 drivers/tty/n_tty.c:405: warning: Function parameter or member 'tty' not described in 'is_continuation'
 drivers/tty/n_tty.c:1701: warning: Function parameter or member 'flow' not described in 'n_tty_receive_buf_common'

Cc: Greg Kroah-Hartman &lt;gregkh@linuxfoundation.org&gt;
Cc: Jiri Slaby &lt;jirislaby@kernel.org&gt;
Cc: "Andrew J. Kroll" &lt;ag784@freenet.buffalo.edu&gt;
Cc: processes-Sapan Bhatia &lt;sapan@corewars.org&gt;
Signed-off-by: Lee Jones &lt;lee.jones@linaro.org&gt;
Link: https://lore.kernel.org/r/20201104193549.4026187-11-lee.jones@linaro.org
Signed-off-by: Greg Kroah-Hartman &lt;gregkh@linuxfoundation.org&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>tty: ldiscs, fix kernel-doc</title>
<updated>2020-08-18T11:51:18+00:00</updated>
<author>
<name>Jiri Slaby</name>
<email>jslaby@suse.cz</email>
</author>
<published>2020-08-18T08:56:52+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux-stable.git/commit/?id=724ac070ffc7a1e7526be4408edbd324a4163e15'/>
<id>724ac070ffc7a1e7526be4408edbd324a4163e15</id>
<content type='text'>
As in the previous patch, fix kernel-doc in line disciplines.

Signed-off-by: Jiri Slaby &lt;jslaby@suse.cz&gt;
Link: https://lore.kernel.org/r/20200818085655.12071-5-jslaby@suse.cz
Signed-off-by: Greg Kroah-Hartman &lt;gregkh@linuxfoundation.org&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
As in the previous patch, fix kernel-doc in line disciplines.

Signed-off-by: Jiri Slaby &lt;jslaby@suse.cz&gt;
Link: https://lore.kernel.org/r/20200818085655.12071-5-jslaby@suse.cz
Signed-off-by: Greg Kroah-Hartman &lt;gregkh@linuxfoundation.org&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>n_tty: Distribute switch variables for initialization</title>
<updated>2020-02-23T19:14:50+00:00</updated>
<author>
<name>Kees Cook</name>
<email>keescook@chromium.org</email>
</author>
<published>2020-02-20T06:23:13+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux-stable.git/commit/?id=e24cd4e6d6aa3ca741cdfdfc01118c4016acebea'/>
<id>e24cd4e6d6aa3ca741cdfdfc01118c4016acebea</id>
<content type='text'>
Variables declared in a switch statement before any case statements
cannot be automatically initialized with compiler instrumentation (as
they are not part of any execution flow). With GCC's proposed automatic
stack variable initialization feature, this triggers a warning (and they
don't get initialized). Clang's automatic stack variable initialization
(via CONFIG_INIT_STACK_ALL=y) doesn't throw a warning, but it also
doesn't initialize such variables[1]. Note that these warnings (or silent
skipping) happen before the dead-store elimination optimization phase,
so even when the automatic initializations are later elided in favor of
direct initializations, the warnings remain.

To avoid these problems, move such variables into the "case" where
they're used or lift them up into the main function body.

drivers/tty/n_tty.c: In function ‘__process_echoes’:
drivers/tty/n_tty.c:657:18: warning: statement will never be executed [-Wswitch-unreachable]
  657 |     unsigned int num_chars, num_bs;
      |                  ^~~~~~~~~

[1] https://bugs.llvm.org/show_bug.cgi?id=44916

Reviewed-by: Jiri Slaby &lt;jslaby@suse.cz&gt;
Signed-off-by: Kees Cook &lt;keescook@chromium.org&gt;
Link: https://lore.kernel.org/r/20200220062313.69209-1-keescook@chromium.org
Signed-off-by: Greg Kroah-Hartman &lt;gregkh@linuxfoundation.org&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Variables declared in a switch statement before any case statements
cannot be automatically initialized with compiler instrumentation (as
they are not part of any execution flow). With GCC's proposed automatic
stack variable initialization feature, this triggers a warning (and they
don't get initialized). Clang's automatic stack variable initialization
(via CONFIG_INIT_STACK_ALL=y) doesn't throw a warning, but it also
doesn't initialize such variables[1]. Note that these warnings (or silent
skipping) happen before the dead-store elimination optimization phase,
so even when the automatic initializations are later elided in favor of
direct initializations, the warnings remain.

To avoid these problems, move such variables into the "case" where
they're used or lift them up into the main function body.

drivers/tty/n_tty.c: In function ‘__process_echoes’:
drivers/tty/n_tty.c:657:18: warning: statement will never be executed [-Wswitch-unreachable]
  657 |     unsigned int num_chars, num_bs;
      |                  ^~~~~~~~~

[1] https://bugs.llvm.org/show_bug.cgi?id=44916

Reviewed-by: Jiri Slaby &lt;jslaby@suse.cz&gt;
Signed-off-by: Kees Cook &lt;keescook@chromium.org&gt;
Link: https://lore.kernel.org/r/20200220062313.69209-1-keescook@chromium.org
Signed-off-by: Greg Kroah-Hartman &lt;gregkh@linuxfoundation.org&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>n_tty: check printk arguments for n_tty_trace</title>
<updated>2020-02-10T20:34:43+00:00</updated>
<author>
<name>Jiri Slaby</name>
<email>jslaby@suse.cz</email>
</author>
<published>2020-01-30T11:58:43+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux-stable.git/commit/?id=a287885f1e378bff5f58cee6d0fd16b886c4863b'/>
<id>a287885f1e378bff5f58cee6d0fd16b886c4863b</id>
<content type='text'>
When N_TTY_TRACE is undefined (the default), define n_tty_trace to use
no_printk. That way, arguments are still checked during compilation.

Signed-off-by: Jiri Slaby &lt;jslaby@suse.cz&gt;
Link: https://lore.kernel.org/r/20200130115843.7452-1-jslaby@suse.cz
Signed-off-by: Greg Kroah-Hartman &lt;gregkh@linuxfoundation.org&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
When N_TTY_TRACE is undefined (the default), define n_tty_trace to use
no_printk. That way, arguments are still checked during compilation.

Signed-off-by: Jiri Slaby &lt;jslaby@suse.cz&gt;
Link: https://lore.kernel.org/r/20200130115843.7452-1-jslaby@suse.cz
Signed-off-by: Greg Kroah-Hartman &lt;gregkh@linuxfoundation.org&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>n_tty: check for negative and zero space return from tty_write_room</title>
<updated>2019-04-16T13:21:33+00:00</updated>
<author>
<name>Colin Ian King</name>
<email>colin.king@canonical.com</email>
</author>
<published>2019-03-30T00:46:28+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux-stable.git/commit/?id=9ef8927f45f2f8e365cda7cda81c079f29b8ad74'/>
<id>9ef8927f45f2f8e365cda7cda81c079f29b8ad74</id>
<content type='text'>
The return from tty_write_room could potentially be negative if
a tty write_room driver returns an error number (not that any seem
to do). Rather than just check for a zero return, also check for
a -ve return. This avoids the unsigned nr being set to a large unsigned
value on the assignment from variable space and can lead to overflowing
the buffer buf.  Better to be safe than assume all write_room
implementations in tty drivers are going to do the right thing.

Signed-off-by: Colin Ian King &lt;colin.king@canonical.com&gt;
Signed-off-by: Greg Kroah-Hartman &lt;gregkh@linuxfoundation.org&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
The return from tty_write_room could potentially be negative if
a tty write_room driver returns an error number (not that any seem
to do). Rather than just check for a zero return, also check for
a -ve return. This avoids the unsigned nr being set to a large unsigned
value on the assignment from variable space and can lead to overflowing
the buffer buf.  Better to be safe than assume all write_room
implementations in tty drivers are going to do the right thing.

Signed-off-by: Colin Ian King &lt;colin.king@canonical.com&gt;
Signed-off-by: Greg Kroah-Hartman &lt;gregkh@linuxfoundation.org&gt;
</pre>
</div>
</content>
</entry>
</feed>
