All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Hurley <peter@hurleysoftware.com>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jiri Slaby <jslaby@suse.cz>,
	linux-kernel@vger.kernel.org, linux-serial@vger.kernel.org,
	Peter Hurley <peter@hurleysoftware.com>
Subject: [PATCH 3/4] n_tty: Fix unsafe update of available buffer space
Date: Sat, 15 Jun 2013 07:28:30 -0400	[thread overview]
Message-ID: <1371295711-3963-4-git-send-email-peter@hurleysoftware.com> (raw)
In-Reply-To: <1371295711-3963-1-git-send-email-peter@hurleysoftware.com>

receive_room is used to control the amount of data the flip
buffer work can push to the read buffer. This update is unsafe:

  CPU 0                        |  CPU 1
                               |
                               | n_tty_read()
                               |   n_tty_set_room()
                               |     left = <calc of space>
n_tty_receive_buf()            |
  <push data to buffer>        |
  n_tty_set_room()             |
    left = <calc of space>     |
    tty->receive_room = left   |
                               |     tty->receive_room = left

receive_room is now updated with a stale calculation of the
available buffer space, and the subsequent work loop will likely
overwrite unread data in the input buffer.

Update receive_room atomically with the calculation of the
available buffer space.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/n_tty.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index fa5cb46..0646165 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -115,13 +115,14 @@ static inline int tty_put_user(struct tty_struct *tty, unsigned char x,
 }
 
 /**
- *	n_tty_set__room	-	receive space
+ *	n_tty_set_room	-	receive space
  *	@tty: terminal
  *
- *	Called by the driver to find out how much data it is
- *	permitted to feed to the line discipline without any being lost
- *	and thus to manage flow control. Not serialized. Answers for the
- *	"instant".
+ *	Sets tty->receive_room to reflect the currently available space
+ *	in the input buffer, and re-schedules the flip buffer work if space
+ *	just became available.
+ *
+ *	Locks: Concurrent update is protected with read_lock
  */
 
 static void n_tty_set_room(struct tty_struct *tty)
@@ -129,8 +130,10 @@ static void n_tty_set_room(struct tty_struct *tty)
 	struct n_tty_data *ldata = tty->disc_data;
 	int left;
 	int old_left;
+	unsigned long flags;
+
+	raw_spin_lock_irqsave(&ldata->read_lock, flags);
 
-	/* ldata->read_cnt is not read locked ? */
 	if (I_PARMRK(tty)) {
 		/* Multiply read_cnt by 3, since each byte might take up to
 		 * three times as many spaces when PARMRK is set (depending on
@@ -150,6 +153,8 @@ static void n_tty_set_room(struct tty_struct *tty)
 	old_left = tty->receive_room;
 	tty->receive_room = left;
 
+	raw_spin_unlock_irqrestore(&ldata->read_lock, flags);
+
 	/* Did this open up the receive buffer? We may need to flip */
 	if (left && !old_left) {
 		WARN_RATELIMIT(tty->port->itty == NULL,
@@ -1872,7 +1877,6 @@ do_it_again:
 				retval = -ERESTARTSYS;
 				break;
 			}
-			/* FIXME: does n_tty_set_room need locking ? */
 			n_tty_set_room(tty);
 			timeout = schedule_timeout(timeout);
 			continue;
-- 
1.8.1.2

  parent reply	other threads:[~2013-06-15 11:28 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-03-06 13:38 [PATCH 0/7] n_tty fixes Peter Hurley
2013-03-06 13:38 ` [PATCH 1/7] n_tty: Fix unsafe driver-side signals Peter Hurley
2013-03-06 13:38 ` [PATCH 2/7] n_tty: Lock access to tty->pgrp for POSIX job control Peter Hurley
2013-03-06 13:38 ` [PATCH 3/7] tty: Fix checkpatch errors in tty_ldisc.h Peter Hurley
2013-03-06 13:38 ` [PATCH 4/7] n_tty: Encapsulate minimum_to_wake within N_TTY Peter Hurley
2013-03-18 23:15   ` Greg Kroah-Hartman
2013-03-19 13:57     ` Peter Hurley
2013-03-19 14:10       ` Greg Kroah-Hartman
2013-03-19 14:26         ` Peter Hurley
2013-03-19 14:26           ` [PATCH 5/7] n_tty: Untangle read completion variables Peter Hurley
2013-03-19 14:26           ` [PATCH 6/7] n_tty: Fix unsafe update of available buffer space Peter Hurley
2013-03-19 20:26             ` Ilya Zykov
2013-03-19 14:26           ` [PATCH 7/7] n_tty: Buffer work should not reschedule itself Peter Hurley
2013-03-19 20:27             ` Ilya Zykov
2013-06-15 11:28         ` [PATCH 0/4] [resend] n_tty fixes Peter Hurley
2013-06-15 11:28           ` [PATCH 1/4] n_tty: Encapsulate minimum_to_wake within N_TTY Peter Hurley
2013-06-15 11:28           ` [PATCH 2/4] n_tty: Untangle read completion variables Peter Hurley
2013-06-15 11:28           ` Peter Hurley [this message]
2013-06-15 11:28           ` [PATCH 4/4] n_tty: Buffer work should not reschedule itself Peter Hurley
2013-06-17 19:58           ` [PATCH 0/4] [resend] n_tty fixes Greg Kroah-Hartman
2013-03-06 13:38 ` [PATCH 5/7] n_tty: Untangle read completion variables Peter Hurley
2013-03-06 13:38 ` [PATCH 6/7] n_tty: Fix unsafe update of available buffer space Peter Hurley
2013-03-06 13:38 ` [PATCH 7/7] n_tty: Buffer work should not reschedule itself Peter Hurley

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=1371295711-3963-4-git-send-email-peter@hurleysoftware.com \
    --to=peter@hurleysoftware.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=jslaby@suse.cz \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-serial@vger.kernel.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.
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.