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

[Nagamani PV <nagamani@linux.ibm.com>]

On 22/09/26 4:45 AM, Jakub Kicinski wrote:
> On Wed, 16 Sep 2026 15:21:47 +0200 Nagamani PV wrote:
>> 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.
>>
>> 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)
> 
> Clashiko won't respond because of the unnecessary threading,
> but there are 4 high severity findings needing your attention:
> 
> https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260916132149.4185912-3-nagamani@linux.ibm.com

Thank you for the pointer. I have reviewed all four High findings.
Two share a root cause and are addressed in v3, sent Tue 22 Sep 2026
as "[PATCH net v3 0/2] s390/ctcm: Fix timer corruption and use-after-free"
<20260922101913.239103-1-nagamani@linux.ibm.com> (raw). The remaining two
are pre-existing issues outside the scope of this series and will be
fixed in a follow-up patch.

--- Findings addressed in v3 ---

1. [High] ch_tasklet re-arms sweep_timer after timer_shutdown_sync()
   returns (Sashiko patch 2/2, finding 2):

   ch_tasklet (ctcmpc_bh -> ctcmpc_send_sweep_resp) can re-arm
   sweep_timer after timer_shutdown_sync() returns because the v2 patch
   killed tasklets after stopping the timers.

2. [High] kfree_fsm(ch->fsm) before tasklet_kill() races with a running
   tasklet accessing ch->fsm (Sashiko patch 2/2, finding 5).

   v3 fix for both: kill ch_tasklet and ch_disc_tasklet first, then
   timer_shutdown_sync(&ch->sweep_timer.tl), then
   timer_delete_sync(&ch->timer.tl), before freeing ch->fsm.
   kfree(ch->discontact_th) is also moved into the MPC teardown block
   after tasklet_kill(ch_disc_tasklet).

--- Pre-existing findings tracked for follow-up ---

3. [High] grp->timer / priv->restart_timer not cancelled before free in
   ctcm_free_netdevice() (Sashiko patch 1/2 finding 2, patch 2/2
   finding 1): predates this series (commit 293d984f0e36), out of scope
   here. Will be fixed in a follow-up patch.

4. [High] NULL deref on peekskb->len in ctcmpc_chx_txdone()
   ctcm_fsms.c:1299 (Sashiko patch 1/2, finding 1): skb_peek() returns
   NULL when collect_queue empties. Predates this series. Same
   follow-up patch.

--- v3 summary ---

  Patch 1 (fsm.c): unchanged from v2.
  Patch 2 (ctcm_main.c): tasklet_kill() moved before timer stops;
    kfree(discontact_th) moved into the MPC teardown block.

Aswin Karuvally will review and test the new teardown order.

Nagamani