[1/2] ath9k: fix tx99 use after free

Commit Message

Miaoqing Pan June 20, 2017, 1:13 a.m. UTC

From: Miaoqing Pan <[e-mail]>

One scenario that could lead to UAF is two threads writing
simultaneously to the "tx99" debug file. One of them would
set the "start" value to true and follow to ath9k_tx99_init().
Inside the function it would set the sc->tx99_state to true
after allocating sc->tx99skb. Then, the other thread would
execute write_file_tx99() and call ath9k_tx99_deinit().
sc->tx99_state would be freed. After that, the first thread
would continue inside ath9k_tx99_init() and call
r = ath9k_tx99_send(sc, sc->tx99_skb, &txctl);
that would make use of the freed sc->tx99_skb memory.

Signed-off-by: Miaoqing Pan <[e-mail]>
---
 drivers/net/wireless/ath/ath9k/tx99.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

Comments

Kalle Valo June 21, 2017, 1:52 p.m. UTC | #1

miaoqing pan <[e-mail]> wrote:

> One scenario that could lead to UAF is two threads writing
> simultaneously to the "tx99" debug file. One of them would
> set the "start" value to true and follow to ath9k_tx99_init().
> Inside the function it would set the sc->tx99_state to true
> after allocating sc->tx99skb. Then, the other thread would
> execute write_file_tx99() and call ath9k_tx99_deinit().
> sc->tx99_state would be freed. After that, the first thread
> would continue inside ath9k_tx99_init() and call
> r = ath9k_tx99_send(sc, sc->tx99_skb, &txctl);
> that would make use of the freed sc->tx99_skb memory.
> 
> Signed-off-by: Miaoqing Pan <[e-mail]>
> Signed-off-by: Kalle Valo <[e-mail]>

I added Cc stable to both patches.

Kalle Valo June 28, 2017, 4:53 p.m. UTC | #2

miaoqing pan <[e-mail]> wrote:

> One scenario that could lead to UAF is two threads writing
> simultaneously to the "tx99" debug file. One of them would
> set the "start" value to true and follow to ath9k_tx99_init().
> Inside the function it would set the sc->tx99_state to true
> after allocating sc->tx99skb. Then, the other thread would
> execute write_file_tx99() and call ath9k_tx99_deinit().
> sc->tx99_state would be freed. After that, the first thread
> would continue inside ath9k_tx99_init() and call
> r = ath9k_tx99_send(sc, sc->tx99_skb, &txctl);
> that would make use of the freed sc->tx99_skb memory.
> 
> Cc: <[e-mail]>
> Signed-off-by: Miaoqing Pan <[e-mail]>
> Signed-off-by: Kalle Valo <[e-mail]>

2 patches applied to ath-next branch of ath.git, thanks.

cf8ce1ea61b7 ath9k: fix tx99 use after free
bde717ab4736 ath9k: fix tx99 bus error

Patch

diff --git a/drivers/net/wireless/ath/ath9k/tx99.c b/drivers/net/wireless/ath/ath9k/tx99.c
index a866cbd..49ed1af 100644
--- a/drivers/net/wireless/ath/ath9k/tx99.c
+++ b/drivers/net/wireless/ath/ath9k/tx99.c
@@ -189,22 +189,27 @@  static ssize_t write_file_tx99(struct file *file, const char __user *user_buf,
 	if (strtobool(buf, &start))
 		return -EINVAL;
 
+	mutex_lock(&sc->mutex);
+
 	if (start == sc->tx99_state) {
 		if (!start)
-			return count;
+			goto out;
 		ath_dbg(common, XMIT, "Resetting TX99\n");
 		ath9k_tx99_deinit(sc);
 	}
 
 	if (!start) {
 		ath9k_tx99_deinit(sc);
-		return count;
+		goto out;
 	}
 
 	r = ath9k_tx99_init(sc);
-	if (r)
+	if (r) {
+		mutex_unlock(&sc->mutex);
 		return r;
-
+	}
+out:
+	mutex_unlock(&sc->mutex);
 	return count;
 }