* bug#46981: Severe Emacs shell mode performance regression in recent Linux-libre
2021-03-07 2:54 ` Mark H Weaver
@ 2021-03-07 2:56 ` Mark H Weaver
2021-03-07 8:53 ` Bengt Richter
2021-03-07 14:51 ` Christopher Baines
2 siblings, 0 replies; 7+ messages in thread
From: Mark H Weaver @ 2021-03-07 2:56 UTC (permalink / raw)
To: 46981
[-- Attachment #1: Type: text/plain, Size: 355 bytes --]
Mark H Weaver <mhw@netris.org> writes:
> I've since confirmed that reverting the two upstream commits mentioned
> in <https://bugs.gnu.org/46978> fixes the performance regression. I've
> attached a preliminary patch for Guix that reverts those upstream
> commits for linux-libre@5.10, and which I've tested on my system.
Here's the patch.
Mark
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: [PATCH] gnu: linux-libre@5.10: Revert recent changes to TTY subsystem. --]
[-- Type: text/x-patch, Size: 28612 bytes --]
From 9522f18387673de5bad51d783e25c7b05dbab7d7 Mon Sep 17 00:00:00 2001
From: Mark H Weaver <mhw@netris.org>
Date: Sat, 6 Mar 2021 18:56:10 -0500
Subject: [PATCH] gnu: linux-libre@5.10: Revert recent changes to TTY
subsystem.
This fixes a performance regression in Emacs shell mode.
* gnu/packages/patches/linux-libre-revert-tty-changes-1.patch,
gnu/packages/patches/linux-libre-revert-tty-changes-2.patch: New files.
* gnu/local.mk (dist_patch_DATA): Add them.
* gnu/packages/linux.scm (%linux-libre-revert-tty-changes-1-patch)
(%linux-libre-revert-tty-changes-2-patch): New variables.
(linux-libre-5.10-source): Add the patches.
---
gnu/local.mk | 2 +
gnu/packages/linux.scm | 10 +-
.../linux-libre-revert-tty-changes-1.patch | 144 ++++
.../linux-libre-revert-tty-changes-2.patch | 667 ++++++++++++++++++
4 files changed, 822 insertions(+), 1 deletion(-)
create mode 100644 gnu/packages/patches/linux-libre-revert-tty-changes-1.patch
create mode 100644 gnu/packages/patches/linux-libre-revert-tty-changes-2.patch
diff --git a/gnu/local.mk b/gnu/local.mk
index 477853cd53..9b4f3395c7 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -1318,6 +1318,8 @@ dist_patch_DATA = \
%D%/packages/patches/linbox-fix-pkgconfig.patch \
%D%/packages/patches/linkchecker-tests-require-network.patch \
%D%/packages/patches/linphoneqt-tabbutton.patch \
+ %D%/packages/patches/linux-libre-revert-tty-changes-1.patch \
+ %D%/packages/patches/linux-libre-revert-tty-changes-2.patch \
%D%/packages/patches/linux-libre-support-for-Pinebook-Pro.patch \
%D%/packages/patches/linux-pam-no-setfsuid.patch \
%D%/packages/patches/lirc-localstatedir.patch \
diff --git a/gnu/packages/linux.scm b/gnu/packages/linux.scm
index 5b5cb8ea8d..da1afe4648 100644
--- a/gnu/packages/linux.scm
+++ b/gnu/packages/linux.scm
@@ -473,6 +473,12 @@ corresponding UPSTREAM-SOURCE (an origin), using the given DEBLOB-SCRIPTS."
(sha256
(base32 "1ifnfhpakzffn4b8n7x7w5cps9mzjxlkcfz9zqak2vaw8nzvl39f"))))
+(define %linux-libre-revert-tty-changes-1-patch
+ (search-patch "linux-libre-revert-tty-changes-1.patch"))
+
+(define %linux-libre-revert-tty-changes-2-patch
+ (search-patch "linux-libre-revert-tty-changes-2.patch"))
+
(define (source-with-patches source patches)
(origin
(inherit source)
@@ -487,7 +493,9 @@ corresponding UPSTREAM-SOURCE (an origin), using the given DEBLOB-SCRIPTS."
(define-public linux-libre-5.10-source
(source-with-patches linux-libre-5.10-pristine-source
(list %boot-logo-patch
- %linux-libre-arm-export-__sync_icache_dcache-patch)))
+ %linux-libre-arm-export-__sync_icache_dcache-patch
+ %linux-libre-revert-tty-changes-1-patch
+ %linux-libre-revert-tty-changes-2-patch)))
(define-public linux-libre-5.4-source
(source-with-patches linux-libre-5.4-pristine-source
diff --git a/gnu/packages/patches/linux-libre-revert-tty-changes-1.patch b/gnu/packages/patches/linux-libre-revert-tty-changes-1.patch
new file mode 100644
index 0000000000..76c0974f87
--- /dev/null
+++ b/gnu/packages/patches/linux-libre-revert-tty-changes-1.patch
@@ -0,0 +1,144 @@
+Revert the following upstream patch, which dramatically decreased the
+performance of shell buffers in Emacs.
+
+
+From 41c6f6b926d0e712d0321f8a8f6511fea748e814 Mon Sep 17 00:00:00 2001
+From: Linus Torvalds <torvalds@linux-foundation.org>
+Date: Tue, 19 Jan 2021 10:49:19 -0800
+Subject: tty: implement read_iter
+
+[ Upstream commit dd78b0c483e33225e0e0782b0ed887129b00f956 ]
+
+Now that the ldisc read() function takes kernel pointers, it's fairly
+straightforward to make the tty file operations use .read_iter() instead
+of .read().
+
+That automatically gives us vread() and friends, and also makes it
+possible to do .splice_read() on ttys again.
+
+Fixes: 36e2c7421f02 ("fs: don't allow splice read/write without explicit ops")
+Reported-by: Oliver Giles <ohw.giles@gmail.com>
+Cc: Christoph Hellwig <hch@lst.de>
+Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
+Cc: Al Viro <viro@zeniv.linux.org.uk>
+Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
+Signed-off-by: Sasha Levin <sashal@kernel.org>
+---
+ drivers/tty/tty_io.c | 36 ++++++++++++++++++------------------
+ 1 file changed, 18 insertions(+), 18 deletions(-)
+
+diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
+index a50c8a4318228..3f55fe7293f31 100644
+--- b/drivers/tty/tty_io.c
++++ a/drivers/tty/tty_io.c
+@@ -142,7 +142,7 @@
+ /* Mutex to protect creating and releasing a tty */
+ DEFINE_MUTEX(tty_mutex);
+
+-static ssize_t tty_read(struct kiocb *, struct iov_iter *);
++static ssize_t tty_read(struct file *, char __user *, size_t, loff_t *);
+ static ssize_t tty_write(struct kiocb *, struct iov_iter *);
+ static __poll_t tty_poll(struct file *, poll_table *);
+ static int tty_open(struct inode *, struct file *);
+@@ -473,9 +473,8 @@
+
+ static const struct file_operations tty_fops = {
+ .llseek = no_llseek,
+- .read_iter = tty_read,
++ .read = tty_read,
+ .write_iter = tty_write,
+- .splice_read = generic_file_splice_read,
+ .splice_write = iter_file_splice_write,
+ .poll = tty_poll,
+ .unlocked_ioctl = tty_ioctl,
+@@ -488,9 +487,8 @@
+
+ static const struct file_operations console_fops = {
+ .llseek = no_llseek,
+- .read_iter = tty_read,
++ .read = tty_read,
+ .write_iter = redirected_tty_write,
+- .splice_read = generic_file_splice_read,
+ .splice_write = iter_file_splice_write,
+ .poll = tty_poll,
+ .unlocked_ioctl = tty_ioctl,
+@@ -843,17 +841,16 @@
+ * data or clears the cookie. The cookie may be something that the
+ * ldisc maintains state for and needs to free.
+ */
+-static int iterate_tty_read(struct tty_ldisc *ld, struct tty_struct *tty,
+- struct file *file, struct iov_iter *to)
++static int iterate_tty_read(struct tty_ldisc *ld, struct tty_struct *tty, struct file *file,
++ char __user *buf, size_t count)
+ {
+ int retval = 0;
+ void *cookie = NULL;
+ unsigned long offset = 0;
+ char kernel_buf[64];
+- size_t count = iov_iter_count(to);
+
+ do {
+- int size, copied;
++ int size, uncopied;
+
+ size = count > sizeof(kernel_buf) ? sizeof(kernel_buf) : count;
+ size = ld->ops->read(tty, file, kernel_buf, size, &cookie, offset);
+@@ -869,9 +866,10 @@
+ return size;
+ }
+
+- copied = copy_to_iter(kernel_buf, size, to);
+- offset += copied;
+- count -= copied;
++ uncopied = copy_to_user(buf+offset, kernel_buf, size);
++ size -= uncopied;
++ offset += size;
++ count -= size;
+
+ /*
+ * If the user copy failed, we still need to do another ->read()
+@@ -879,7 +877,7 @@
+ *
+ * But make sure size is zeroed.
+ */
+- if (unlikely(copied != size)) {
++ if (unlikely(uncopied)) {
+ count = 0;
+ retval = -EFAULT;
+ }
+@@ -906,10 +904,10 @@
+ * read calls may be outstanding in parallel.
+ */
+
+-static ssize_t tty_read(struct kiocb *iocb, struct iov_iter *to)
++static ssize_t tty_read(struct file *file, char __user *buf, size_t count,
++ loff_t *ppos)
+ {
+ int i;
+- struct file *file = iocb->ki_filp;
+ struct inode *inode = file_inode(file);
+ struct tty_struct *tty = file_tty(file);
+ struct tty_ldisc *ld;
+@@ -922,9 +920,11 @@
+ /* We want to wait for the line discipline to sort out in this
+ situation */
+ ld = tty_ldisc_ref_wait(tty);
++ if (!ld)
++ return hung_up_tty_read(file, buf, count, ppos);
+ i = -EIO;
+- if (ld && ld->ops->read)
+- i = iterate_tty_read(ld, tty, file, to);
++ if (ld->ops->read)
++ i = iterate_tty_read(ld, tty, file, buf, count);
+ tty_ldisc_deref(ld);
+
+ if (i > 0)
+@@ -2943,7 +2943,7 @@
+
+ static int this_tty(const void *t, struct file *file, unsigned fd)
+ {
+- if (likely(file->f_op->read_iter != tty_read))
++ if (likely(file->f_op->read != tty_read))
+ return 0;
+ return file_tty(file) != t ? 0 : fd + 1;
+ }
diff --git a/gnu/packages/patches/linux-libre-revert-tty-changes-2.patch b/gnu/packages/patches/linux-libre-revert-tty-changes-2.patch
new file mode 100644
index 0000000000..9ef0ff477e
--- /dev/null
+++ b/gnu/packages/patches/linux-libre-revert-tty-changes-2.patch
@@ -0,0 +1,667 @@
+Revert the following upstream patch, which dramatically decreased the
+performance of shell buffers in Emacs.
+
+
+From 279e54536ddbb4dbd337fca74926b68651160043 Mon Sep 17 00:00:00 2001
+From: Linus Torvalds <torvalds@linux-foundation.org>
+Date: Mon, 18 Jan 2021 13:31:30 -0800
+Subject: tty: convert tty_ldisc_ops 'read()' function to take a kernel pointer
+
+[ 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 ->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 <hch@lst.de>
+Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
+Cc: Al Viro <viro@zeniv.linux.org.uk>
+Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
+Signed-off-by: Sasha Levin <sashal@kernel.org>
+---
+ drivers/bluetooth/hci_ldisc.c | 34 +++++++++---------
+ drivers/input/serio/serport.c | 4 ++-
+ drivers/net/ppp/ppp_async.c | 3 +-
+ drivers/net/ppp/ppp_synctty.c | 3 +-
+ drivers/tty/n_gsm.c | 3 +-
+ drivers/tty/n_hdlc.c | 60 +++++++++++++++++++++----------
+ drivers/tty/n_null.c | 3 +-
+ drivers/tty/n_r3964.c | 10 +++---
+ drivers/tty/n_tracerouter.c | 4 ++-
+ drivers/tty/n_tracesink.c | 4 ++-
+ drivers/tty/n_tty.c | 82 ++++++++++++++++++-------------------------
+ drivers/tty/tty_io.c | 64 +++++++++++++++++++++++++++++++--
+ include/linux/tty_ldisc.h | 3 +-
+ net/nfc/nci/uart.c | 3 +-
+ 14 files changed, 178 insertions(+), 102 deletions(-)
+
+diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
+index 8be4d807d1370..637c5b8c2aa1a 100644
+--- b/drivers/bluetooth/hci_ldisc.c
++++ a/drivers/bluetooth/hci_ldisc.c
+@@ -801,8 +801,7 @@
+ * We don't provide read/write/poll interface for user space.
+ */
+ static ssize_t hci_uart_tty_read(struct tty_struct *tty, struct file *file,
+- unsigned char *buf, size_t nr,
+- void **cookie, unsigned long offset)
++ unsigned char __user *buf, size_t nr)
+ {
+ return 0;
+ }
+@@ -819,28 +818,29 @@
+ return 0;
+ }
+
+-static struct tty_ldisc_ops hci_uart_ldisc = {
+- .owner = THIS_MODULE,
+- .magic = TTY_LDISC_MAGIC,
+- .name = "n_hci",
+- .open = hci_uart_tty_open,
+- .close = hci_uart_tty_close,
+- .read = hci_uart_tty_read,
+- .write = hci_uart_tty_write,
+- .ioctl = hci_uart_tty_ioctl,
+- .compat_ioctl = hci_uart_tty_ioctl,
+- .poll = hci_uart_tty_poll,
+- .receive_buf = hci_uart_tty_receive,
+- .write_wakeup = hci_uart_tty_wakeup,
+-};
+-
+ static int __init hci_uart_init(void)
+ {
++ static struct tty_ldisc_ops hci_uart_ldisc;
+ int err;
+
+ BT_INFO("HCI UART driver ver %s", VERSION);
+
+ /* Register the tty discipline */
++
++ memset(&hci_uart_ldisc, 0, sizeof(hci_uart_ldisc));
++ hci_uart_ldisc.magic = TTY_LDISC_MAGIC;
++ hci_uart_ldisc.name = "n_hci";
++ hci_uart_ldisc.open = hci_uart_tty_open;
++ hci_uart_ldisc.close = hci_uart_tty_close;
++ hci_uart_ldisc.read = hci_uart_tty_read;
++ hci_uart_ldisc.write = hci_uart_tty_write;
++ hci_uart_ldisc.ioctl = hci_uart_tty_ioctl;
++ hci_uart_ldisc.compat_ioctl = hci_uart_tty_ioctl;
++ hci_uart_ldisc.poll = hci_uart_tty_poll;
++ hci_uart_ldisc.receive_buf = hci_uart_tty_receive;
++ hci_uart_ldisc.write_wakeup = hci_uart_tty_wakeup;
++ hci_uart_ldisc.owner = THIS_MODULE;
++
+ err = tty_register_ldisc(N_HCI, &hci_uart_ldisc);
+ if (err) {
+ BT_ERR("HCI line discipline registration failed. (%d)", err);
+diff --git a/drivers/input/serio/serport.c b/drivers/input/serio/serport.c
+index 8ac970a423de6..33e9d9bfd036f 100644
+--- b/drivers/input/serio/serport.c
++++ a/drivers/input/serio/serport.c
+@@ -156,9 +156,7 @@
+ * returning 0 characters.
+ */
+
+-static ssize_t serport_ldisc_read(struct tty_struct * tty, struct file * file,
+- unsigned char *kbuf, size_t nr,
+- void **cookie, unsigned long offset)
++static ssize_t serport_ldisc_read(struct tty_struct * tty, struct file * file, unsigned char __user * buf, size_t nr)
+ {
+ struct serport *serport = (struct serport*) tty->disc_data;
+ struct serio *serio;
+diff --git a/drivers/net/ppp/ppp_async.c b/drivers/net/ppp/ppp_async.c
+index 29a0917a81e60..f14a9d190de91 100644
+--- b/drivers/net/ppp/ppp_async.c
++++ a/drivers/net/ppp/ppp_async.c
+@@ -259,8 +259,7 @@
+ */
+ static ssize_t
+ ppp_asynctty_read(struct tty_struct *tty, struct file *file,
+- unsigned char *buf, size_t count,
+- void **cookie, unsigned long offset)
++ unsigned char __user *buf, size_t count)
+ {
+ return -EAGAIN;
+ }
+diff --git a/drivers/net/ppp/ppp_synctty.c b/drivers/net/ppp/ppp_synctty.c
+index 0f338752c38b9..f774b7e52da44 100644
+--- b/drivers/net/ppp/ppp_synctty.c
++++ a/drivers/net/ppp/ppp_synctty.c
+@@ -257,8 +257,7 @@
+ */
+ static ssize_t
+ ppp_sync_read(struct tty_struct *tty, struct file *file,
+- unsigned char *buf, size_t count,
+- void **cookie, unsigned long offset)
++ unsigned char __user *buf, size_t count)
+ {
+ return -EAGAIN;
+ }
+diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
+index 25f3152089c2a..fea1eeac5b907 100644
+--- b/drivers/tty/n_gsm.c
++++ a/drivers/tty/n_gsm.c
+@@ -2557,8 +2557,7 @@
+ */
+
+ static ssize_t gsmld_read(struct tty_struct *tty, struct file *file,
+- unsigned char *buf, size_t nr,
+- void **cookie, unsigned long offset)
++ unsigned char __user *buf, size_t nr)
+ {
+ return -EOPNOTSUPP;
+ }
+diff --git a/drivers/tty/n_hdlc.c b/drivers/tty/n_hdlc.c
+index 12557ee1edb68..1363e659dc1db 100644
+--- b/drivers/tty/n_hdlc.c
++++ a/drivers/tty/n_hdlc.c
+@@ -416,19 +416,13 @@
+ * Returns the number of bytes returned or error code.
+ */
+ static ssize_t n_hdlc_tty_read(struct tty_struct *tty, struct file *file,
+- __u8 *kbuf, size_t nr,
+- void **cookie, unsigned long offset)
++ __u8 __user *buf, size_t nr)
+ {
+ struct n_hdlc *n_hdlc = tty->disc_data;
+ int ret = 0;
+ struct n_hdlc_buf *rbuf;
+ DECLARE_WAITQUEUE(wait, current);
+
+- /* Is this a repeated call for an rbuf we already found earlier? */
+- rbuf = *cookie;
+- if (rbuf)
+- goto have_rbuf;
+-
+ add_wait_queue(&tty->read_wait, &wait);
+
+ for (;;) {
+@@ -442,8 +436,25 @@
+ set_current_state(TASK_INTERRUPTIBLE);
+
+ rbuf = n_hdlc_buf_get(&n_hdlc->rx_buf_list);
+- if (rbuf)
++ if (rbuf) {
++ if (rbuf->count > nr) {
++ /* too large for caller's buffer */
++ ret = -EOVERFLOW;
++ } else {
++ __set_current_state(TASK_RUNNING);
++ if (copy_to_user(buf, rbuf->buf, rbuf->count))
++ ret = -EFAULT;
++ else
++ ret = rbuf->count;
++ }
++
++ if (n_hdlc->rx_free_buf_list.count >
++ DEFAULT_RX_BUF_COUNT)
++ kfree(rbuf);
++ else
++ n_hdlc_buf_put(&n_hdlc->rx_free_buf_list, rbuf);
+ break;
++ }
+
+ /* no data */
+ if (tty_io_nonblock(tty, file)) {
+@@ -462,39 +473,6 @@
+ remove_wait_queue(&tty->read_wait, &wait);
+ __set_current_state(TASK_RUNNING);
+
+- if (!rbuf)
+- return ret;
+- *cookie = rbuf;
+-
+-have_rbuf:
+- /* Have we used it up entirely? */
+- if (offset >= rbuf->count)
+- goto done_with_rbuf;
+-
+- /* More data to go, but can't copy any more? EOVERFLOW */
+- ret = -EOVERFLOW;
+- if (!nr)
+- goto done_with_rbuf;
+-
+- /* Copy as much data as possible */
+- ret = rbuf->count - offset;
+- if (ret > nr)
+- ret = nr;
+- memcpy(kbuf, rbuf->buf+offset, ret);
+- offset += ret;
+-
+- /* If we still have data left, we leave the rbuf in the cookie */
+- if (offset < rbuf->count)
+- return ret;
+-
+-done_with_rbuf:
+- *cookie = NULL;
+-
+- if (n_hdlc->rx_free_buf_list.count > DEFAULT_RX_BUF_COUNT)
+- kfree(rbuf);
+- else
+- n_hdlc_buf_put(&n_hdlc->rx_free_buf_list, rbuf);
+-
+ return ret;
+
+ } /* end of n_hdlc_tty_read() */
+diff --git a/drivers/tty/n_null.c b/drivers/tty/n_null.c
+index 96feabae47407..ce03ae78f5c6a 100644
+--- b/drivers/tty/n_null.c
++++ a/drivers/tty/n_null.c
+@@ -20,8 +20,7 @@
+ }
+
+ static ssize_t n_null_read(struct tty_struct *tty, struct file *file,
+- unsigned char *buf, size_t nr,
+- void **cookie, unsigned long offset)
++ unsigned char __user * buf, size_t nr)
+ {
+ return -EOPNOTSUPP;
+ }
+diff --git a/drivers/tty/n_r3964.c b/drivers/tty/n_r3964.c
+index 934dd2fb2ec80..3161f0a535e37 100644
+--- b/drivers/tty/n_r3964.c
++++ a/drivers/tty/n_r3964.c
+@@ -129,7 +129,7 @@
+ static int r3964_open(struct tty_struct *tty);
+ static void r3964_close(struct tty_struct *tty);
+ static ssize_t r3964_read(struct tty_struct *tty, struct file *file,
+- void *cookie, unsigned char *buf, size_t nr);
++ unsigned char __user * buf, size_t nr);
+ static ssize_t r3964_write(struct tty_struct *tty, struct file *file,
+ const unsigned char *buf, size_t nr);
+ static int r3964_ioctl(struct tty_struct *tty, struct file *file,
+@@ -1058,8 +1058,7 @@
+ }
+
+ static ssize_t r3964_read(struct tty_struct *tty, struct file *file,
+- unsigned char *kbuf, size_t nr,
+- void **cookie, unsigned long offset)
++ unsigned char __user * buf, size_t nr)
+ {
+ struct r3964_info *pInfo = tty->disc_data;
+ struct r3964_client_info *pClient;
+@@ -1110,7 +1109,10 @@
+ kfree(pMsg);
+ TRACE_M("r3964_read - msg kfree %p", pMsg);
+
+- memcpy(kbuf, &theMsg, ret);
++ if (copy_to_user(buf, &theMsg, ret)) {
++ ret = -EFAULT;
++ goto unlock;
++ }
+
+ TRACE_PS("read - return %d", ret);
+ goto unlock;
+diff --git a/drivers/tty/n_tracerouter.c b/drivers/tty/n_tracerouter.c
+index 4479af4d2fa5c..3490ed51b1a3c 100644
+--- b/drivers/tty/n_tracerouter.c
++++ a/drivers/tty/n_tracerouter.c
+@@ -118,9 +118,7 @@
+ * -EINVAL
+ */
+ static ssize_t n_tracerouter_read(struct tty_struct *tty, struct file *file,
+- unsigned char *buf, size_t nr,
+- void **cookie, unsigned long offset)
+-{
++ unsigned char __user *buf, size_t nr) {
+ return -EINVAL;
+ }
+
+diff --git a/drivers/tty/n_tracesink.c b/drivers/tty/n_tracesink.c
+index d96ba82cc3569..1d9931041fd8b 100644
+--- b/drivers/tty/n_tracesink.c
++++ a/drivers/tty/n_tracesink.c
+@@ -115,9 +115,7 @@
+ * -EINVAL
+ */
+ static ssize_t n_tracesink_read(struct tty_struct *tty, struct file *file,
+- unsigned char *buf, size_t nr,
+- void **cookie, unsigned long offset)
+-{
++ unsigned char __user *buf, size_t nr) {
+ return -EINVAL;
+ }
+
+diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
+index c2869489ba681..e8963165082ee 100644
+--- b/drivers/tty/n_tty.c
++++ a/drivers/tty/n_tty.c
+@@ -164,24 +164,29 @@
+ memset(buffer, 0x00, size);
+ }
+
+-static void tty_copy(struct tty_struct *tty, void *to, size_t tail, size_t n)
++static int tty_copy_to_user(struct tty_struct *tty, void __user *to,
++ size_t tail, size_t n)
+ {
+ struct n_tty_data *ldata = tty->disc_data;
+ size_t size = N_TTY_BUF_SIZE - tail;
+ void *from = read_buf_addr(ldata, tail);
++ int uncopied;
+
+ if (n > size) {
+ tty_audit_add_data(tty, from, size);
+- memcpy(to, from, size);
+- zero_buffer(tty, from, size);
++ uncopied = copy_to_user(to, from, size);
++ zero_buffer(tty, from, size - uncopied);
++ if (uncopied)
++ return uncopied;
+ to += size;
+ n -= size;
+ from = ldata->read_buf;
+ }
+
+ tty_audit_add_data(tty, from, n);
+- memcpy(to, from, n);
+- zero_buffer(tty, from, n);
++ uncopied = copy_to_user(to, from, n);
++ zero_buffer(tty, from, n - uncopied);
++ return uncopied;
+ }
+
+ /**
+@@ -1937,16 +1942,15 @@
+ /**
+ * copy_from_read_buf - copy read data directly
+ * @tty: terminal device
+- * @kbp: data
++ * @b: user data
+ * @nr: size of data
+ *
+ * Helper function to speed up n_tty_read. It is only called when
+- * ICANON is off; it copies characters straight from the tty queue.
+- *
+- * It can be profitably called twice; once to drain the space from
+- * the tail pointer to the (physical) end of the buffer, and once
+- * to drain the space from the (physical) beginning of the buffer
+- * to head pointer.
++ * ICANON is off; it copies characters straight from the tty queue to
++ * user space directly. It can be profitably called twice; once to
++ * drain the space from the tail pointer to the (physical) end of the
++ * buffer, and once to drain the space from the (physical) beginning of
++ * the buffer to head pointer.
+ *
+ * Called under the ldata->atomic_read_lock sem
+ *
+@@ -1956,7 +1960,7 @@
+ */
+
+ static int copy_from_read_buf(struct tty_struct *tty,
+- unsigned char **kbp,
++ unsigned char __user **b,
+ size_t *nr)
+
+ {
+@@ -1972,7 +1976,8 @@
+ n = min(*nr, n);
+ if (n) {
+ unsigned char *from = read_buf_addr(ldata, tail);
+- memcpy(*kbp, from, n);
++ retval = copy_to_user(*b, from, n);
++ n -= retval;
+ is_eof = n == 1 && *from == EOF_CHAR(tty);
+ tty_audit_add_data(tty, from, n);
+ zero_buffer(tty, from, n);
+@@ -1981,7 +1986,7 @@
+ if (L_EXTPROC(tty) && ldata->icanon && is_eof &&
+ (head == ldata->read_tail))
+ n = 0;
+- *kbp += n;
++ *b += n;
+ *nr -= n;
+ }
+ return retval;
+@@ -1990,12 +1995,12 @@
+ /**
+ * canon_copy_from_read_buf - copy read data in canonical mode
+ * @tty: terminal device
+- * @kbp: data
++ * @b: user data
+ * @nr: size of data
+ *
+ * Helper function for n_tty_read. It is only called when ICANON is on;
+ * it copies one line of input up to and including the line-delimiting
+- * character into the result buffer.
++ * character into the user-space buffer.
+ *
+ * NB: When termios is changed from non-canonical to canonical mode and
+ * the read buffer contains data, n_tty_set_termios() simulates an EOF
+@@ -2011,14 +2016,14 @@
+ */
+
+ static int canon_copy_from_read_buf(struct tty_struct *tty,
+- unsigned char **kbp,
++ unsigned char __user **b,
+ size_t *nr)
+ {
+ struct n_tty_data *ldata = tty->disc_data;
+ size_t n, size, more, c;
+ size_t eol;
+ size_t tail;
+- int found = 0;
++ int ret, found = 0;
+
+ /* N.B. avoid overrun if nr == 0 */
+ if (!*nr)
+@@ -2054,8 +2059,10 @@
+ n_tty_trace("%s: eol:%zu found:%d n:%zu c:%zu tail:%zu more:%zu\n",
+ __func__, eol, found, n, c, tail, more);
+
+- tty_copy(tty, *kbp, tail, n);
+- *kbp += n;
++ ret = tty_copy_to_user(tty, *b, tail, n);
++ if (ret)
++ return -EFAULT;
++ *b += n;
+ *nr -= n;
+
+ if (found)
+@@ -2120,11 +2127,10 @@
+ */
+
+ static ssize_t n_tty_read(struct tty_struct *tty, struct file *file,
+- unsigned char *kbuf, size_t nr,
+- void **cookie, unsigned long offset)
++ unsigned char __user *buf, size_t nr)
+ {
+ struct n_tty_data *ldata = tty->disc_data;
+- unsigned char *kb = kbuf;
++ unsigned char __user *b = buf;
+ DEFINE_WAIT_FUNC(wait, woken_wake_function);
+ int c;
+ int minimum, time;
+@@ -2170,13 +2176,17 @@
+ /* First test for status change. */
+ if (packet && tty->link->ctrl_status) {
+ unsigned char cs;
+- if (kb != kbuf)
++ if (b != buf)
+ break;
+ spin_lock_irq(&tty->link->ctrl_lock);
+ cs = tty->link->ctrl_status;
+ tty->link->ctrl_status = 0;
+ spin_unlock_irq(&tty->link->ctrl_lock);
+- *kb++ = cs;
++ if (put_user(cs, b)) {
++ retval = -EFAULT;
++ break;
++ }
++ b++;
+ nr--;
+ break;
+ }
+@@ -2219,20 +2229,24 @@
+ }
+
+ if (ldata->icanon && !L_EXTPROC(tty)) {
+- retval = canon_copy_from_read_buf(tty, &kb, &nr);
++ retval = canon_copy_from_read_buf(tty, &b, &nr);
+ if (retval)
+ break;
+ } else {
+ int uncopied;
+
+ /* Deal with packet mode. */
+- if (packet && kb == kbuf) {
+- *kb++ = TIOCPKT_DATA;
++ if (packet && b == buf) {
++ if (put_user(TIOCPKT_DATA, b)) {
++ retval = -EFAULT;
++ break;
++ }
++ b++;
+ nr--;
+ }
+
+- uncopied = copy_from_read_buf(tty, &kb, &nr);
+- uncopied += copy_from_read_buf(tty, &kb, &nr);
++ uncopied = copy_from_read_buf(tty, &b, &nr);
++ uncopied += copy_from_read_buf(tty, &b, &nr);
+ if (uncopied) {
+ retval = -EFAULT;
+ break;
+@@ -2241,7 +2255,7 @@
+
+ n_tty_check_unthrottle(tty);
+
+- if (kb - kbuf >= minimum)
++ if (b - buf >= minimum)
+ break;
+ if (time)
+ timeout = time;
+@@ -2253,8 +2267,8 @@
+ remove_wait_queue(&tty->read_wait, &wait);
+ mutex_unlock(&ldata->atomic_read_lock);
+
+- if (kb - kbuf)
+- retval = kb - kbuf;
++ if (b - buf)
++ retval = b - buf;
+
+ return retval;
+ }
+diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
+index 21cd5ac6ca8b5..a50c8a4318228 100644
+--- b/drivers/tty/tty_io.c
++++ a/drivers/tty/tty_io.c
+@@ -830,65 +830,6 @@
+ time->tv_sec = sec;
+ }
+
+-/*
+- * Iterate on the ldisc ->read() function until we've gotten all
+- * the data the ldisc has for us.
+- *
+- * The "cookie" is something that the ldisc read function can fill
+- * in to let us know that there is more data to be had.
+- *
+- * We promise to continue to call the ldisc until it stops returning
+- * data or clears the cookie. The cookie may be something that the
+- * ldisc maintains state for and needs to free.
+- */
+-static int iterate_tty_read(struct tty_ldisc *ld, struct tty_struct *tty, struct file *file,
+- char __user *buf, size_t count)
+-{
+- int retval = 0;
+- void *cookie = NULL;
+- unsigned long offset = 0;
+- char kernel_buf[64];
+-
+- do {
+- int size, uncopied;
+-
+- size = count > sizeof(kernel_buf) ? sizeof(kernel_buf) : count;
+- size = ld->ops->read(tty, file, kernel_buf, size, &cookie, offset);
+- if (!size)
+- break;
+-
+- /*
+- * A ldisc read error return will override any previously copied
+- * data (eg -EOVERFLOW from HDLC)
+- */
+- if (size < 0) {
+- memzero_explicit(kernel_buf, sizeof(kernel_buf));
+- return size;
+- }
+-
+- uncopied = copy_to_user(buf+offset, kernel_buf, size);
+- size -= uncopied;
+- offset += size;
+- count -= size;
+-
+- /*
+- * If the user copy failed, we still need to do another ->read()
+- * call if we had a cookie to let the ldisc clear up.
+- *
+- * But make sure size is zeroed.
+- */
+- if (unlikely(uncopied)) {
+- count = 0;
+- retval = -EFAULT;
+- }
+- } while (cookie);
+-
+- /* We always clear tty buffer in case they contained passwords */
+- memzero_explicit(kernel_buf, sizeof(kernel_buf));
+- return offset ? offset : retval;
+-}
+-
+-
+ /**
+ * tty_read - read method for tty device files
+ * @file: pointer to tty file
+@@ -922,9 +863,10 @@
+ ld = tty_ldisc_ref_wait(tty);
+ if (!ld)
+ return hung_up_tty_read(file, buf, count, ppos);
+- i = -EIO;
+ if (ld->ops->read)
+- i = iterate_tty_read(ld, tty, file, buf, count);
++ i = ld->ops->read(tty, file, buf, count);
++ else
++ i = -EIO;
+ tty_ldisc_deref(ld);
+
+ if (i > 0)
+diff --git a/include/linux/tty_ldisc.h b/include/linux/tty_ldisc.h
+index b1e6043e99175..572a079761165 100644
+--- b/include/linux/tty_ldisc.h
++++ a/include/linux/tty_ldisc.h
+@@ -185,8 +185,7 @@
+ void (*close)(struct tty_struct *);
+ void (*flush_buffer)(struct tty_struct *tty);
+ ssize_t (*read)(struct tty_struct *tty, struct file *file,
+- unsigned char *buf, size_t nr,
+- void **cookie, unsigned long offset);
++ unsigned char __user *buf, size_t nr);
+ ssize_t (*write)(struct tty_struct *tty, struct file *file,
+ const unsigned char *buf, size_t nr);
+ int (*ioctl)(struct tty_struct *tty, struct file *file,
+diff --git a/net/nfc/nci/uart.c b/net/nfc/nci/uart.c
+index 11b554ce07ffc..1204c438e87dc 100644
+--- b/net/nfc/nci/uart.c
++++ a/net/nfc/nci/uart.c
+@@ -292,8 +292,7 @@
+
+ /* We don't provide read/write/poll interface for user space. */
+ static ssize_t nci_uart_tty_read(struct tty_struct *tty, struct file *file,
+- unsigned char *buf, size_t nr,
+- void **cookie, unsigned long offset)
++ unsigned char __user *buf, size_t nr)
+ {
+ return 0;
+ }
--
2.30.1
^ permalink raw reply related [flat|nested] 7+ messages in thread