Re: [PATCH net-next v8 3/3] w5100: detect carrier state using link status bit and optional interrupt

[Arthur Crepin Leblond <arthur@marmottus.net>]

On Wed, Sep 16, 2026 at 08:07:54AM +0000, sashiko-bot@kernel.org wrote:
>> @@ -820,14 +953,22 @@ void w5100_remove(struct device *dev)
>>  	struct net_device *ndev = dev_get_drvdata(dev);
>>  	struct w5100_priv *priv = netdev_priv(ndev);
>>
>> -	w5100_hw_reset(priv);
>> +	unregister_netdev(ndev);
>> +
>> +	cancel_work_sync(&priv->rx_work);
>> +	cancel_work_sync(&priv->tx_work);
>> +	cancel_work_sync(&priv->setrx_work);
>> +	cancel_work_sync(&priv->restart_work);
>> +
>> +	if (priv->link_irq > 0)
>> +		free_irq(priv->link_irq, ndev);
>> +
>>  	free_irq(priv->irq, ndev);
>
>[Severity: High]
>Does this code introduce a teardown race by canceling the work queues before
>freeing the interrupts?
>
>If the threaded interrupt handler (w5100_interrupt) is running concurrently
>during w5100_remove(), it could queue a new rx_work or tx_work immediately
>after cancel_work_sync() returns.
>
>Later in the teardown, destroy_workqueue() drains and forcibly executes all
>pending works. Since w5100_hw_reset() is called prior to destroy_workqueue(),
>could these newly queued works execute against a reset and uninitialized
>hardware state, potentially causing I/O corruption or kernel crashes?
>
>Should free_irq() be called before cancel_work_sync() to guarantee the
>interrupt handler has completed and no new works will be queued?

Good catch and seems like it's the only issue you found! Let's see
what Clashiko will find. I'll fix that in the next patch.

Arthur