tty: Complete ownership transfer of flip buffers

Waiting for buffer work to complete is not required for safely performing changes to the line discipline, once the line discipline is halted. The buffer work routine, flush_to_ldisc(), will be unable to acquire an ldisc ref and all existing references were waited until released (so it can't already have one). Ensure running buffer work which may reference the soon-to-be-gone tty completes and any buffer work running after this point retrieves a NULL tty. Also, ensure all buffer work is cancelled on port destruction. Signed-off-by: Peter Hurley <peter@hurleysoftware.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

tty: Complete ownership transfer of flip buffers
Waiting for buffer work to complete is not required for safely performing changes to the line discipline, once the line discipline is halted. The buffer work routine, flush_to_ldisc(), will be unable to acquire an ldisc ref and all existing references were waited until released (so it can't already have one). Ensure running buffer work which may reference the soon-to-be-gone tty completes and any buffer work running after this point retrieves a NULL tty. Also, ensure all buffer work is cancelled on port destruction. Signed-off-by: Peter Hurley <peter@hurleysoftware.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Peter Hurley · Greg Kroah-Hartman
1 parent d912156605
Showing 3 changed files with 14 additions and 35 deletions Side-by-side Diff
drivers/tty/tty_io.c
drivers/tty/tty_ldisc.c
drivers/tty/tty_port.c
@@ -1595,6 +1595,7 @@
 	tty_free_termios(tty);
 	tty_driver_remove_tty(tty->driver, tty);
 	tty->port->itty = NULL;
+	cancel_work_sync(&tty->port->buf.work);
  
 	if (tty->link)
 		tty_kref_put(tty->link);
@@ -508,7 +508,6 @@
 {
 	flush_work(&tty->SAK_work);
 	flush_work(&tty->hangup_work);
-	flush_work(&tty->port->buf.work);
 }
  
 /**
  
  
  
  
@@ -531,30 +530,22 @@
  *	tty_ldisc_halt		-	shut down the line discipline
  *	@tty: tty device
  *	@o_tty: paired pty device (can be NULL)
- *	@pending: returns true if work was scheduled when cancelled
- *		  (can be set to NULL)
- *	@o_pending: returns true if work was scheduled when cancelled
- *		    (can be set to NULL)
  *	@timeout: # of jiffies to wait for ldisc refs to be released
  *
  *	Shut down the line discipline and work queue for this tty device and
  *	its paired pty (if exists). Clearing the TTY_LDISC flag ensures
- *	no further references can be obtained while the work queue halt
- *	ensures that no more data is fed to the ldisc.
+ *	no further references can be obtained, while waiting for existing
+ *	references to be released ensures no more data is fed to the ldisc.
  *
- *	Furthermore, guarantee that existing ldisc references have been
- *	released, which in turn, guarantees that no future buffer work
- *	can be rescheduled.
- *
  *	You need to do a 'flush_scheduled_work()' (outside the ldisc_mutex)
  *	in order to make sure any currently executing ldisc work is also
  *	flushed.
  */
  
 static int tty_ldisc_halt(struct tty_struct *tty, struct tty_struct *o_tty,
-			  int *pending, int *o_pending, long timeout)
+			  long timeout)
 {
-	int scheduled, o_scheduled, retval;
+	int retval;
  
 	clear_bit(TTY_LDISC, &tty->flags);
 	if (o_tty)
  
  
  
@@ -566,17 +557,10 @@
 	if (retval)
 		return retval;
  
-	scheduled = cancel_work_sync(&tty->port->buf.work);
 	set_bit(TTY_LDISC_HALTED, &tty->flags);
-	if (o_tty) {
-		o_scheduled = cancel_work_sync(&o_tty->port->buf.work);
+	if (o_tty)
 		set_bit(TTY_LDISC_HALTED, &o_tty->flags);
-	}
  
-	if (pending)
-		*pending = scheduled;
-	if (o_tty && o_pending)
-		*o_pending = o_scheduled;
 	return 0;
 }
  
  
@@ -586,17 +570,12 @@
  *
  *	Shut down the line discipline and work queue for the tty device
  *	being hungup. Clear the TTY_LDISC flag to ensure no further
- *	references can be obtained, wait for remaining references to be
- *	released, and cancel pending buffer work to ensure no more
- *	data is fed to this ldisc.
+ *	references can be obtained and wait for remaining references to be
+ *	released to ensure no more data is fed to this ldisc.
  *	Caller must hold legacy and ->ldisc_mutex.
  *
  *	NB: tty_set_ldisc() is prevented from changing the ldisc concurrently
  *	with this function by checking the TTY_HUPPING flag.
- *
- *	NB: if tty->ldisc is NULL then buffer work does not need to be
- *	cancelled because it must already have done as a precondition
- *	of closing the ldisc and setting tty->ldisc to NULL
  */
 static bool tty_ldisc_hangup_halt(struct tty_struct *tty)
 {
@@ -616,7 +595,6 @@
 				tty_name(tty, tty_n));
 		}
  
-		cancel_work_sync(&tty->port->buf.work);
 		set_bit(TTY_LDISC_HALTED, &tty->flags);
  
 		/* must reacquire both locks and preserve lock order */
@@ -644,7 +622,6 @@
 {
 	int retval;
 	struct tty_ldisc *o_ldisc, *new_ldisc;
-	int work, o_work = 0;
 	struct tty_struct *o_tty;
  
 	new_ldisc = tty_ldisc_get(ldisc);
@@ -718,7 +695,7 @@
 	 *	parallel to the change and re-referencing the tty.
 	 */
  
-	retval = tty_ldisc_halt(tty, o_tty, &work, &o_work, 5 * HZ);
+	retval = tty_ldisc_halt(tty, o_tty, 5 * HZ);
  
 	/*
 	 * Wait for ->hangup_work and ->buf.work handlers to terminate.
  
@@ -782,10 +759,10 @@
  
 	/* Restart the work queue in case no characters kick it off. Safe if
 	   already running */
-	if (work)
-		schedule_work(&tty->port->buf.work);
-	if (o_work)
+	schedule_work(&tty->port->buf.work);
+	if (o_tty)
 		schedule_work(&o_tty->port->buf.work);
+
 	mutex_unlock(&tty->ldisc_mutex);
 	tty_unlock(tty);
 	return retval;
@@ -979,7 +956,7 @@
 	 * race with the set_ldisc code path.
 	 */
  
-	tty_ldisc_halt(tty, o_tty, NULL, NULL, MAX_SCHEDULE_TIMEOUT);
+	tty_ldisc_halt(tty, o_tty, MAX_SCHEDULE_TIMEOUT);
 	tty_ldisc_flush_works(tty);
 	if (o_tty)
 		tty_ldisc_flush_works(o_tty);
@@ -132,6 +132,7 @@
  */
 void tty_port_destroy(struct tty_port *port)
 {
+	cancel_work_sync(&port->buf.work);
 	tty_buffer_free_all(port);
 }
 EXPORT_SYMBOL(tty_port_destroy);
...	...	@@ -1595,6 +1595,7 @@
1595	1595	tty_free_termios(tty);
1596	1596	tty_driver_remove_tty(tty->driver, tty);
1597	1597	tty->port->itty = NULL;
	1598	+ cancel_work_sync(&tty->port->buf.work);
1598	1599
1599	1600	if (tty->link)
1600	1601	tty_kref_put(tty->link);
...	...	@@ -508,7 +508,6 @@
508	508	{
509	509	flush_work(&tty->SAK_work);
510	510	flush_work(&tty->hangup_work);
511		- flush_work(&tty->port->buf.work);
512	511	}
513	512
514	513	/**
515	514
516	515
517	516
518	517
...	...	@@ -531,30 +530,22 @@
531	530	* tty_ldisc_halt - shut down the line discipline
532	531	* @tty: tty device
533	532	* @o_tty: paired pty device (can be NULL)
534		- * @pending: returns true if work was scheduled when cancelled
535		- * (can be set to NULL)
536		- * @o_pending: returns true if work was scheduled when cancelled
537		- * (can be set to NULL)
538	533	* @timeout: # of jiffies to wait for ldisc refs to be released
539	534	*
540	535	* Shut down the line discipline and work queue for this tty device and
541	536	* its paired pty (if exists). Clearing the TTY_LDISC flag ensures
542		- * no further references can be obtained while the work queue halt
543		- * ensures that no more data is fed to the ldisc.
	537	+ * no further references can be obtained, while waiting for existing
	538	+ * references to be released ensures no more data is fed to the ldisc.
544	539	*
545		- * Furthermore, guarantee that existing ldisc references have been
546		- * released, which in turn, guarantees that no future buffer work
547		- * can be rescheduled.
548		- *
549	540	* You need to do a 'flush_scheduled_work()' (outside the ldisc_mutex)
550	541	* in order to make sure any currently executing ldisc work is also
551	542	* flushed.
552	543	*/
553	544
554	545	static int tty_ldisc_halt(struct tty_struct tty, struct tty_struct o_tty,
555		- int pending, int o_pending, long timeout)
	546	+ long timeout)
556	547	{
557		- int scheduled, o_scheduled, retval;
	548	+ int retval;
558	549
559	550	clear_bit(TTY_LDISC, &tty->flags);
560	551	if (o_tty)
561	552
562	553
563	554
...	...	@@ -566,17 +557,10 @@
566	557	if (retval)
567	558	return retval;
568	559
569		- scheduled = cancel_work_sync(&tty->port->buf.work);
570	560	set_bit(TTY_LDISC_HALTED, &tty->flags);
571		- if (o_tty) {
572		- o_scheduled = cancel_work_sync(&o_tty->port->buf.work);
	561	+ if (o_tty)
573	562	set_bit(TTY_LDISC_HALTED, &o_tty->flags);
574		- }
575	563
576		- if (pending)
577		- *pending = scheduled;
578		- if (o_tty && o_pending)
579		- *o_pending = o_scheduled;
580	564	return 0;
581	565	}
582	566
583	567
...	...	@@ -586,17 +570,12 @@
586	570	*
587	571	* Shut down the line discipline and work queue for the tty device
588	572	* being hungup. Clear the TTY_LDISC flag to ensure no further
589		- * references can be obtained, wait for remaining references to be
590		- * released, and cancel pending buffer work to ensure no more
591		- * data is fed to this ldisc.
	573	+ * references can be obtained and wait for remaining references to be
	574	+ * released to ensure no more data is fed to this ldisc.
592	575	* Caller must hold legacy and ->ldisc_mutex.
593	576	*
594	577	* NB: tty_set_ldisc() is prevented from changing the ldisc concurrently
595	578	* with this function by checking the TTY_HUPPING flag.
596		- *
597		- * NB: if tty->ldisc is NULL then buffer work does not need to be
598		- * cancelled because it must already have done as a precondition
599		- * of closing the ldisc and setting tty->ldisc to NULL
600	579	*/
601	580	static bool tty_ldisc_hangup_halt(struct tty_struct *tty)
602	581	{
...	...	@@ -616,7 +595,6 @@
616	595	tty_name(tty, tty_n));
617	596	}
618	597
619		- cancel_work_sync(&tty->port->buf.work);
620	598	set_bit(TTY_LDISC_HALTED, &tty->flags);
621	599
622	600	/* must reacquire both locks and preserve lock order */
...	...	@@ -644,7 +622,6 @@
644	622	{
645	623	int retval;
646	624	struct tty_ldisc o_ldisc, new_ldisc;
647		- int work, o_work = 0;
648	625	struct tty_struct *o_tty;
649	626
650	627	new_ldisc = tty_ldisc_get(ldisc);
...	...	@@ -718,7 +695,7 @@
718	695	* parallel to the change and re-referencing the tty.
719	696	*/
720	697
721		- retval = tty_ldisc_halt(tty, o_tty, &work, &o_work, 5 * HZ);
	698	+ retval = tty_ldisc_halt(tty, o_tty, 5 * HZ);
722	699
723	700	/*
724	701	* Wait for ->hangup_work and ->buf.work handlers to terminate.
725	702
...	...	@@ -782,10 +759,10 @@
782	759
783	760	/* Restart the work queue in case no characters kick it off. Safe if
784	761	already running */
785		- if (work)
786		- schedule_work(&tty->port->buf.work);
787		- if (o_work)
	762	+ schedule_work(&tty->port->buf.work);
	763	+ if (o_tty)
788	764	schedule_work(&o_tty->port->buf.work);
	765	+
789	766	mutex_unlock(&tty->ldisc_mutex);
790	767	tty_unlock(tty);
791	768	return retval;
...	...	@@ -979,7 +956,7 @@
979	956	* race with the set_ldisc code path.
980	957	*/
981	958
982		- tty_ldisc_halt(tty, o_tty, NULL, NULL, MAX_SCHEDULE_TIMEOUT);
	959	+ tty_ldisc_halt(tty, o_tty, MAX_SCHEDULE_TIMEOUT);
983	960	tty_ldisc_flush_works(tty);
984	961	if (o_tty)
985	962	tty_ldisc_flush_works(o_tty);
...	...	@@ -132,6 +132,7 @@
132	132	*/
133	133	void tty_port_destroy(struct tty_port *port)
134	134	{
	135	+ cancel_work_sync(&port->buf.work);
135	136	tty_buffer_free_all(port);
136	137	}
137	138	EXPORT_SYMBOL(tty_port_destroy);