All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org
Cc: "Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	stable@vger.kernel.org, 一只狗 <chennbnbnb@gmail.com>,
	"Dan Carpenter" <dan.carpenter@oracle.com>,
	"Hillf Danton" <hdanton@sina.com>, "Jiri Slaby" <jslaby@suse.cz>
Subject: [PATCH 4.9 25/26] tty: use new tty_insert_flip_string_and_push_buffer() in pty_write()
Date: Wed, 27 Jul 2022 18:10:54 +0200	[thread overview]
Message-ID: <20220727161000.122591546@linuxfoundation.org> (raw)
In-Reply-To: <20220727160959.122591422@linuxfoundation.org>

From: Jiri Slaby <jslaby@suse.cz>

commit a501ab75e7624d133a5a3c7ec010687c8b961d23 upstream.

There is a race in pty_write(). pty_write() can be called in parallel
with e.g. ioctl(TIOCSTI) or ioctl(TCXONC) which also inserts chars to
the buffer. Provided, tty_flip_buffer_push() in pty_write() is called
outside the lock, it can commit inconsistent tail. This can lead to out
of bounds writes and other issues. See the Link below.

To fix this, we have to introduce a new helper called
tty_insert_flip_string_and_push_buffer(). It does both
tty_insert_flip_string() and tty_flip_buffer_commit() under the port
lock. It also calls queue_work(), but outside the lock. See
71a174b39f10 (pty: do tty_flip_buffer_push without port->lock in
pty_write) for the reasons.

Keep the helper internal-only (in drivers' tty.h). It is not intended to
be used widely.

Link: https://seclists.org/oss-sec/2022/q2/155
Fixes: 71a174b39f10 (pty: do tty_flip_buffer_push without port->lock in pty_write)
Cc: 一只狗 <chennbnbnb@gmail.com>
Cc: Dan Carpenter <dan.carpenter@oracle.com>
Suggested-by: Hillf Danton <hdanton@sina.com>
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Link: https://lore.kernel.org/r/20220707082558.9250-2-jslaby@suse.cz
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/tty/pty.c        |   14 ++------------
 drivers/tty/tty_buffer.c |   31 +++++++++++++++++++++++++++++++
 include/linux/tty_flip.h |    3 +++
 3 files changed, 36 insertions(+), 12 deletions(-)

--- a/drivers/tty/pty.c
+++ b/drivers/tty/pty.c
@@ -106,21 +106,11 @@ static void pty_unthrottle(struct tty_st
 static int pty_write(struct tty_struct *tty, const unsigned char *buf, int c)
 {
 	struct tty_struct *to = tty->link;
-	unsigned long flags;
 
-	if (tty->stopped)
+	if (tty->stopped || !c)
 		return 0;
 
-	if (c > 0) {
-		spin_lock_irqsave(&to->port->lock, flags);
-		/* Stuff the data into the input queue of the other end */
-		c = tty_insert_flip_string(to->port, buf, c);
-		spin_unlock_irqrestore(&to->port->lock, flags);
-		/* And shovel */
-		if (c)
-			tty_flip_buffer_push(to->port);
-	}
-	return c;
+	return tty_insert_flip_string_and_push_buffer(to->port, buf, c);
 }
 
 /**
--- a/drivers/tty/tty_buffer.c
+++ b/drivers/tty/tty_buffer.c
@@ -556,6 +556,37 @@ void tty_flip_buffer_push(struct tty_por
 EXPORT_SYMBOL(tty_flip_buffer_push);
 
 /**
+ * tty_insert_flip_string_and_push_buffer - add characters to the tty buffer and
+ *	push
+ * @port: tty port
+ * @chars: characters
+ * @size: size
+ *
+ * The function combines tty_insert_flip_string() and tty_flip_buffer_push()
+ * with the exception of properly holding the @port->lock.
+ *
+ * To be used only internally (by pty currently).
+ *
+ * Returns: the number added.
+ */
+int tty_insert_flip_string_and_push_buffer(struct tty_port *port,
+		const unsigned char *chars, size_t size)
+{
+	struct tty_bufhead *buf = &port->buf;
+	unsigned long flags;
+
+	spin_lock_irqsave(&port->lock, flags);
+	size = tty_insert_flip_string(port, chars, size);
+	if (size)
+		tty_flip_buffer_commit(buf->tail);
+	spin_unlock_irqrestore(&port->lock, flags);
+
+	queue_work(system_unbound_wq, &buf->work);
+
+	return size;
+}
+
+/**
  *	tty_buffer_init		-	prepare a tty buffer structure
  *	@tty: tty to initialise
  *
--- a/include/linux/tty_flip.h
+++ b/include/linux/tty_flip.h
@@ -38,4 +38,7 @@ static inline int tty_insert_flip_string
 extern void tty_buffer_lock_exclusive(struct tty_port *port);
 extern void tty_buffer_unlock_exclusive(struct tty_port *port);
 
+int tty_insert_flip_string_and_push_buffer(struct tty_port *port,
+		const unsigned char *chars, size_t cnt);
+
 #endif /* _LINUX_TTY_FLIP_H */



  parent reply	other threads:[~2022-07-27 16:24 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-27 16:10 [PATCH 4.9 00/26] 4.9.325-rc1 review Greg Kroah-Hartman
2022-07-27 16:10 ` [PATCH 4.9 01/26] security,selinux,smack: kill security_task_wait hook Greg Kroah-Hartman
2022-07-27 16:10 ` [PATCH 4.9 02/26] xen/gntdev: Ignore failure to unmap INVALID_GRANT_HANDLE Greg Kroah-Hartman
2022-07-27 16:10 ` [PATCH 4.9 03/26] misc: rtsx_usb: fix use of dma mapped buffer for usb bulk transfer Greg Kroah-Hartman
2022-07-27 16:10 ` [PATCH 4.9 04/26] misc: rtsx_usb: use separate command and response buffers Greg Kroah-Hartman
2022-07-27 16:10 ` [PATCH 4.9 05/26] misc: rtsx_usb: set return value in rsp_buf alloc err path Greg Kroah-Hartman
2022-07-27 16:10 ` [PATCH 4.9 06/26] xfrm: xfrm_policy: fix a possible double xfrm_pols_put() in xfrm_bundle_lookup() Greg Kroah-Hartman
2022-07-27 16:10 ` [PATCH 4.9 07/26] power/reset: arm-versatile: Fix refcount leak in versatile_reboot_probe Greg Kroah-Hartman
2022-07-27 16:10 ` [PATCH 4.9 08/26] perf/core: Fix data race between perf_event_set_output() and perf_mmap_close() Greg Kroah-Hartman
2022-07-27 16:10 ` [PATCH 4.9 09/26] ip: Fix a data-race around sysctl_fwmark_reflect Greg Kroah-Hartman
2022-07-27 16:10 ` [PATCH 4.9 10/26] tcp/dccp: Fix a data-race around sysctl_tcp_fwmark_accept Greg Kroah-Hartman
2022-07-27 16:10 ` [PATCH 4.9 11/26] tcp: Fix a data-race around sysctl_tcp_probe_threshold Greg Kroah-Hartman
2022-07-27 16:10 ` [PATCH 4.9 12/26] i2c: cadence: Change large transfer count reset logic to be unconditional Greg Kroah-Hartman
2022-07-27 16:10 ` [PATCH 4.9 13/26] igmp: Fix data-races around sysctl_igmp_llm_reports Greg Kroah-Hartman
2022-07-27 16:10 ` [PATCH 4.9 14/26] igmp: Fix a data-race around sysctl_igmp_max_memberships Greg Kroah-Hartman
2022-07-27 16:10 ` [PATCH 4.9 15/26] tcp: Fix a data-race around sysctl_tcp_notsent_lowat Greg Kroah-Hartman
2022-07-27 16:10 ` [PATCH 4.9 16/26] be2net: Fix buffer overflow in be_get_module_eeprom Greg Kroah-Hartman
2022-07-27 16:10 ` [PATCH 4.9 17/26] Revert "Revert "char/random: silence a lockdep splat with printk()"" Greg Kroah-Hartman
2022-07-27 16:10 ` [PATCH 4.9 18/26] mm/mempolicy: fix uninit-value in mpol_rebind_policy() Greg Kroah-Hartman
2022-07-27 16:10 ` [PATCH 4.9 19/26] bpf: Make sure mac_header was set before using it Greg Kroah-Hartman
2022-07-27 16:10 ` [PATCH 4.9 20/26] ALSA: memalloc: Align buffer allocations in page size Greg Kroah-Hartman
2022-07-27 16:10 ` [PATCH 4.9 21/26] tty: drivers/tty/, stop using tty_schedule_flip() Greg Kroah-Hartman
2022-07-27 16:10 ` [PATCH 4.9 22/26] tty: the rest, " Greg Kroah-Hartman
2022-07-27 16:10 ` [PATCH 4.9 23/26] tty: drop tty_schedule_flip() Greg Kroah-Hartman
2022-07-27 16:10 ` [PATCH 4.9 24/26] tty: extract tty_flip_buffer_commit() from tty_flip_buffer_push() Greg Kroah-Hartman
2022-07-27 16:10 ` Greg Kroah-Hartman [this message]
2022-07-27 16:10 ` [PATCH 4.9 26/26] net: usb: ax88179_178a needs FLAG_SEND_ZLP Greg Kroah-Hartman
2022-07-27 22:18 ` [PATCH 4.9 00/26] 4.9.325-rc1 review Florian Fainelli
2022-07-28  9:42 ` Pavel Machek
2022-07-28 10:08 ` Naresh Kamboju
2022-07-28 14:32 ` Jon Hunter
2022-07-28 14:52 ` Shuah Khan
2022-07-28 22:57 ` Guenter Roeck

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

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

  git send-email \
    --in-reply-to=20220727161000.122591546@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=chennbnbnb@gmail.com \
    --cc=dan.carpenter@oracle.com \
    --cc=hdanton@sina.com \
    --cc=jslaby@suse.cz \
    --cc=linux-kernel@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    --subject='Re: [PATCH 4.9 25/26] tty: use new tty_insert_flip_string_and_push_buffer() in pty_write()' \
    /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

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.