[PATCH] net: ax25: fix UAF in timer paths

From: zhanjun <[email protected]>

syzbot reported a use-after-free in ax25_disconnect() from an AX.25
timer callback.

AX.25 protocol timers do not hold a reference to ax25_cb while they are
queued or running. As a result, one timer callback can tear down the
control block and drop the last reference through ax25_destroy_socket()
or ax25_link_failed() while another timer callback is still pending or
already executing on another CPU.

The deferred destroy timer has the same problem because it is embedded in
ax25_cb and can outlive the last external reference as well.

Fix this by making each armed timer hold a reference to ax25_cb. When a
timer callback starts, convert the queued-timer reference into a callback
reference and drop it when the callback finishes. Apply the same rule to
dtimer and initialize it once so it can be safely rearmed with mod_timer().

This keeps ax25_cb alive for all timer paths without adding
timer_delete_sync() to ax25_destroy_socket().

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Reported-by: [email protected]
Closes: https://syzkaller.appspot.com/bug?extid=41ebf587f04e2bcfa8e5
Signed-off-by: zhanjun <[email protected]>
---
 net/ax25/af_ax25.c    | 15 +++++++--
 net/ax25/ax25_timer.c | 75 ++++++++++++++++++++++++++++++++++++-------
 2 files changed, 75 insertions(+), 15 deletions(-)

diff --git a/net/ax25/af_ax25.c b/net/ax25/af_ax25.c
index a76f4793aed2..862d65ff3e94 100644
--- a/net/ax25/af_ax25.c
+++ b/net/ax25/af_ax25.c
@@ -290,6 +290,9 @@ static void ax25_destroy_timer(struct timer_list *t)
 	ax25_cb *ax25 = timer_container_of(ax25, t, dtimer);
 	struct sock *sk;
 
+	ax25_cb_hold(ax25);
+	ax25_cb_put(ax25);
+
 	sk=ax25->sk;
 
 	bh_lock_sock(sk);
@@ -297,6 +300,13 @@ static void ax25_destroy_timer(struct timer_list *t)
 	ax25_destroy_socket(ax25);
 	bh_unlock_sock(sk);
 	sock_put(sk);
+	ax25_cb_put(ax25);
+}
+
+static void ax25_start_destroy_timer(ax25_cb *ax25)
+{
+	if (!mod_timer(&ax25->dtimer, jiffies + 2 * HZ))
+		ax25_cb_hold(ax25);
 }
 
 /*
@@ -343,9 +353,7 @@ void ax25_destroy_socket(ax25_cb *ax25)
 	if (ax25->sk != NULL) {
 		if (sk_has_allocations(ax25->sk)) {
 			/* Defer: outstanding buffers */
-			timer_setup(&ax25->dtimer, ax25_destroy_timer, 0);
-			ax25->dtimer.expires  = jiffies + 2 * HZ;
-			add_timer(&ax25->dtimer);
+			ax25_start_destroy_timer(ax25);
 		} else {
 			struct sock *sk=ax25->sk;
 			ax25->sk=NULL;
@@ -537,6 +545,7 @@ ax25_cb *ax25_create_cb(void)
 	skb_queue_head_init(&ax25->frag_queue);
 	skb_queue_head_init(&ax25->ack_queue);
 	skb_queue_head_init(&ax25->reseq_queue);
+	timer_setup(&ax25->dtimer, ax25_destroy_timer, 0);
 
 	ax25_setup_timers(ax25);
 
diff --git a/net/ax25/ax25_timer.c b/net/ax25/ax25_timer.c
index a69bfbc8b679..0fefb5643c6c 100644
--- a/net/ax25/ax25_timer.c
+++ b/net/ax25/ax25_timer.c
@@ -36,6 +36,35 @@ static void ax25_t2timer_expiry(struct timer_list *);
 static void ax25_t3timer_expiry(struct timer_list *);
 static void ax25_idletimer_expiry(struct timer_list *);
 
+static void ax25_mod_timer(struct timer_list *timer, ax25_cb *ax25,
+			   unsigned long expires)
+{
+	if (!mod_timer(timer, expires))
+		ax25_cb_hold(ax25);
+}
+
+static void ax25_del_timer(struct timer_list *timer, ax25_cb *ax25)
+{
+	if (timer_delete(timer))
+		ax25_cb_put(ax25);
+}
+
+static void ax25_timer_expiry_start(ax25_cb *ax25)
+{
+	/*
+	 * A pending timer holds one reference to the control block. Once the
+	 * timer callback starts running that reference no longer belongs to a
+	 * queued timer, so turn it into a temporary execution reference.
+	 */
+	ax25_cb_hold(ax25);
+	ax25_cb_put(ax25);
+}
+
+static void ax25_timer_expiry_stop(ax25_cb *ax25)
+{
+	ax25_cb_put(ax25);
+}
+
 void ax25_setup_timers(ax25_cb *ax25)
 {
 	timer_setup(&ax25->timer, ax25_heartbeat_expiry, 0);
@@ -47,58 +76,60 @@ void ax25_setup_timers(ax25_cb *ax25)
 
 void ax25_start_heartbeat(ax25_cb *ax25)
 {
-	mod_timer(&ax25->timer, jiffies + 5 * HZ);
+	ax25_mod_timer(&ax25->timer, ax25, jiffies + 5 * HZ);
 }
 
 void ax25_start_t1timer(ax25_cb *ax25)
 {
-	mod_timer(&ax25->t1timer, jiffies + ax25->t1);
+	ax25_mod_timer(&ax25->t1timer, ax25, jiffies + ax25->t1);
 }
 
 void ax25_start_t2timer(ax25_cb *ax25)
 {
-	mod_timer(&ax25->t2timer, jiffies + ax25->t2);
+	ax25_mod_timer(&ax25->t2timer, ax25, jiffies + ax25->t2);
 }
 
 void ax25_start_t3timer(ax25_cb *ax25)
 {
 	if (ax25->t3 > 0)
-		mod_timer(&ax25->t3timer, jiffies + ax25->t3);
+		ax25_mod_timer(&ax25->t3timer, ax25,
+			       jiffies + ax25->t3);
 	else
-		timer_delete(&ax25->t3timer);
+		ax25_del_timer(&ax25->t3timer, ax25);
 }
 
 void ax25_start_idletimer(ax25_cb *ax25)
 {
 	if (ax25->idle > 0)
-		mod_timer(&ax25->idletimer, jiffies + ax25->idle);
+		ax25_mod_timer(&ax25->idletimer, ax25,
+			       jiffies + ax25->idle);
 	else
-		timer_delete(&ax25->idletimer);
+		ax25_del_timer(&ax25->idletimer, ax25);
 }
 
 void ax25_stop_heartbeat(ax25_cb *ax25)
 {
-	timer_delete(&ax25->timer);
+	ax25_del_timer(&ax25->timer, ax25);
 }
 
 void ax25_stop_t1timer(ax25_cb *ax25)
 {
-	timer_delete(&ax25->t1timer);
+	ax25_del_timer(&ax25->t1timer, ax25);
 }
 
 void ax25_stop_t2timer(ax25_cb *ax25)
 {
-	timer_delete(&ax25->t2timer);
+	ax25_del_timer(&ax25->t2timer, ax25);
 }
 
 void ax25_stop_t3timer(ax25_cb *ax25)
 {
-	timer_delete(&ax25->t3timer);
+	ax25_del_timer(&ax25->t3timer, ax25);
 }
 
 void ax25_stop_idletimer(ax25_cb *ax25)
 {
-	timer_delete(&ax25->idletimer);
+	ax25_del_timer(&ax25->idletimer, ax25);
 }
 
 int ax25_t1timer_running(ax25_cb *ax25)
@@ -123,6 +154,8 @@ static void ax25_heartbeat_expiry(struct timer_list *t)
 	int proto = AX25_PROTO_STD_SIMPLEX;
 	ax25_cb *ax25 = timer_container_of(ax25, t, timer);
 
+	ax25_timer_expiry_start(ax25);
+
 	if (ax25->ax25_dev)
 		proto = ax25->ax25_dev->values[AX25_VALUES_PROTOCOL];
 
@@ -141,12 +174,16 @@ static void ax25_heartbeat_expiry(struct timer_list *t)
 		break;
 #endif
 	}
+
+	ax25_timer_expiry_stop(ax25);
 }
 
 static void ax25_t1timer_expiry(struct timer_list *t)
 {
 	ax25_cb *ax25 = timer_container_of(ax25, t, t1timer);
 
+	ax25_timer_expiry_start(ax25);
+
 	switch (ax25->ax25_dev->values[AX25_VALUES_PROTOCOL]) {
 	case AX25_PROTO_STD_SIMPLEX:
 	case AX25_PROTO_STD_DUPLEX:
@@ -160,12 +197,16 @@ static void ax25_t1timer_expiry(struct timer_list *t)
 		break;
 #endif
 	}
+
+	ax25_timer_expiry_stop(ax25);
 }
 
 static void ax25_t2timer_expiry(struct timer_list *t)
 {
 	ax25_cb *ax25 = timer_container_of(ax25, t, t2timer);
 
+	ax25_timer_expiry_start(ax25);
+
 	switch (ax25->ax25_dev->values[AX25_VALUES_PROTOCOL]) {
 	case AX25_PROTO_STD_SIMPLEX:
 	case AX25_PROTO_STD_DUPLEX:
@@ -179,12 +220,16 @@ static void ax25_t2timer_expiry(struct timer_list *t)
 		break;
 #endif
 	}
+
+	ax25_timer_expiry_stop(ax25);
 }
 
 static void ax25_t3timer_expiry(struct timer_list *t)
 {
 	ax25_cb *ax25 = timer_container_of(ax25, t, t3timer);
 
+	ax25_timer_expiry_start(ax25);
+
 	switch (ax25->ax25_dev->values[AX25_VALUES_PROTOCOL]) {
 	case AX25_PROTO_STD_SIMPLEX:
 	case AX25_PROTO_STD_DUPLEX:
@@ -200,12 +245,16 @@ static void ax25_t3timer_expiry(struct timer_list *t)
 		break;
 #endif
 	}
+
+	ax25_timer_expiry_stop(ax25);
 }
 
 static void ax25_idletimer_expiry(struct timer_list *t)
 {
 	ax25_cb *ax25 = timer_container_of(ax25, t, idletimer);
 
+	ax25_timer_expiry_start(ax25);
+
 	switch (ax25->ax25_dev->values[AX25_VALUES_PROTOCOL]) {
 	case AX25_PROTO_STD_SIMPLEX:
 	case AX25_PROTO_STD_DUPLEX:
@@ -221,4 +270,6 @@ static void ax25_idletimer_expiry(struct timer_list *t)
 		break;
 #endif
 	}
+
+	ax25_timer_expiry_stop(ax25);
 }
-- 
2.50.1