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

On 10/09/26 8:16 AM, netdev-bot+sashiko@kernel.org wrote:

- [High] Incomplete fix: the commit removes timer_setup() from
  fsm_addtimer()/fsm_modtimer() on the premise that "[t]he timer is…

This refers to priv->restart_timer — addressed inline below.

[Severity: Low]
There is no symbol named ctcm_send_sweep() in the tree. The ctcm_main.c
call site that arms a timer without a preceding fsm_deltimer() appears to
be ctcmpc_send_sweep_req():

Correct. Will fix the function name in v2.

[Severity: High]
With this removed, fsm_settimer() becomes the only timer_setup() call site
for fsm timers, and it is also called unconditionally. Does that leave the
same corruption reachable for priv->restart_timer?

Pre-existing issue not introduced by this patch.

[Severity: Low]
This now propagates the mod_timer() result, but the kernel-doc for the
exported fsm_addtimer() in fsm.h still says:

 * Return: Always returns 0
Would it be better to keep mod_timer(...) followed by return 0, or update
the fsm.h kernel-doc to describe the 0/1 "was pending" semantics?

Will fix in v2 by calling mod_timer() separately and returning 0
explicitly, preserving the documented return contract.

[Severity: Medium]
This isn't a bug introduced by this patch, but with the timer_delete()
gone, the two payload stores are now published with plain stores while the
timer may be pending or its callback already running:

Pre-existing issue not introduced by this patch.

[Severity: High]
This is a pre-existing issue, but it interacts with the switch to
mod_timer(). fsm_deltimer() only does:

	timer_delete(&this->tl);

which does not wait for a callback running on another CPU, and
ctcm_free_netdevice() performs no cancellation at all for grp->timer or
priv->restart_timer before freeing their containers:

Pre-existing issue not introduced by this patch.

[Severity: Critical]
This isn't a bug introduced by this patch, but ctcm_free_netdevice() frees
priv->xid with kfree() even though it is an interior pointer into an SKB
data buffer that was released a few statements earlier:

Pre-existing issue not introduced by this patch.

[Severity: Medium]
This is a pre-existing issue as well, but does ctcm_init_netdevice() leak
priv->fsm when the MPC group setup fails?

Pre-existing issue not introduced by this patch.