[PATCH net-next v3 0/2] hdlc_cisco/hdlc_fr: shut down protocol timers on ->detach

[PATCH net-next v3 0/2] hdlc_cisco/hdlc_fr: shut down protocol timers on ->detach

[Chengyu Liu]

Both cisco and fr arm a self-re-arming timer in ->start() but never shut
it down on the protocol-detach path: detach_hdlc_protocol() calls only
->detach() (never ->stop()) and then kfree()s hdlc->state, so
unregistering an up device leaves the timer armed on freed memory.  This
is the same defect class as CVE-2026-63803 ("hdlc_ppp: sync per-proto
timers before freeing hdlc state").

Each patch shuts the respective timer down with timer_shutdown_sync() and
moves timer_setup() to the attach ioctl (attach_hdlc_protocol() uses
kmalloc(), so the timer must be initialized exactly once at attach time).
Both bugs were found by static analysis and confirmed with a KASAN
reproducer (slab-use-after-free in the timer softirq after rmmod of an up
device).

---
v3: process-only repost, no code change; fix mail setup (previous relay
    rewrote Message-IDs, breaking threading and preventing the series
    from being assembled by the CI bots); add lore links below.
v2: https://lore.kernel.org/netdev/tencent_BA6DA4F17D2035F5DEEEA21B9578E2234307@qq.com/
  - retarget to net-next and drop Cc: stable per maintainer guidance;
    carry Simon's Reviewed-by; no code change.
v1: https://lore.kernel.org/netdev/tencent_83B7D10D4DBCC6118AB6F6B83A509FFA5C06@qq.com/

Chengyu Liu (2):
  hdlc_cisco: shut down the keepalive timer on ->detach
  hdlc_fr: shut down the LMI timer in fr_destroy()

 drivers/net/wan/hdlc_cisco.c | 10 +++++++++-
 drivers/net/wan/hdlc_fr.c    |  5 ++++-
 2 files changed, 13 insertions(+), 2 deletions(-)

-- 
2.25.1

[PATCH net-next v3 1/2] hdlc_cisco: shut down the keepalive timer on ->detach

[Chengyu Liu]

cisco_start() arms st->timer unconditionally and cisco_timer() re-arms
itself on every fire.  The only deletion lives in cisco_stop(), which is
unreachable on the protocol-detach path: detach_hdlc_protocol() calls
only ->detach() (never ->stop()) and then kfree()s hdlc->state.  The
cisco hdlc_proto has no ->detach at all, so unregistering an up cisco
device leaves the keepalive timer armed on top of freed memory; it fires
within one keepalive interval and cisco_timer() writes through the dead
cisco_state (st->lock, st->txseq++, cisco_keepalive_send()).

Add a ->detach callback that shuts the timer down permanently before the
state is freed, mirroring the CVE-2026-63803 ppp fix
(timer_shutdown_sync() in ppp_timer_release()).

Initialize the timer in cisco_ioctl() when the protocol is attached and
drop the timer_setup() from cisco_start(), so the timer is initialized
exactly once at attach time: attach_hdlc_protocol() uses kmalloc() (not
kzalloc), so st->timer is uninitialized garbage until the first
timer_setup() and ->detach() must never run timer_shutdown_sync() on it.

This bug was found by static analysis and confirmed with a KASAN
reproducer: slab-use-after-free write in __run_timer_base() within one
keepalive interval after rmmod of an up cisco HDLC device (allocated by
attach_hdlc_protocol() via cisco_ioctl(), freed by
detach_hdlc_protocol() via unregister_hdlc_device()).

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Chengyu Liu <lui2012qwq@gmail.com>
Reviewed-by: Simon Horman <horms@kernel.org>
---
v2: retarget to net-next and drop Cc: stable per maintainer guidance;
    no code change.  v1: https://lore.kernel.org/netdev/
 drivers/net/wan/hdlc_cisco.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wan/hdlc_cisco.c b/drivers/net/wan/hdlc_cisco.c
index bfc978b15b..2173d5c6d6 100644
--- a/drivers/net/wan/hdlc_cisco.c
+++ b/drivers/net/wan/hdlc_cisco.c
@@ -274,7 +274,6 @@ static void cisco_start(struct net_device *dev)
 	spin_unlock_irqrestore(&st->lock, flags);
 
 	st->dev = dev;
-	timer_setup(&st->timer, cisco_timer, 0);
 	st->timer.expires = jiffies + HZ; /* First poll after 1 s */
 	add_timer(&st->timer);
 }
@@ -293,9 +292,17 @@ static void cisco_stop(struct net_device *dev)
 	spin_unlock_irqrestore(&st->lock, flags);
 }
 
+static void cisco_detach(struct net_device *dev)
+{
+	struct cisco_state *st = state(dev_to_hdlc(dev));
+
+	timer_shutdown_sync(&st->timer);
+}
+
 static struct hdlc_proto proto = {
 	.start		= cisco_start,
 	.stop		= cisco_stop,
+	.detach		= cisco_detach,
 	.type_trans	= cisco_type_trans,
 	.ioctl		= cisco_ioctl,
 	.netif_rx	= cisco_rx,
@@ -353,6 +360,7 @@ static int cisco_ioctl(struct net_device *dev, struct if_settings *ifs)
 
 		memcpy(&state(hdlc)->settings, &new_settings, size);
 		spin_lock_init(&state(hdlc)->lock);
+		timer_setup(&state(hdlc)->timer, cisco_timer, 0);
 		dev->header_ops = &cisco_header_ops;
 		dev->hard_header_len = sizeof(struct hdlc_header);
 		dev->type = ARPHRD_CISCO;
-- 
2.25.1

[PATCH net-next v3 2/2] hdlc_fr: shut down the LMI timer in fr_destroy()

[Chengyu Liu]

fr_start() arms state(hdlc)->timer when LMI is enabled
(settings.lmi != LMI_NONE).  The only deletion lives in fr_stop(), which
is unreachable on the protocol-detach path: detach_hdlc_protocol() calls
only ->detach() (never ->stop()) and then kfree()s hdlc->state.  FR's
->detach, fr_destroy(), tears down the PVC list but never touches the
timer, so unregistering an up FR device with LMI enabled leaves the timer
armed on top of freed memory; fr_timer() then fires into the dead
frad_state.

Shut the timer down at the top of fr_destroy().  Use
timer_shutdown_sync() so the timer cannot be re-armed afterwards,
matching the CVE-2026-63803 ppp fix (ppp_timer_release()).

Initialize the timer in fr_ioctl() when the protocol is attached and
drop the timer_setup() from fr_start(): attach_hdlc_protocol() uses
kmalloc() (not kzalloc), so state(hdlc)->timer is uninitialized garbage
until the first timer_setup() and fr_destroy() must never run
timer_shutdown_sync() on it.

This bug was found by static analysis and confirmed with a KASAN
reproducer: slab-use-after-free in the timer softirq after rmmod of an
up Frame Relay HDLC device with LMI enabled.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Chengyu Liu <lui2012qwq@gmail.com>
Reviewed-by: Simon Horman <horms@kernel.org>
---
v2: retarget to net-next and drop Cc: stable per maintainer guidance;
    no code change.  v1: https://lore.kernel.org/netdev/
 drivers/net/wan/hdlc_fr.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wan/hdlc_fr.c b/drivers/net/wan/hdlc_fr.c
index 5f0e54ed1e..2086ff85b5 100644
--- a/drivers/net/wan/hdlc_fr.c
+++ b/drivers/net/wan/hdlc_fr.c
@@ -1009,7 +1009,6 @@ static void fr_start(struct net_device *dev)
 		state(hdlc)->txseq = state(hdlc)->rxseq = 0;
 
 		state(hdlc)->dev = dev;
-		timer_setup(&state(hdlc)->timer, fr_timer, 0);
 		/* First poll after 1 s */
 		state(hdlc)->timer.expires = jiffies + HZ;
 		add_timer(&state(hdlc)->timer);
@@ -1153,6 +1152,9 @@ static void fr_destroy(struct net_device *frad)
 	hdlc_device *hdlc = dev_to_hdlc(frad);
 	struct pvc_device *pvc = state(hdlc)->first_pvc;
 
+	/* The LMI timer must not outlive the state being torn down. */
+	timer_shutdown_sync(&state(hdlc)->timer);
+
 	state(hdlc)->first_pvc = NULL; /* All PVCs destroyed */
 	state(hdlc)->dce_pvc_count = 0;
 	state(hdlc)->dce_changed = 1;
@@ -1242,6 +1244,7 @@ static int fr_ioctl(struct net_device *dev, struct if_settings *ifs)
 				return result;
 			state(hdlc)->first_pvc = NULL;
 			state(hdlc)->dce_pvc_count = 0;
+			timer_setup(&state(hdlc)->timer, fr_timer, 0);
 		}
 		memcpy(&state(hdlc)->settings, &new_settings, size);
 		dev->type = ARPHRD_FRAD;
-- 
2.25.1