tty: hold lock across tty buffer finding and buffer filling

tty_buffer_request_room is well protected, but while after it returns, it releases the port->lock. tty->buf.tail might be modified by either irq handler or other threads. The patch adds more protection by holding the lock across tty buffer finding and buffer filling. Signed-off-by: Alek Du <alek.du@intel.com> Signed-off-by: Xiaobing Tu <xiaobing.tu@intel.com> Cc: Jiri Slaby <jslaby@suse.cz> Cc: Alan Cox <alan@lxorguk.ukuu.org.uk> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

tty: hold lock across tty buffer finding and buffer filling
tty_buffer_request_room is well protected, but while after it returns, it releases the port->lock. tty->buf.tail might be modified by either irq handler or other threads. The patch adds more protection by holding the lock across tty buffer finding and buffer filling. Signed-off-by: Alek Du <alek.du@intel.com> Signed-off-by: Xiaobing Tu <xiaobing.tu@intel.com> Cc: Jiri Slaby <jslaby@suse.cz> Cc: Alan Cox <alan@lxorguk.ukuu.org.uk> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Xiaobing Tu · Greg Kroah-Hartman
1 parent 871bdea6f8
Showing 1 changed file with 65 additions and 20 deletions Side-by-side Diff
drivers/tty/tty_buffer.c
@@ -185,25 +185,19 @@
 	/* Should possibly check if this fails for the largest buffer we
 	   have queued and recycle that ? */
 }
-
 /**
- *	tty_buffer_request_room		-	grow tty buffer if needed
+ *	__tty_buffer_request_room		-	grow tty buffer if needed
  *	@tty: tty structure
  *	@size: size desired
  *
  *	Make at least size bytes of linear space available for the tty
  *	buffer. If we fail return the size we managed to find.
- *
- *	Locking: Takes tty->buf.lock
+ *      Locking: Caller must hold tty->buf.lock
  */
-int tty_buffer_request_room(struct tty_struct *tty, size_t size)
+static int __tty_buffer_request_room(struct tty_struct *tty, size_t size)
 {
 	struct tty_buffer *b, *n;
 	int left;
-	unsigned long flags;
-
-	spin_lock_irqsave(&tty->buf.lock, flags);
-
 	/* OPTIMISATION: We could keep a per tty "zero" sized buffer to
 	   remove this conditional if its worth it. This would be invisible
 	   to the callers */
  
@@ -225,9 +219,30 @@
 			size = left;
 	}
  
-	spin_unlock_irqrestore(&tty->buf.lock, flags);
 	return size;
 }
+
+
+/**
+ *	tty_buffer_request_room		-	grow tty buffer if needed
+ *	@tty: tty structure
+ *	@size: size desired
+ *
+ *	Make at least size bytes of linear space available for the tty
+ *	buffer. If we fail return the size we managed to find.
+ *
+ *	Locking: Takes tty->buf.lock
+ */
+int tty_buffer_request_room(struct tty_struct *tty, size_t size)
+{
+	unsigned long flags;
+	int length;
+
+	spin_lock_irqsave(&tty->buf.lock, flags);
+	length = __tty_buffer_request_room(tty, size);
+	spin_unlock_irqrestore(&tty->buf.lock, flags);
+	return length;
+}
 EXPORT_SYMBOL_GPL(tty_buffer_request_room);
  
 /**
  
  
  
@@ -249,14 +264,22 @@
 	int copied = 0;
 	do {
 		int goal = min_t(size_t, size - copied, TTY_BUFFER_PAGE);
-		int space = tty_buffer_request_room(tty, goal);
-		struct tty_buffer *tb = tty->buf.tail;
+		int space;
+		unsigned long flags;
+		struct tty_buffer *tb;
+
+		spin_lock_irqsave(&tty->buf.lock, flags);
+		space = __tty_buffer_request_room(tty, goal);
+		tb = tty->buf.tail;
 		/* If there is no space then tb may be NULL */
-		if (unlikely(space == 0))
+		if (unlikely(space == 0)) {
+			spin_unlock_irqrestore(&tty->buf.lock, flags);
 			break;
+		}
 		memcpy(tb->char_buf_ptr + tb->used, chars, space);
 		memset(tb->flag_buf_ptr + tb->used, flag, space);
 		tb->used += space;
+		spin_unlock_irqrestore(&tty->buf.lock, flags);
 		copied += space;
 		chars += space;
 		/* There is a small chance that we need to split the data over
  
  
  
@@ -286,14 +309,22 @@
 	int copied = 0;
 	do {
 		int goal = min_t(size_t, size - copied, TTY_BUFFER_PAGE);
-		int space = tty_buffer_request_room(tty, goal);
-		struct tty_buffer *tb = tty->buf.tail;
+		int space;
+		unsigned long __flags;
+		struct tty_buffer *tb;
+
+		spin_lock_irqsave(&tty->buf.lock, __flags);
+		space = __tty_buffer_request_room(tty, goal);
+		tb = tty->buf.tail;
 		/* If there is no space then tb may be NULL */
-		if (unlikely(space == 0))
+		if (unlikely(space == 0)) {
+			spin_unlock_irqrestore(&tty->buf.lock, __flags);
 			break;
+		}
 		memcpy(tb->char_buf_ptr + tb->used, chars, space);
 		memcpy(tb->flag_buf_ptr + tb->used, flags, space);
 		tb->used += space;
+		spin_unlock_irqrestore(&tty->buf.lock, __flags);
 		copied += space;
 		chars += space;
 		flags += space;
  
  
@@ -344,13 +375,20 @@
 int tty_prepare_flip_string(struct tty_struct *tty, unsigned char **chars,
 								size_t size)
 {
-	int space = tty_buffer_request_room(tty, size);
+	int space;
+	unsigned long flags;
+	struct tty_buffer *tb;
+
+	spin_lock_irqsave(&tty->buf.lock, flags);
+	space = __tty_buffer_request_room(tty, size);
+
+	tb = tty->buf.tail;
 	if (likely(space)) {
-		struct tty_buffer *tb = tty->buf.tail;
 		*chars = tb->char_buf_ptr + tb->used;
 		memset(tb->flag_buf_ptr + tb->used, TTY_NORMAL, space);
 		tb->used += space;
 	}
+	spin_unlock_irqrestore(&tty->buf.lock, flags);
 	return space;
 }
 EXPORT_SYMBOL_GPL(tty_prepare_flip_string);
  
  
@@ -374,13 +412,20 @@
 int tty_prepare_flip_string_flags(struct tty_struct *tty,
 			unsigned char **chars, char **flags, size_t size)
 {
-	int space = tty_buffer_request_room(tty, size);
+	int space;
+	unsigned long __flags;
+	struct tty_buffer *tb;
+
+	spin_lock_irqsave(&tty->buf.lock, __flags);
+	space = __tty_buffer_request_room(tty, size);
+
+	tb = tty->buf.tail;
 	if (likely(space)) {
-		struct tty_buffer *tb = tty->buf.tail;
 		*chars = tb->char_buf_ptr + tb->used;
 		*flags = tb->flag_buf_ptr + tb->used;
 		tb->used += space;
 	}
+	spin_unlock_irqrestore(&tty->buf.lock, __flags);
 	return space;
 }
 EXPORT_SYMBOL_GPL(tty_prepare_flip_string_flags);
...	...	@@ -185,25 +185,19 @@
185	185	/* Should possibly check if this fails for the largest buffer we
186	186	have queued and recycle that ? */
187	187	}
188		-
189	188	/**
190		- * tty_buffer_request_room - grow tty buffer if needed
	189	+ * __tty_buffer_request_room - grow tty buffer if needed
191	190	* @tty: tty structure
192	191	* @size: size desired
193	192	*
194	193	* Make at least size bytes of linear space available for the tty
195	194	* buffer. If we fail return the size we managed to find.
196		- *
197		- * Locking: Takes tty->buf.lock
	195	+ * Locking: Caller must hold tty->buf.lock
198	196	*/
199		-int tty_buffer_request_room(struct tty_struct *tty, size_t size)
	197	+static int __tty_buffer_request_room(struct tty_struct *tty, size_t size)
200	198	{
201	199	struct tty_buffer b, n;
202	200	int left;
203		- unsigned long flags;
204		-
205		- spin_lock_irqsave(&tty->buf.lock, flags);
206		-
207	201	/* OPTIMISATION: We could keep a per tty "zero" sized buffer to
208	202	remove this conditional if its worth it. This would be invisible
209	203	to the callers */
210	204
...	...	@@ -225,9 +219,30 @@
225	219	size = left;
226	220	}
227	221
228		- spin_unlock_irqrestore(&tty->buf.lock, flags);
229	222	return size;
230	223	}
	224	+
	225	+
	226	+/**
	227	+ * tty_buffer_request_room - grow tty buffer if needed
	228	+ * @tty: tty structure
	229	+ * @size: size desired
	230	+ *
	231	+ * Make at least size bytes of linear space available for the tty
	232	+ * buffer. If we fail return the size we managed to find.
	233	+ *
	234	+ * Locking: Takes tty->buf.lock
	235	+ */
	236	+int tty_buffer_request_room(struct tty_struct *tty, size_t size)
	237	+{
	238	+ unsigned long flags;
	239	+ int length;
	240	+
	241	+ spin_lock_irqsave(&tty->buf.lock, flags);
	242	+ length = __tty_buffer_request_room(tty, size);
	243	+ spin_unlock_irqrestore(&tty->buf.lock, flags);
	244	+ return length;
	245	+}
231	246	EXPORT_SYMBOL_GPL(tty_buffer_request_room);
232	247
233	248	/**
234	249
235	250
236	251
...	...	@@ -249,14 +264,22 @@
249	264	int copied = 0;
250	265	do {
251	266	int goal = min_t(size_t, size - copied, TTY_BUFFER_PAGE);
252		- int space = tty_buffer_request_room(tty, goal);
253		- struct tty_buffer *tb = tty->buf.tail;
	267	+ int space;
	268	+ unsigned long flags;
	269	+ struct tty_buffer *tb;
	270	+
	271	+ spin_lock_irqsave(&tty->buf.lock, flags);
	272	+ space = __tty_buffer_request_room(tty, goal);
	273	+ tb = tty->buf.tail;
254	274	/* If there is no space then tb may be NULL */
255		- if (unlikely(space == 0))
	275	+ if (unlikely(space == 0)) {
	276	+ spin_unlock_irqrestore(&tty->buf.lock, flags);
256	277	break;
	278	+ }
257	279	memcpy(tb->char_buf_ptr + tb->used, chars, space);
258	280	memset(tb->flag_buf_ptr + tb->used, flag, space);
259	281	tb->used += space;
	282	+ spin_unlock_irqrestore(&tty->buf.lock, flags);
260	283	copied += space;
261	284	chars += space;
262	285	/* There is a small chance that we need to split the data over
263	286
264	287
265	288
...	...	@@ -286,14 +309,22 @@
286	309	int copied = 0;
287	310	do {
288	311	int goal = min_t(size_t, size - copied, TTY_BUFFER_PAGE);
289		- int space = tty_buffer_request_room(tty, goal);
290		- struct tty_buffer *tb = tty->buf.tail;
	312	+ int space;
	313	+ unsigned long __flags;
	314	+ struct tty_buffer *tb;
	315	+
	316	+ spin_lock_irqsave(&tty->buf.lock, __flags);
	317	+ space = __tty_buffer_request_room(tty, goal);
	318	+ tb = tty->buf.tail;
291	319	/* If there is no space then tb may be NULL */
292		- if (unlikely(space == 0))
	320	+ if (unlikely(space == 0)) {
	321	+ spin_unlock_irqrestore(&tty->buf.lock, __flags);
293	322	break;
	323	+ }
294	324	memcpy(tb->char_buf_ptr + tb->used, chars, space);
295	325	memcpy(tb->flag_buf_ptr + tb->used, flags, space);
296	326	tb->used += space;
	327	+ spin_unlock_irqrestore(&tty->buf.lock, __flags);
297	328	copied += space;
298	329	chars += space;
299	330	flags += space;
300	331
301	332
...	...	@@ -344,13 +375,20 @@
344	375	int tty_prepare_flip_string(struct tty_struct tty, unsigned char *chars,
345	376	size_t size)
346	377	{
347		- int space = tty_buffer_request_room(tty, size);
	378	+ int space;
	379	+ unsigned long flags;
	380	+ struct tty_buffer *tb;
	381	+
	382	+ spin_lock_irqsave(&tty->buf.lock, flags);
	383	+ space = __tty_buffer_request_room(tty, size);
	384	+
	385	+ tb = tty->buf.tail;
348	386	if (likely(space)) {
349		- struct tty_buffer *tb = tty->buf.tail;
350	387	*chars = tb->char_buf_ptr + tb->used;
351	388	memset(tb->flag_buf_ptr + tb->used, TTY_NORMAL, space);
352	389	tb->used += space;
353	390	}
	391	+ spin_unlock_irqrestore(&tty->buf.lock, flags);
354	392	return space;
355	393	}
356	394	EXPORT_SYMBOL_GPL(tty_prepare_flip_string);
357	395
358	396
...	...	@@ -374,13 +412,20 @@
374	412	int tty_prepare_flip_string_flags(struct tty_struct *tty,
375	413	unsigned char chars, char flags, size_t size)
376	414	{
377		- int space = tty_buffer_request_room(tty, size);
	415	+ int space;
	416	+ unsigned long __flags;
	417	+ struct tty_buffer *tb;
	418	+
	419	+ spin_lock_irqsave(&tty->buf.lock, __flags);
	420	+ space = __tty_buffer_request_room(tty, size);
	421	+
	422	+ tb = tty->buf.tail;
378	423	if (likely(space)) {
379		- struct tty_buffer *tb = tty->buf.tail;
380	424	*chars = tb->char_buf_ptr + tb->used;
381	425	*flags = tb->flag_buf_ptr + tb->used;
382	426	tb->used += space;
383	427	}
	428	+ spin_unlock_irqrestore(&tty->buf.lock, __flags);
384	429	return space;
385	430	}
386	431	EXPORT_SYMBOL_GPL(tty_prepare_flip_string_flags);