net/ax25/af_ax25.c | 15 +++++++-- net/ax25/ax25_timer.c | 75 ++++++++++++++++++++++++++++++++++++------- 2 files changed, 75 insertions(+), 15 deletions(-)
From: zhanjun <zhanjun@uniontech.com>
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: syzbot+41ebf587f04e2bcfa8e5@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=41ebf587f04e2bcfa8e5
Signed-off-by: zhanjun <zhanjun@uniontech.com>
---
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© 2016 - 2026 Red Hat, Inc.