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

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

On 10/09/26 8:16 AM, netdev-bot+sashiko@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential
> issue(s) to consider:
> 
> Critical: 0 · High: 1 (3) · Medium: 0 · Low: 1
> 
> - [High] channel_remove() (drivers/s390/net/ctcm_main.c:214/216) uses
>   timer_delete_sync() instead of timer_shutdown_sync().
> - [Low] The patch fixes a real use-after-free reachable during device
>   teardown but carries no `Fixes:` tag, so stable maintainers cannot…
> 
> Pre-existing issues:
> - [High] The same fsm_timer/kfree_fsm() UAF pattern that this patch fixes
>   for ch->timer/ch->sweep_timer remains completely unfixed for the…
> - [High] The synchronous cancel was added in channel_remove(), but
>   ctcm_shutdown_device() frees the objects that timer-driven FSM actions…
> - [High] The MPC channel tasklets are killed only after the resources
>   they dereference have been freed.
> 
> --
> 
> [Severity: Low]
> There is no Fixes: tag on this patch. git blame on the two replaced
> fsm_deltimer() calls and the following kfree_fsm(ch->fsm) points at:
> 
>   293d984f0e3604 ("ctcm: infrastructure for replaced ctc driver")
> 
> Could that be added as a Fixes: tag so stable can work out the affected
> range?
> 

Will add in v2:
  Fixes: 293d984f0e36 ("ctcm: infrastructure for replaced ctc driver")
  Cc: stable@vger.kernel.org
 > [Severity: High]
> Does that hold for the MPC case?  timer_delete_sync() leaves
> timer->function intact, so a later fsm_addtimer() simply re-queues the
> timer:
> For ch == priv->channel[CTCM_WRITE], can a sweep callback that has already
> been dispatched on another CPU execute the fsm_addtimer(&wch->timer, ...)
> above after timer_delete_sync(&ch->timer.tl) has returned?  

Pre-existing issue not introduced by this patch.

> 
> [Severity: High]
> This is a pre-existing ordering problem and not something the patch
> introduces, but kfree_fsm(ch->fsm) here runs before the MPC tasklets
> are stopped later in the same function.

Pre-existing issue not introduced by this patch.

> [Severity: High]
> This is a pre-existing gap rather than a regression from this patch, but
> the driver has two more fsm_timer instances with the same kfree_fsm()
> pattern, and neither is cancelled at all in the teardown path.

Pre-existing issue not introduced by this patch.

> 
> [Severity: High]
> Also pre-existing, but related to where the new synchronous cancel sits in
> the overall shutdown ordering.  In ctcm_shutdown_device() the objects the
> channel timer callbacks dereference are freed before channel_remove() runs:
> 

Pre-existing issue not introduced by this patch.

Nagamani