diff mbox

6pack: fix free memory scribbles

Message ID 20160105115042.8134.4926.stgit@localhost.localdomain
State Accepted, archived
Delegated to: David Miller
Headers show

Commit Message

Alan Cox Jan. 5, 2016, 11:51 a.m. UTC
commit acf673a3187edf72068ee2f92f4dc47d66baed47 fixed a user triggerable free
memory scribble but in doing so replaced it with a different one that allows
the user to control the data and scribble even more.

sixpack_close is called by the tty layer in tty context. The tty context is
protected by sp_get() and sp_put(). However network layer activity via
sp_xmit() is not protected this way. We must therefore stop the queue
otherwise the user gets to dump a buffer mostly of their choice into freed
kernel pages.

Signed-off-by: Alan Cox <alan@linux.intel.com>
---
 drivers/net/hamradio/6pack.c |    6 ++++++
 1 file changed, 6 insertions(+)


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

David Miller Jan. 6, 2016, 6:25 a.m. UTC | #1
From: Alan <gnomes@lxorguk.ukuu.org.uk>
Date: Tue, 05 Jan 2016 11:51:25 +0000

> commit acf673a3187edf72068ee2f92f4dc47d66baed47 fixed a user triggerable free
> memory scribble but in doing so replaced it with a different one that allows
> the user to control the data and scribble even more.
> 
> sixpack_close is called by the tty layer in tty context. The tty context is
> protected by sp_get() and sp_put(). However network layer activity via
> sp_xmit() is not protected this way. We must therefore stop the queue
> otherwise the user gets to dump a buffer mostly of their choice into freed
> kernel pages.
> 
> Signed-off-by: Alan Cox <alan@linux.intel.com>

Applied, thanks Alan.

> +	/* We must stop the queue to avoid potentially scribbling
> +	   on the free buffers. The sp->dead_sem is not sufficient
> +	   to protect us from sp->xbuff access */

Please next time format comments:

	/* Like
	 * this.
	 */

Thanks again!
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/net/hamradio/6pack.c b/drivers/net/hamradio/6pack.c
index 9f0b1c3..b3ba6a8 100644
--- a/drivers/net/hamradio/6pack.c
+++ b/drivers/net/hamradio/6pack.c
@@ -683,6 +683,12 @@  static void sixpack_close(struct tty_struct *tty)
 	if (!atomic_dec_and_test(&sp->refcnt))
 		down(&sp->dead_sem);
 
+	/* We must stop the queue to avoid potentially scribbling
+	   on the free buffers. The sp->dead_sem is not sufficient
+	   to protect us from sp->xbuff access */
+
+	netif_stop_queue(sp->dev);
+
 	del_timer_sync(&sp->tx_t);
 	del_timer_sync(&sp->resync_t);