[PATCH net v4 0/2] s390/ctcm: Fix timer corruption and use-after-free

[PATCH net v4 0/2] s390/ctcm: Fix timer corruption and use-after-free

[Nagamani PV]

Fix two bugs in drivers/s390/net/fsm.c and drivers/s390/net/ctcm_main.c
found by Sashiko AI code review.

Patch 1 fixes timer list corruption when fsm_addtimer() is called on
an already-pending timer - timer_setup() re-initializes the timer
list_head while it is still enqueued in the wheel.

Patch 2 fixes a use-after-free in channel_remove(). For MPC channels
the teardown has a circular dependency: the ch->timer callback can
schedule ch_disc_tasklet via mpc_action_go_inop(), ch_tasklet can
re-arm sweep_timer, and sweep_timer's callback re-arms ch->timer.
Fix by shutting down both MPC timers with timer_shutdown_sync() first
(sweep_timer before ch->timer), then killing both tasklets. This breaks
all three re-arm paths before freeing channel resources.

Changes in v4:
- Patch 2: shut down timers before tasklet_kill() rather than after,
  shutting down sweep_timer before ch->timer.
- Patch 2: use timer_shutdown_sync() for ch->timer on both MPC and
  non-MPC paths.
- Patch 2: name prerequisite by commit title and add structured
  stable tag dependency (Jakub Kicinski).
- Patch 2: code changed; Reviewed-by and Tested-by dropped.

Changes in v3:
- Patch 2: fix MPC tasklet/timer re-arm UAF identified by Sashiko:
  kill ch_tasklet and ch_disc_tasklet before stopping the timers, then
  shut down sweep_timer before deleting ch->timer; move
  kfree(discontact_th) into the MPC teardown block.
- Patch 2: code changed; Reviewed-by and Tested-by dropped.

Note: two pre-existing UAFs in ctcm_free_netdevice() (grp->timer,
priv->restart_timer) and a NULL deref in ctcmpc_chx_txdone() are
confirmed but out of scope for this series; follow-up patch planned.

Changes in v2:
- Patch 1: fix function name ctcm_send_sweep() -> ctcmpc_send_sweep_req()
  in the commit message (Sashiko netdev-bot)
- Patch 1: call mod_timer() then return 0 explicitly, preserving the
  "Always returns 0" contract documented in fsm.h (Sashiko netdev-bot)
- Patch 2: add Fixes: and Cc: stable@vger.kernel.org tags
  (Sashiko netdev-bot)

Nagamani PV (2):
  s390/ctcm: Fix timer corruption in fsm_addtimer()
  s390/ctcm: Fix use-after-free in channel_remove()

 drivers/s390/net/ctcm_main.c | 14 +++++++-------
 drivers/s390/net/fsm.c       |  9 ++-------
 2 files changed, 9 insertions(+), 14 deletions(-)

-- 
2.53.0

[PATCH net v4 1/2] s390/ctcm: Fix timer corruption in fsm_addtimer()

[Nagamani PV]

fsm_addtimer() calls timer_setup() unconditionally before add_timer().
If called on an already-pending timer, timer_setup() re-initializes
the timer's list_head fields while the timer is still enqueued in the
wheel, corrupting the timer list.

The timer is already initialized once by fsm_settimer() which calls
timer_setup() correctly. Multiple callsites invoke fsm_addtimer()
without a preceding fsm_deltimer(), including ctcm_main.c
ctcmpc_send_sweep_req() and ctcm_mpc.c mpc_action_side_xid(), making the
redundant timer_setup() in fsm_addtimer() a real corruption risk.

Remove the redundant timer_setup() calls from fsm_addtimer() and
fsm_modtimer(), and replace add_timer() with mod_timer() which safely
handles both pending and non-pending timers atomically without
corrupting the timer wheel.

Fixes: e99e88a9d2b0 ("treewide: setup_timer() -> timer_setup()")
Cc: stable@vger.kernel.org
Reported-by: Sashiko <sashiko-bot@kernel.org>
Link: https://sashiko.dev/#/patchset/20260803182736.2356374-1-nagamani@linux.ibm.com?part=1
Reviewed-by: Aswin Karuvally <aswin@linux.ibm.com>
Tested-by: Aswin Karuvally <aswin@linux.ibm.com>
Signed-off-by: Nagamani PV <nagamani@linux.ibm.com>
---
Changes in v4:
- No changes to this patch.

Changes in v3:
- No changes to this patch.

Changes in v2:
- Fix function name ctcm_send_sweep() -> ctcmpc_send_sweep_req()
  in the commit message.
- Call mod_timer() then return 0 explicitly, preserving the
  "Always returns 0" contract documented in fsm.h.
---
 drivers/s390/net/fsm.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/s390/net/fsm.c b/drivers/s390/net/fsm.c
index 9b8622aef807..936ed0bc8401 100644
--- a/drivers/s390/net/fsm.c
+++ b/drivers/s390/net/fsm.c
@@ -171,11 +171,9 @@ fsm_addtimer(fsm_timer *this, int millisec, int event, void *arg)
 	       this->fi->name, this, millisec);
 #endif
 
-	timer_setup(&this->tl, fsm_expire_timer, 0);
 	this->expire_event = event;
 	this->event_arg = arg;
-	this->tl.expires = jiffies + (millisec * HZ) / 1000;
-	add_timer(&this->tl);
+	mod_timer(&this->tl, jiffies + msecs_to_jiffies(millisec));
 	return 0;
 }
 
@@ -189,12 +187,9 @@ fsm_modtimer(fsm_timer *this, int millisec, int event, void *arg)
 		this->fi->name, this, millisec);
 #endif
 
-	timer_delete(&this->tl);
-	timer_setup(&this->tl, fsm_expire_timer, 0);
 	this->expire_event = event;
 	this->event_arg = arg;
-	this->tl.expires = jiffies + (millisec * HZ) / 1000;
-	add_timer(&this->tl);
+	mod_timer(&this->tl, jiffies + msecs_to_jiffies(millisec));
 }
 
 EXPORT_SYMBOL(init_fsm);
-- 
2.53.0

[PATCH net v4 2/2] s390/ctcm: Fix use-after-free in channel_remove()

[Nagamani PV]

channel_remove() uses fsm_deltimer() which calls timer_delete(),
returning immediately even if the timer callback is running on
another CPU. This leaves a window where fsm_expire_timer() accesses
ch->fsm after kfree_fsm().

For MPC channels the teardown has a circular dependency:

  ch->timer callback -> ctcm_chx_txretry() -> mpc_action_go_inop()
    -> tasklet_hi_schedule(&ch->ch_disc_tasklet)

  ch_tasklet -> ctcmpc_send_sweep_resp() -> fsm_addtimer(&ch->sweep_timer)

  sweep_timer callback -> fsm_addtimer(&ch->timer)

Use timer_shutdown_sync() for both timers: it waits for any running
callback and permanently prevents rearming. Shut down sweep_timer
first (its callback rearms ch->timer at ctcm_fsms.c), then
ch->timer. Only then call tasklet_kill() -- catching any tasklet
scheduled by an in-flight callback before shutdown.
kfree(ch->discontact_th) follows tasklet_kill(ch_disc_tasklet) since
ch->ccw[15].cda points to discontact_th and
mpc_action_send_discontact() starts I/O through ch->ccw[15].

timer_shutdown_sync() is also used for non-MPC ch->timer:
ctcm_chx_txretry() (CTC_STATE_TX + CTC_EVENT_TIMER) calls
fsm_addtimer(&ch->timer) on both paths.

Depends on "s390/ctcm: Fix timer corruption in fsm_addtimer()" which
replaces add_timer() with mod_timer(). mod_timer() on a shut-down
timer is discarded; without it fsm_addtimer() calls timer_setup()
defeating timer_shutdown_sync(). Backporting without that
prerequisite is unsafe.

Fixes: 293d984f0e36 ("ctcm: infrastructure for replaced ctc driver")
Cc: stable@vger.kernel.org # depends on: s390/ctcm: Fix timer corruption in fsm_addtimer()
Reported-by: Sashiko <sashiko-bot@kernel.org>
Link: https://sashiko.dev/#/patchset/20260922101913.239103-1-nagamani@linux.ibm.com?part=2
Signed-off-by: Nagamani PV <nagamani@linux.ibm.com>
---
Changes in v4:
- Shut down timers before tasklet_kill() rather than after,
  shutting down sweep_timer before ch->timer.
- Use timer_shutdown_sync() for ch->timer on both MPC and non-MPC paths.
- Name prerequisite by commit title and add structured stable tag
  dependency (Jakub Kicinski).
- Code changed; Reviewed-by and Tested-by dropped.

Changes in v3:
- Kill ch_tasklet and ch_disc_tasklet before stopping the timers and
  freeing ch->fsm: closes freed ch->fsm access from tasklet context and
  eliminates tasklet-driven sweep_timer re-arm.
- Shut down sweep_timer with timer_shutdown_sync() before deleting
  ch->timer to prevent sweep_timer callback from re-arming ch->timer.
- Move kfree(ch->discontact_th) after tasklet_kill(ch_disc_tasklet).
- Drop Reviewed-by and Tested-by; please re-review and re-test the
  new teardown order.

Changes in v2:
- Add Fixes: and Cc: stable@vger.kernel.org tags.
---
 drivers/s390/net/ctcm_main.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/s390/net/ctcm_main.c b/drivers/s390/net/ctcm_main.c
index 8b0d76a47d9f..27a22dbc9280 100644
--- a/drivers/s390/net/ctcm_main.c
+++ b/drivers/s390/net/ctcm_main.c
@@ -211,9 +211,14 @@ static void channel_remove(struct channel *ch)
 	while (*c) {
 		if (*c == ch) {
 			*c = ch->next;
-			fsm_deltimer(&ch->timer);
 			if (IS_MPC(ch))
-				fsm_deltimer(&ch->sweep_timer);
+				timer_shutdown_sync(&ch->sweep_timer.tl);
+			timer_shutdown_sync(&ch->timer.tl);
+			if (IS_MPC(ch)) {
+				tasklet_kill(&ch->ch_tasklet);
+				tasklet_kill(&ch->ch_disc_tasklet);
+				kfree(ch->discontact_th);
+			}
 
 			kfree_fsm(ch->fsm);
 			clear_normalized_cda(&ch->ccw[4]);
@@ -221,11 +226,6 @@ static void channel_remove(struct channel *ch)
 				clear_normalized_cda(&ch->ccw[1]);
 				dev_kfree_skb_any(ch->trans_skb);
 			}
-			if (IS_MPC(ch)) {
-				tasklet_kill(&ch->ch_tasklet);
-				tasklet_kill(&ch->ch_disc_tasklet);
-				kfree(ch->discontact_th);
-			}
 			kfree(ch->ccw);
 			kfree(ch->irb);
 			kfree(ch);
-- 
2.53.0

Re: [PATCH net v4 0/2] s390/ctcm: Fix timer corruption and use-after-free

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by code inspection). If it was triggered please include
   the symptoms, like the stack trace or error messages.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

Re: [PATCH net v4 1/2] s390/ctcm: Fix timer corruption in fsm_addtimer()

[sashiko-bot]

> fsm_addtimer() calls timer_setup() unconditionally before add_timer().
> If called on an already-pending timer, timer_setup() re-initializes
> the timer's list_head fields while the timer is still enqueued in the
> wheel, corrupting the timer list.
> 
> The timer is already initialized once by fsm_settimer() which calls
> timer_setup() correctly. Multiple callsites invoke fsm_addtimer()
> without a preceding fsm_deltimer(), including ctcm_main.c
> ctcmpc_send_sweep_req() and ctcm_mpc.c mpc_action_side_xid(), making the
> redundant timer_setup() in fsm_addtimer() a real corruption risk.
> 
> Remove the redundant timer_setup() calls from fsm_addtimer() and
> fsm_modtimer(), and replace add_timer() with mod_timer() which safely
> handles both pending and non-pending timers atomically without
> corrupting the timer wheel.
> 
> Fixes: e99e88a9d2b0 ("treewide: setup_timer() -> timer_setup()")
> Cc: stable@vger.kernel.org
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Link: https://sashiko.dev/#/patchset/20260803182736.2356374-1-nagamani@linux.ibm.com?part=1
> Reviewed-by: Aswin Karuvally <aswin@linux.ibm.com>
> Tested-by: Aswin Karuvally <aswin@linux.ibm.com>
> Signed-off-by: Nagamani PV <nagamani@linux.ibm.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260930070234.569500-1-nagamani@linux.ibm.com?part=1

Re: [PATCH net v4 2/2] s390/ctcm: Fix use-after-free in channel_remove()

[sashiko-bot]

> channel_remove() uses fsm_deltimer() which calls timer_delete(),
> returning immediately even if the timer callback is running on
> another CPU. This leaves a window where fsm_expire_timer() accesses
> ch->fsm after kfree_fsm().
> 
> For MPC channels the teardown has a circular dependency:
> 
>   ch->timer callback -> ctcm_chx_txretry() -> mpc_action_go_inop()
>     -> tasklet_hi_schedule(&ch->ch_disc_tasklet)
> 
>   ch_tasklet -> ctcmpc_send_sweep_resp() -> fsm_addtimer(&ch->sweep_timer)
> 
>   sweep_timer callback -> fsm_addtimer(&ch->timer)
> 
> Use timer_shutdown_sync() for both timers: it waits for any running
> [ ... ]
> Fixes: 293d984f0e36 ("ctcm: infrastructure for replaced ctc driver")
> Cc: stable@vger.kernel.org # depends on: s390/ctcm: Fix timer corruption in fsm_addtimer()
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Link: https://sashiko.dev/#/patchset/20260922101913.239103-1-nagamani@linux.ibm.com?part=2
> Signed-off-by: Nagamani PV <nagamani@linux.ibm.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260930070234.569500-1-nagamani@linux.ibm.com?part=2

Re: [PATCH net v4 0/2] s390/ctcm: Fix timer corruption and use-after-free

[Nagamani PV]

On 30/09/26 12:39 PM, netdev-bot+sinfo@kernel.org wrote:
> Hi!
> 
> This is an automated message. This series looks like a fix, but its
> commit messages seem to be missing some information:
> 
>  - Whether the issue was actually triggered, or is only theoretical
>    (e.g. found by code inspection). If it was triggered please include
>    the symptoms, like the stack trace or error messages.
> 
> Please do not repost the series just to address the above. Instead,
> reply to this email with the missing information, so that reviewers
> can take it into account. If the series needs another revision for
> other reasons, please include the information in the commit messages
> then.
> 
> The evaluation is done by an LLM so it may be wrong, if you think
> that is the case please reply and explain.

Both issues were identified through static code inspection / automated
analysis (Sashiko code review) and confirmed by manual code review.
They have not been triggered in production with an observed stack trace.

Nagamani

Re: [PATCH net v4 2/2] s390/ctcm: Fix use-after-free in channel_remove()

[Aswin Karuvally]

On 30/09/26 12:32, Nagamani PV wrote:
> channel_remove() uses fsm_deltimer() which calls timer_delete(),
> returning immediately even if the timer callback is running on
> another CPU. This leaves a window where fsm_expire_timer() accesses
> ch->fsm after kfree_fsm().
> 
> For MPC channels the teardown has a circular dependency:
> 
>   ch->timer callback -> ctcm_chx_txretry() -> mpc_action_go_inop()
>     -> tasklet_hi_schedule(&ch->ch_disc_tasklet)
> 
>   ch_tasklet -> ctcmpc_send_sweep_resp() -> fsm_addtimer(&ch->sweep_timer)
> 
>   sweep_timer callback -> fsm_addtimer(&ch->timer)
> 
> Use timer_shutdown_sync() for both timers: it waits for any running
> callback and permanently prevents rearming. Shut down sweep_timer
> first (its callback rearms ch->timer at ctcm_fsms.c), then
> ch->timer. Only then call tasklet_kill() -- catching any tasklet
> scheduled by an in-flight callback before shutdown.
> kfree(ch->discontact_th) follows tasklet_kill(ch_disc_tasklet) since
> ch->ccw[15].cda points to discontact_th and
> mpc_action_send_discontact() starts I/O through ch->ccw[15].
> 
> timer_shutdown_sync() is also used for non-MPC ch->timer:
> ctcm_chx_txretry() (CTC_STATE_TX + CTC_EVENT_TIMER) calls
> fsm_addtimer(&ch->timer) on both paths.
> 
> Depends on "s390/ctcm: Fix timer corruption in fsm_addtimer()" which
> replaces add_timer() with mod_timer(). mod_timer() on a shut-down
> timer is discarded; without it fsm_addtimer() calls timer_setup()
> defeating timer_shutdown_sync(). Backporting without that
> prerequisite is unsafe.
> 
> Fixes: 293d984f0e36 ("ctcm: infrastructure for replaced ctc driver")
> Cc: stable@vger.kernel.org # depends on: s390/ctcm: Fix timer corruption in fsm_addtimer()
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Link: https://sashiko.dev/#/patchset/20260922101913.239103-1-nagamani@linux.ibm.com?part=2
> Signed-off-by: Nagamani PV <nagamani@linux.ibm.com>
> ---
> Changes in v4:
> - Shut down timers before tasklet_kill() rather than after,
>   shutting down sweep_timer before ch->timer.
> - Use timer_shutdown_sync() for ch->timer on both MPC and non-MPC paths.
> - Name prerequisite by commit title and add structured stable tag
>   dependency (Jakub Kicinski).
> - Code changed; Reviewed-by and Tested-by dropped.
> 
> Changes in v3:
> - Kill ch_tasklet and ch_disc_tasklet before stopping the timers and
>   freeing ch->fsm: closes freed ch->fsm access from tasklet context and
>   eliminates tasklet-driven sweep_timer re-arm.
> - Shut down sweep_timer with timer_shutdown_sync() before deleting
>   ch->timer to prevent sweep_timer callback from re-arming ch->timer.
> - Move kfree(ch->discontact_th) after tasklet_kill(ch_disc_tasklet).
> - Drop Reviewed-by and Tested-by; please re-review and re-test the
>   new teardown order.
> 
> Changes in v2:
> - Add Fixes: and Cc: stable@vger.kernel.org tags.
> ---
>  drivers/s390/net/ctcm_main.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/s390/net/ctcm_main.c b/drivers/s390/net/ctcm_main.c
> index 8b0d76a47d9f..27a22dbc9280 100644
> --- a/drivers/s390/net/ctcm_main.c
> +++ b/drivers/s390/net/ctcm_main.c
> @@ -211,9 +211,14 @@ static void channel_remove(struct channel *ch)
>  	while (*c) {
>  		if (*c == ch) {
>  			*c = ch->next;
> -			fsm_deltimer(&ch->timer);
>  			if (IS_MPC(ch))
> -				fsm_deltimer(&ch->sweep_timer);
> +				timer_shutdown_sync(&ch->sweep_timer.tl);
> +			timer_shutdown_sync(&ch->timer.tl);
> +			if (IS_MPC(ch)) {
> +				tasklet_kill(&ch->ch_tasklet);
> +				tasklet_kill(&ch->ch_disc_tasklet);
> +				kfree(ch->discontact_th);
> +			}
>  
>  			kfree_fsm(ch->fsm);
>  			clear_normalized_cda(&ch->ccw[4]);
> @@ -221,11 +226,6 @@ static void channel_remove(struct channel *ch)
>  				clear_normalized_cda(&ch->ccw[1]);
>  				dev_kfree_skb_any(ch->trans_skb);
>  			}
> -			if (IS_MPC(ch)) {
> -				tasklet_kill(&ch->ch_tasklet);
> -				tasklet_kill(&ch->ch_disc_tasklet);
> -				kfree(ch->discontact_th);
> -			}
>  			kfree(ch->ccw);
>  			kfree(ch->irb);
>  			kfree(ch);

Reviewed-by: Aswin Karuvally <aswin@linux.ibm.com>

Re: [PATCH net v4 2/2] s390/ctcm: Fix use-after-free in channel_remove()

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 1 (1) · Medium: 0 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] The patch waits for channel timers and tasklets too late.

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but it is the same
  cancel-without-sync-then-free pattern in sibling fsm_timers.

--

s390/ctcm: Fix use-after-free in channel_remove()

In channel_remove(), this patch replaces fsm_deltimer() with
timer_shutdown_sync() for ch->timer and ch->sweep_timer. It also moves
the MPC tasklet_kill() calls and kfree(ch->discontact_th) so they run
after the timers are shut down and before kfree_fsm(ch->fsm).

Link: https://sashiko.dev/#/patchset/20260922101913.239103-1-nagamani@linux.ibm.com?part=2

> diff --git a/drivers/s390/net/ctcm_main.c b/drivers/s390/net/ctcm_main.c
> index 8b0d76a47d9f0..27a22dbc92806 100644
> --- a/drivers/s390/net/ctcm_main.c
> +++ b/drivers/s390/net/ctcm_main.c
> @@ -211,9 +211,14 @@ static void channel_remove(struct channel *ch)
>  	while (*c) {
>  		if (*c == ch) {
>  			*c = ch->next;
> -			fsm_deltimer(&ch->timer);
>  			if (IS_MPC(ch))
> -				fsm_deltimer(&ch->sweep_timer);
> +				timer_shutdown_sync(&ch->sweep_timer.tl);
> +			timer_shutdown_sync(&ch->timer.tl);
> +			if (IS_MPC(ch)) {
> +				tasklet_kill(&ch->ch_tasklet);
> +				tasklet_kill(&ch->ch_disc_tasklet);
> +				kfree(ch->discontact_th);
> +			}

[Severity: High]
Do these waits run too late to protect the state that the channel
callbacks use?

The only teardown caller of channel_remove() is ctcm_shutdown_device(),
and it calls it after the net_device and its private state are gone:

drivers/s390/net/ctcm_main.c:ctcm_shutdown_device() {
    ...
	if (dev) {
		unregister_netdev(dev);
		ctcm_free_netdevice(dev);
	}
    ...
	ccw_device_set_offline(cgdev->cdev[1]);
	ccw_device_set_offline(cgdev->cdev[0]);
	channel_remove(priv->channel[CTCM_READ]);
	channel_remove(priv->channel[CTCM_WRITE]);
    ...
}

ctcm_free_netdevice() does kfree_fsm(grp->fsm), kfree(grp),
priv->mpcg = NULL, kfree_fsm(priv->fsm) and priv->fsm = NULL. Under MODULE
it also calls free_netdev(dev). The ccw_device_set_offline() calls in
between can sleep, so a channel timer or tasklet can fire in that window.

The channel callbacks reach this state through ch->netdev->ml_priv:

  ctcm_chx_txretry() (ch->timer):
	fsm_event(priv->fsm, DEV_EVENT_TXDOWN, dev);

  ctcmpc_chx_send_sweep() (sweep_timer):
	if (grp->in_sweep == 0)

  ctcmpc_bh() (ch_tasklet):
	while ((fsm_getstate(grp->fsm) != MPCG_STATE_FLOWC) &&

  ctcm_ccw_check_rc()->ctcm_chx_iofatal():
	fsm_event(priv->fsm, ...);

The circular chain in the commit message also goes through priv->fsm or
priv->mpcg. For example, mpc_action_go_inop() does this before it reaches
tasklet_hi_schedule():

	grp =  priv->mpcg;
	grp->flow_off_called = 0;

At channel_remove() time priv->fsm and priv->mpcg are already freed or
NULL. Wouldn't that chain crash before it can re-arm anything?

If so, timer_shutdown_sync() only waits for a callback that is already
dereferencing freed or NULL memory. tasklet_kill() also lets a queued
ch_tasklet run against the freed grp.

channel_free() moves ch->fsm to CTC_STATE_IDLE, which has no handlers,
so most late ch->fsm events do nothing. But that state change takes no
lock. ctcm_irq_handler() runs FSM actions under the ccwdev lock, and for
MPC ctcm_close() does nothing:

	if (!IS_MPC(priv))
		fsm_event(priv->fsm, DEV_EVENT_STOP, dev);

As a result the MPC channels are not halted. An action that races
channel_free() could leave the channel FSM in TXIDLE or TX with timers
armed.

ctcmpc_bh() never checks the ch->fsm state, and ctc_mpc_flow_control()
can schedule it at any time.

The ctcm_new_device() out_dev path uses the same order, but its channels
were never started there.

Could both channels' timers and tasklets be stopped, and the subchannels
taken offline, before ctcm_free_netdevice() and kfree_fsm(priv->fsm) run?
That would mean moving the quiescing out of channel_remove() instead of
reordering it inside channel_remove().

[Severity: High]
This isn't a bug introduced by this patch, but the other fsm_timers in
this driver seem to follow the same pattern: cancel without a sync, then
free.

priv->restart_timer is bound to priv->fsm by fsm_settimer() in
ctcm_init_netdevice(). grp->timer is bound to grp->fsm by fsm_settimer()
in ctcmpc_init_mpc_group(). Neither timer is ever cancelled with a sync
call. The only cancellations are fsm_deltimer(), which is timer_delete().

mpc_action_go_inop() always re-arms the restart timer:

	fsm_deltimer(&priv->restart_timer);
	fsm_addtimer(&priv->restart_timer, 500, DEV_EVENT_RESTART, dev);

dev_action_restart() re-arms it for 1s in the MPC case. So during MPC
group recovery the restart timer is usually pending.

For MPC, ctcm_close() does nothing. So ctcm_shutdown_device() then calls
ctcm_free_netdevice(), which frees grp->fsm, grp (which contains
grp->timer) and priv->fsm while these timers may be pending or running.
After that, ctcm_remove_device() does kfree(priv), and priv contains
restart_timer.

When one of these timers expires, fsm_expire_timer() calls
fsm_event(this->fi, ...) on a freed fsm_instance. That reads the freed
state and jumpmatrix, and calls a function pointer taken from freed
memory. A timer still pending inside freed grp or priv memory can also
corrupt the timer wheel.

There is a similar ordering problem in ctcm_free_netdevice():

	if (grp->fsm)
		kfree_fsm(grp->fsm);
	dev_kfree_skb(grp->xid_skb);
	dev_kfree_skb(grp->rcvd_xid_skb);
	tasklet_kill(&grp->mpc_tasklet2);
	kfree(grp);

Can a pending mpc_tasklet2 run here after its FSM has been freed?

Should priv->restart_timer and grp->timer also get timer_shutdown_sync()
before priv->fsm and grp are freed? And should mpc_tasklet2 be killed
before kfree_fsm(grp->fsm)?

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260930070234.569500-1-nagamani%40linux.ibm.com

Re: [PATCH net v4 2/2] s390/ctcm: Fix use-after-free in channel_remove()

[Nagamani PV]

On 04/10/26 12:47 PM, netdev-bot+sashiko@kernel.org wrote:
> [Severity: High]
> Do these waits run too late to protect the state that the channel
> callbacks use?
> 
> The only teardown caller of channel_remove() is ctcm_shutdown_device(),
> and it calls it after the net_device and its private state are gone:
> 
> drivers/s390/net/ctcm_main.c:ctcm_shutdown_device() {
>     ...
> 	if (dev) {
> 		unregister_netdev(dev);
> 		ctcm_free_netdevice(dev);
> 	}
>     ...
> 	ccw_device_set_offline(cgdev->cdev[1]);
> 	ccw_device_set_offline(cgdev->cdev[0]);
> 	channel_remove(priv->channel[CTCM_READ]);
> 	channel_remove(priv->channel[CTCM_WRITE]);
>     ...
> }
> 
> ctcm_free_netdevice() does kfree_fsm(grp->fsm), kfree(grp),
> priv->mpcg = NULL, kfree_fsm(priv->fsm) and priv->fsm = NULL. Under MODULE
> it also calls free_netdev(dev). The ccw_device_set_offline() calls in
> between can sleep, so a channel timer or tasklet can fire in that window.
> 
> The channel callbacks reach this state through ch->netdev->ml_priv:
> 
>   ctcm_chx_txretry() (ch->timer):
> 	fsm_event(priv->fsm, DEV_EVENT_TXDOWN, dev);
> 
>   ctcmpc_chx_send_sweep() (sweep_timer):
> 	if (grp->in_sweep == 0)
> 
>   ctcmpc_bh() (ch_tasklet):
> 	while ((fsm_getstate(grp->fsm) != MPCG_STATE_FLOWC) &&
> 
>   ctcm_ccw_check_rc()->ctcm_chx_iofatal():
> 	fsm_event(priv->fsm, ...);
> 
> The circular chain in the commit message also goes through priv->fsm or
> priv->mpcg. For example, mpc_action_go_inop() does this before it reaches
> tasklet_hi_schedule():
> 
> 	grp =  priv->mpcg;
> 	grp->flow_off_called = 0;
> 
> At channel_remove() time priv->fsm and priv->mpcg are already freed or
> NULL. Wouldn't that chain crash before it can re-arm anything?
> 
> If so, timer_shutdown_sync() only waits for a callback that is already
> dereferencing freed or NULL memory. tasklet_kill() also lets a queued
> ch_tasklet run against the freed grp.
> 
> channel_free() moves ch->fsm to CTC_STATE_IDLE, which has no handlers,
> so most late ch->fsm events do nothing. But that state change takes no
> lock. ctcm_irq_handler() runs FSM actions under the ccwdev lock, and for
> MPC ctcm_close() does nothing:
> 
> 	if (!IS_MPC(priv))
> 		fsm_event(priv->fsm, DEV_EVENT_STOP, dev);
> 
> As a result the MPC channels are not halted. An action that races
> channel_free() could leave the channel FSM in TXIDLE or TX with timers
> armed.
> 
> ctcmpc_bh() never checks the ch->fsm state, and ctc_mpc_flow_control()
> can schedule it at any time.
> 
> The ctcm_new_device() out_dev path uses the same order, but its channels
> were never started there.
> 
> Could both channels' timers and tasklets be stopped, and the subchannels
> taken offline, before ctcm_free_netdevice() and kfree_fsm(priv->fsm) run?
> That would mean moving the quiescing out of channel_remove() instead of
> reordering it inside channel_remove().
> 
> [Severity: High]
> This isn't a bug introduced by this patch, but the other fsm_timers in
> this driver seem to follow the same pattern: cancel without a sync, then
> free.
> 
> priv->restart_timer is bound to priv->fsm by fsm_settimer() in
> ctcm_init_netdevice(). grp->timer is bound to grp->fsm by fsm_settimer()
> in ctcmpc_init_mpc_group(). Neither timer is ever cancelled with a sync
> call. The only cancellations are fsm_deltimer(), which is timer_delete().
> 
> mpc_action_go_inop() always re-arms the restart timer:
> 
> 	fsm_deltimer(&priv->restart_timer);
> 	fsm_addtimer(&priv->restart_timer, 500, DEV_EVENT_RESTART, dev);
> 
> dev_action_restart() re-arms it for 1s in the MPC case. So during MPC
> group recovery the restart timer is usually pending.
> 
> For MPC, ctcm_close() does nothing. So ctcm_shutdown_device() then calls
> ctcm_free_netdevice(), which frees grp->fsm, grp (which contains
> grp->timer) and priv->fsm while these timers may be pending or running.
> After that, ctcm_remove_device() does kfree(priv), and priv contains
> restart_timer.
> 
> When one of these timers expires, fsm_expire_timer() calls
> fsm_event(this->fi, ...) on a freed fsm_instance. That reads the freed
> state and jumpmatrix, and calls a function pointer taken from freed
> memory. A timer still pending inside freed grp or priv memory can also
> corrupt the timer wheel.
> 
> There is a similar ordering problem in ctcm_free_netdevice():
> 
> 	if (grp->fsm)
> 		kfree_fsm(grp->fsm);
> 	dev_kfree_skb(grp->xid_skb);
> 	dev_kfree_skb(grp->rcvd_xid_skb);
> 	tasklet_kill(&grp->mpc_tasklet2);
> 	kfree(grp);
> 
> Can a pending mpc_tasklet2 run here after its FSM has been freed?
> 
> Should priv->restart_timer and grp->timer also get timer_shutdown_sync()
> before priv->fsm and grp are freed? And should mpc_tasklet2 be killed
> before kfree_fsm(grp->fsm)?
> 
> [ ... ]
> 

Both are confirmed pre-existing issues in ctcm_shutdown_device() and
ctcm_free_netdevice(), outside the scope of channel_remove(). They are
tracked for a separate follow-up fix.

Nagamani

Re: [PATCH net v4 1/2] s390/ctcm: Fix timer corruption in fsm_addtimer()

[Simon Horman]

On Wed, Sep 30, 2026 at 09:02:33AM +0200, Nagamani PV wrote:
> fsm_addtimer() calls timer_setup() unconditionally before add_timer().
> If called on an already-pending timer, timer_setup() re-initializes
> the timer's list_head fields while the timer is still enqueued in the
> wheel, corrupting the timer list.
> 
> The timer is already initialized once by fsm_settimer() which calls
> timer_setup() correctly. Multiple callsites invoke fsm_addtimer()
> without a preceding fsm_deltimer(), including ctcm_main.c
> ctcmpc_send_sweep_req() and ctcm_mpc.c mpc_action_side_xid(), making the
> redundant timer_setup() in fsm_addtimer() a real corruption risk.
> 
> Remove the redundant timer_setup() calls from fsm_addtimer() and
> fsm_modtimer(), and replace add_timer() with mod_timer() which safely
> handles both pending and non-pending timers atomically without
> corrupting the timer wheel.
> 
> Fixes: e99e88a9d2b0 ("treewide: setup_timer() -> timer_setup()")
> Cc: stable@vger.kernel.org
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Link: https://sashiko.dev/#/patchset/20260803182736.2356374-1-nagamani@linux.ibm.com?part=1
> Reviewed-by: Aswin Karuvally <aswin@linux.ibm.com>
> Tested-by: Aswin Karuvally <aswin@linux.ibm.com>
> Signed-off-by: Nagamani PV <nagamani@linux.ibm.com>

Reviewed-by: Simon Horman <horms@kernel.org>

Re: [PATCH net v4 2/2] s390/ctcm: Fix use-after-free in channel_remove()

[Simon Horman]

On Wed, Sep 30, 2026 at 09:02:34AM +0200, Nagamani PV wrote:
> channel_remove() uses fsm_deltimer() which calls timer_delete(),
> returning immediately even if the timer callback is running on
> another CPU. This leaves a window where fsm_expire_timer() accesses
> ch->fsm after kfree_fsm().
> 
> For MPC channels the teardown has a circular dependency:
> 
>   ch->timer callback -> ctcm_chx_txretry() -> mpc_action_go_inop()
>     -> tasklet_hi_schedule(&ch->ch_disc_tasklet)
> 
>   ch_tasklet -> ctcmpc_send_sweep_resp() -> fsm_addtimer(&ch->sweep_timer)
> 
>   sweep_timer callback -> fsm_addtimer(&ch->timer)
> 
> Use timer_shutdown_sync() for both timers: it waits for any running
> callback and permanently prevents rearming. Shut down sweep_timer
> first (its callback rearms ch->timer at ctcm_fsms.c), then
> ch->timer. Only then call tasklet_kill() -- catching any tasklet
> scheduled by an in-flight callback before shutdown.
> kfree(ch->discontact_th) follows tasklet_kill(ch_disc_tasklet) since
> ch->ccw[15].cda points to discontact_th and
> mpc_action_send_discontact() starts I/O through ch->ccw[15].
> 
> timer_shutdown_sync() is also used for non-MPC ch->timer:
> ctcm_chx_txretry() (CTC_STATE_TX + CTC_EVENT_TIMER) calls
> fsm_addtimer(&ch->timer) on both paths.
> 
> Depends on "s390/ctcm: Fix timer corruption in fsm_addtimer()" which
> replaces add_timer() with mod_timer(). mod_timer() on a shut-down
> timer is discarded; without it fsm_addtimer() calls timer_setup()
> defeating timer_shutdown_sync(). Backporting without that
> prerequisite is unsafe.
> 
> Fixes: 293d984f0e36 ("ctcm: infrastructure for replaced ctc driver")
> Cc: stable@vger.kernel.org # depends on: s390/ctcm: Fix timer corruption in fsm_addtimer()
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Link: https://sashiko.dev/#/patchset/20260922101913.239103-1-nagamani@linux.ibm.com?part=2
> Signed-off-by: Nagamani PV <nagamani@linux.ibm.com>
> ---
> Changes in v4:
> - Shut down timers before tasklet_kill() rather than after,
>   shutting down sweep_timer before ch->timer.
> - Use timer_shutdown_sync() for ch->timer on both MPC and non-MPC paths.
> - Name prerequisite by commit title and add structured stable tag
>   dependency (Jakub Kicinski).
> - Code changed; Reviewed-by and Tested-by dropped.

Reviewed-by: Simon Horman <horms@kernel.org>

Re: [PATCH net v4 0/2] s390/ctcm: Fix timer corruption and use-after-free

[Jakub Kicinski]

On Thu, 1 Oct 2026 13:36:13 +0530 Nagamani PV wrote:
> > This is an automated message. This series looks like a fix, but its
> > commit messages seem to be missing some information:
> > 
> >  - Whether the issue was actually triggered, or is only theoretical
> >    (e.g. found by code inspection). If it was triggered please include
> >    the symptoms, like the stack trace or error messages.
> > 
> > Please do not repost the series just to address the above. Instead,
> > reply to this email with the missing information, so that reviewers
> > can take it into account. If the series needs another revision for
> > other reasons, please include the information in the commit messages
> > then.
> > 
> > The evaluation is done by an LLM so it may be wrong, if you think
> > that is the case please reply and explain.  
> 
> Both issues were identified through static code inspection / automated
> analysis (Sashiko code review) and confirmed by manual code review.
> They have not been triggered in production with an observed stack trace.

To be clear - the question is whether you triggered it _at all_
(validation), not whether they were hit in production (discovery).
If author does not bother to validate the trigger the patches are
usually not worth sending to LTS. Also - the Fixes tag on patch 1
looked sus, I changed it.

Re: [PATCH net v4 0/2] s390/ctcm: Fix timer corruption and use-after-free

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Wed, 30 Sep 2026 09:02:32 +0200 you wrote:
> Fix two bugs in drivers/s390/net/fsm.c and drivers/s390/net/ctcm_main.c
> found by Sashiko AI code review.
> 
> Patch 1 fixes timer list corruption when fsm_addtimer() is called on
> an already-pending timer - timer_setup() re-initializes the timer
> list_head while it is still enqueued in the wheel.
> 
> [...]

Here is the summary with links:
  - [net,v4,1/2] s390/ctcm: Fix timer corruption in fsm_addtimer()
    https://git.kernel.org/netdev/net-next/c/10c339aff290
  - [net,v4,2/2] s390/ctcm: Fix use-after-free in channel_remove()
    https://git.kernel.org/netdev/net-next/c/65629387fd4c

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html