unofficial mirror of bug-guix@gnu.org 
 help / color / mirror / code / Atom feed
From: Mark H Weaver <mhw@netris.org>
To: 46981@debbugs.gnu.org
Subject: bug#46981: Severe Emacs shell mode performance regression in recent Linux-libre
Date: Sat, 06 Mar 2021 21:56:28 -0500	[thread overview]
Message-ID: <87a6rfisco.fsf@netris.org> (raw)
In-Reply-To: <87czwbisfm.fsf@netris.org>

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


  reply	other threads:[~2021-03-07  2:59 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-07  0:38 bug#46981: Severe Emacs shell mode performance regression in recent Linux-libre Mark H Weaver
2021-03-07  2:54 ` Mark H Weaver
2021-03-07  2:56   ` Mark H Weaver [this message]
2021-03-07  8:53   ` Bengt Richter
2021-03-07 14:51   ` Christopher Baines
2021-03-08  4:59     ` Mark H Weaver
2021-03-10 11:23       ` Ludovic Courtès

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://guix.gnu.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87a6rfisco.fsf@netris.org \
    --to=mhw@netris.org \
    --cc=46981@debbugs.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).