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

[sashiko-bot@kernel.org]

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