Message ID | 20160105115042.8134.4926.stgit@localhost.localdomain |
---|---|
State | Accepted, archived |
Delegated to: | David Miller |
Headers | show |
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 --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);
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