[PATCH net 0/2] s390/ctcm: Fix timer bugs in fsm.c

[PATCH net 0/2] s390/ctcm: Fix timer bugs in fsm.c

[Nagamani PV]

Fix two bugs in drivers/s390/net/fsm.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() - timer_delete()
returns before any running callback finishes, leaving a window where
the timer callback can access freed memory. Fix by waiting for any
running callback to complete before freeing the channel.

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 |  4 ++--
 drivers/s390/net/fsm.c       | 10 ++--------
 2 files changed, 4 insertions(+), 10 deletions(-)

-- 
2.47.1

[PATCH net 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
ctcm_send_sweep() 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>
---
 drivers/s390/net/fsm.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/s390/net/fsm.c b/drivers/s390/net/fsm.c
index 9b8622aef807..8d2e9bbb74a2 100644
--- a/drivers/s390/net/fsm.c
+++ b/drivers/s390/net/fsm.c
@@ -171,12 +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);
-	return 0;
+	return mod_timer(&this->tl, jiffies + msecs_to_jiffies(millisec));
 }
 
 /* FIXME: this function is never used, why */
@@ -189,12 +186,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

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

[sashiko-bot]

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

Pre-existing issues:
- [High] Use-After-Free during teardown due to non-synchronous timer cancellation of grp->timer and priv->restart_timer
--

commit 6ecc23adfe371b690518fad3a360058f37350370
Author: Nagamani PV <nagamani@linux.ibm.com>
Subject: s390/ctcm: Fix timer corruption in fsm_addtimer()

fsm_addtimer() calls timer_setup() unconditionally before add_timer(), which
corrupts the timer list if the timer is already pending. This commit removes
the redundant timer_setup() calls from fsm_addtimer() and fsm_modtimer(),
and replaces add_timer() with mod_timer().

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

> diff --git a/drivers/s390/net/fsm.c b/drivers/s390/net/fsm.c
> index 9b8622aef807b..8d2e9bbb74a23 100644
> --- a/drivers/s390/net/fsm.c
> +++ b/drivers/s390/net/fsm.c

[ ... ]

> @@ -189,12 +186,9 @@ fsm_modtimer(fsm_timer *this, int millisec, int event, void *arg)
>  		this->fi->name, this, millisec);
>  #endif
>  
> -	timer_delete(&this->tl);

[Severity: High]
This is a pre-existing issue, but while reviewing the timer handling changes
here, I noticed a potential Use-After-Free during device teardown due to
fsm_deltimer() using the non-synchronous timer_delete().

While a later patch in this series ("s390/ctcm: Fix use-after-free in
channel_remove()") correctly avoids fsm_deltimer() in favor of
timer_delete_sync() for ch->timer, it appears the same vulnerability remains
for grp->timer and priv->restart_timer.

During device unbind or offline actions, the driver calls fsm_deltimer()
to cancel these timers and then immediately frees the enclosing structures.

For instance, in drivers/s390/net/ctcm_main.c:ctcm_free_netdevice():

	tasklet_kill(&grp->mpc_tasklet2);
	kfree(grp);

And similarly in ctcm_remove_device():

	if (cgdev->state == CCWGROUP_ONLINE)
		ctcm_shutdown_device(cgdev);
	dev_set_drvdata(&cgdev->dev, NULL);
	kfree(priv);

If the fsm_expire_timer() callback is executing concurrently on another CPU
when kfree() is called, could it dereference the freed grp or priv
structures?

Would it make sense to update fsm_deltimer() to use timer_delete_sync() to
safely wait for concurrent callbacks, or otherwise ensure synchronization
during teardown?

> -	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));
>  }

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

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

[Nagamani PV]

channel_remove() calls fsm_deltimer() which internally uses
timer_delete(), then immediately frees the channel structure:

  fsm_deltimer(&ch->timer);
  kfree_fsm(ch->fsm);       /* freed while callback may still run */
  kfree(ch);

timer_delete() returns immediately even if the timer callback is
currently executing on another CPU, creating a window where
fsm_expire_timer() accesses this->fi (which points to ch->fsm)
after it has been freed by kfree_fsm().

Fix this by calling timer_delete_sync() directly on the underlying
timer_list fields before freeing, instead of going through
fsm_deltimer(). timer_delete_sync() is used rather than
timer_shutdown_sync() because the channel is fully torn down by
channel_remove() and the timer is never re-armed after this point,
making the re-arm prevention of timer_shutdown_sync() unnecessary.
This cannot be fixed in fsm_deltimer() itself because FSM action
functions triggered by CTC_EVENT_TIMER call fsm_deltimer() from within
the timer callback chain, which would cause a self-deadlock in
timer_delete_sync().

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>
---
 drivers/s390/net/ctcm_main.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/s390/net/ctcm_main.c b/drivers/s390/net/ctcm_main.c
index 8b0d76a47d9f..63f8832f9b14 100644
--- a/drivers/s390/net/ctcm_main.c
+++ b/drivers/s390/net/ctcm_main.c
@@ -211,9 +211,9 @@ static void channel_remove(struct channel *ch)
 	while (*c) {
 		if (*c == ch) {
 			*c = ch->next;
-			fsm_deltimer(&ch->timer);
+			timer_delete_sync(&ch->timer.tl);
 			if (IS_MPC(ch))
-				fsm_deltimer(&ch->sweep_timer);
+				timer_delete_sync(&ch->sweep_timer.tl);
 
 			kfree_fsm(ch->fsm);
 			clear_normalized_cda(&ch->ccw[4]);
-- 
2.53.0

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

[sashiko-bot]

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

Pre-existing issues:
- [High] The patch fixes a use-after-free for `ch->timer` by switching to `timer_delete_sync()`, but fails to apply the same fix for `priv->restart_timer` and `grp->timer`, leaving identical use-after-free vulnerabilities in the teardown paths.
- [High] Lockless traversal and modification of the global `channels` list can lead to list corruption and crashes.
--

commit 0701d8bf2eb4b9bc69bd86286f3b23ff8226be83
Author: Nagamani PV <nagamani@linux.ibm.com>

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

The commit fixes a use-after-free vulnerability by transitioning from
fsm_deltimer() to timer_delete_sync() in channel_remove(). This prevents
timer callbacks from executing after the channel structure has been freed.

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

> diff --git a/drivers/s390/net/ctcm_main.c b/drivers/s390/net/ctcm_main.c
> index 8b0d76a47d9f0..63f8832f9b149 100644
> --- a/drivers/s390/net/ctcm_main.c
> +++ b/drivers/s390/net/ctcm_main.c
> @@ -211,9 +211,9 @@ static void channel_remove(struct channel *ch)
>  	while (*c) {
>  		if (*c == ch) {
>  			*c = ch->next;

[Severity: High]
This is a pre-existing issue, but does modifying the global channels list
locklessly here race with other list modifications?

For example, in drivers/s390/net/ctcm_main.c:add_channel():

	ch->next = *c;
	*c = ch;

Since device teardown and probe can be triggered concurrently by sysfs
writes to the online attribute, could this lead to list corruption or
crashes?

> -			fsm_deltimer(&ch->timer);
> +			timer_delete_sync(&ch->timer.tl);

[Severity: High]
This isn't a bug introduced by this patch, but should a similar fix be
applied to grp->timer and priv->restart_timer?

In drivers/s390/net/ctcm_main.c:ctcm_free_netdevice(), grp->timer is not
synchronously canceled before the FSM and mpc_group are freed:

	if (grp->fsm)
		kfree_fsm(grp->fsm);
	...
	kfree(grp);

Similarly, in drivers/s390/net/ctcm_main.c:ctcm_remove_device(),
priv->restart_timer is not synchronously canceled before priv is freed:

	kfree(priv);

Since unprivileged userspace can trigger device teardown via the sysfs
online attribute, could a concurrent timer callback access the freed FSM
and driver state, leading to a use-after-free?

>  			if (IS_MPC(ch))
> -				fsm_deltimer(&ch->sweep_timer);
> +				timer_delete_sync(&ch->sweep_timer.tl);
>  
>  			kfree_fsm(ch->fsm);
>  			clear_normalized_cda(&ch->ccw[4]);

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